Delete replication metrics upon project deletion
Change I7259f520 introduced the handling of
ProjectDeletionReplicationSucceededEvent events to remove the metrics of
deleted projects upon successful replication of project deletions.
However the metrics were not removed from the gerrit instance that
initiated the replication, since it did not consume the
ProjectDeletionReplicationSucceededEvent produced by itself.
Register ReplicationStatus as a ProjectDeletedListener so that local
replication metrics can be removed as soon as the project is deleted
from the local repository.
Bug: Issue 12958
Change-Id: I5c3f2e51fddace3d76cf5b94ecd19dc4c3004440
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatus.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatus.java
index 2f38720..7ce4606 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatus.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatus.java
@@ -19,6 +19,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.events.ProjectDeletedListener;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.cache.serialize.JavaCacheSerializer;
import com.google.gerrit.server.cache.serialize.StringCacheSerializer;
@@ -39,7 +40,7 @@
import java.util.stream.Collectors;
@Singleton
-public class ReplicationStatus implements LifecycleListener {
+public class ReplicationStatus implements LifecycleListener, ProjectDeletedListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final Map<String, Long> replicationStatusPerProject = new HashMap<>();
@@ -121,7 +122,7 @@
void removeProjectFromReplicationLagMetrics(Project.NameKey projectName) {
Optional<Long> localVersion = projectVersionRefUpdate.getProjectLocalVersion(projectName.get());
- if (!localVersion.isPresent() && localVersionPerProject.containsKey(projectName.get())) {
+ if (!localVersion.isPresent() && replicationStatusPerProject.containsKey(projectName.get())) {
cache.invalidate(projectName.get());
replicationStatusPerProject.remove(projectName.get());
localVersionPerProject.remove(projectName.get());
@@ -159,4 +160,9 @@
projectCache.all().stream().map(Project.NameKey::get).collect(Collectors.toSet());
replicationStatusPerProject.putAll(cache.getAllPresent(cachedProjects));
}
+
+ @Override
+ public void onProjectDeleted(Event event) {
+ removeProjectFromReplicationLagMetrics(Project.nameKey(event.getProjectName()));
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusModule.java
index e954bc4..791517f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusModule.java
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.multisite.consumer;
+import com.google.gerrit.extensions.events.ProjectDeletedListener;
+import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.inject.Scopes;
@@ -23,5 +25,6 @@
bind(ReplicationStatus.class).in(Scopes.SINGLETON);
install(ReplicationStatus.cacheModule());
listener().to(ReplicationStatus.class);
+ DynamicSet.bind(binder(), ProjectDeletedListener.class).to(ReplicationStatus.class);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusTest.java
index beb93dd..aec9404 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusTest.java
@@ -15,15 +15,18 @@
package com.googlesource.gerrit.plugins.multisite.consumer;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableSortedSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.events.ProjectDeletedListener;
import com.google.gerrit.server.project.ProjectCache;
import com.googlesource.gerrit.plugins.multisite.ProjectVersionLogger;
import com.googlesource.gerrit.plugins.multisite.validation.ProjectVersionRefUpdate;
+import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -86,4 +89,73 @@
assertThat(replicationStatusCache.getIfPresent("projectA")).isEqualTo(20L);
}
+
+ @Test
+ public void shouldRemoveProjectFromPersistedCache() {
+ String projectName = "projectA";
+ long lag = 100;
+ setupReplicationLag(projectName, lag);
+ when(projectVersionRefUpdate.getProjectLocalVersion(projectName)).thenReturn(Optional.empty());
+
+ objectUnderTest.onProjectDeleted(projectDeletedEvent(projectName));
+
+ assertThat(replicationStatusCache.getIfPresent(projectName)).isNull();
+ }
+
+ @Test
+ public void shouldRemoveProjectFromReplicationLags() {
+ String projectName = "projectA";
+ long lag = 100;
+ setupReplicationLag(projectName, lag);
+ when(projectVersionRefUpdate.getProjectLocalVersion(projectName)).thenReturn(Optional.empty());
+
+ assertThat(objectUnderTest.getReplicationLags(1).keySet()).containsExactly(projectName);
+
+ objectUnderTest.onProjectDeleted(projectDeletedEvent(projectName));
+
+ assertThat(objectUnderTest.getReplicationLags(1).keySet()).isEmpty();
+ }
+
+ @Test
+ public void shouldNotRemoveProjectFromReplicationLagsIfLocalVersionStillExists() {
+ String projectName = "projectA";
+ long lag = 100;
+ setupReplicationLag(projectName, lag);
+ when(projectVersionRefUpdate.getProjectLocalVersion(projectName))
+ .thenReturn(Optional.of(System.currentTimeMillis()));
+
+ objectUnderTest.onProjectDeleted(projectDeletedEvent(projectName));
+
+ assertThat(objectUnderTest.getReplicationLags(1).keySet()).containsExactly(projectName);
+ }
+
+ @Test
+ public void shouldNotEvictFromPersistentCacheIfLocalVersionStillExists() {
+ String projectName = "projectA";
+ long lag = 100;
+ setupReplicationLag(projectName, lag);
+ when(projectVersionRefUpdate.getProjectLocalVersion(projectName))
+ .thenReturn(Optional.of(System.currentTimeMillis()));
+
+ objectUnderTest.onProjectDeleted(projectDeletedEvent(projectName));
+
+ assertThat(replicationStatusCache.getIfPresent(projectName)).isEqualTo(lag);
+ }
+
+ private void setupReplicationLag(String projectName, long lag) {
+ long currentVersion = System.currentTimeMillis();
+ long newVersion = currentVersion + lag;
+ replicationStatusCache.put(projectName, 3L);
+ when(projectVersionRefUpdate.getProjectRemoteVersion(projectName))
+ .thenReturn(Optional.of(newVersion));
+ when(projectVersionRefUpdate.getProjectLocalVersion(projectName))
+ .thenReturn(Optional.of(currentVersion));
+ objectUnderTest.updateReplicationLag(Project.nameKey(projectName));
+ }
+
+ private ProjectDeletedListener.Event projectDeletedEvent(String projectName) {
+ ProjectDeletedListener.Event event = mock(ProjectDeletedListener.Event.class);
+ when(event.getProjectName()).thenReturn(projectName);
+ return event;
+ }
}
