Update the refs/multi-site/version/value once per batch-refupdate The update of refs/multi-site/version/value on the global-refdb is a very expensive operation for two reasons: - The global-refdb is a remote system, potentially with a high network latency - It needs to maintain global consistency across geographical areas which would also include sever-side synchronisation Reducing the number of updates on the global-refdb is paramount for keeping an acceptable performance in Gerrit updates. When multiple updates happen in the same batch for Gerrit, it would not make sense to update the global-refdb multiple times for the version value because globally the batch represents a unit of work and performing it as a whole means updating the version of the repository once. Move from individual stream-events to batch ref-update listener for updating the refs/multi-site/version/value, so that multiple updates would result in a single global-refdb update. Note that the batch ref-update listener is not a stream event and therefore the code does not need to check and test the instance-id anymore because all the events received are always local. For example, the upload of a new patch-set for a change was previously generating four different updates on the global-refdb for the refs/multi-site/version/value, whilst now there is only one single update generated for the whole batch. Change-Id: Ia6c7f5aa085a8e2770b40df34145bb937cf1772e
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java index 5e70aca..8c67823 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java
@@ -16,9 +16,9 @@ import com.gerritforge.gerrit.eventbroker.publisher.StreamEventPublisherConfig; import com.gerritforge.gerrit.eventbroker.publisher.StreamEventPublisherModule; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.lifecycle.LifecycleModule; -import com.google.gerrit.server.events.EventListener; import com.google.inject.Inject; import com.google.inject.multibindings.OptionalBinder; import com.googlesource.gerrit.plugins.multisite.Configuration; @@ -47,7 +47,8 @@ OptionalBinder<ProjectVersionRefUpdate> projectVersionRefUpdateBinder = OptionalBinder.newOptionalBinder(binder(), ProjectVersionRefUpdate.class); if (configuration.getSharedRefDbConfiguration().getSharedRefDb().isEnabled()) { - DynamicSet.bind(binder(), EventListener.class).to(ProjectVersionRefUpdateImpl.class); + DynamicSet.bind(binder(), GitBatchRefUpdateListener.class) + .to(ProjectVersionRefUpdateImpl.class); projectVersionRefUpdateBinder.setBinding().to(ProjectVersionRefUpdateImpl.class); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java index 3e73bf1..150ea52 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java
@@ -24,10 +24,7 @@ import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project.NameKey; -import com.google.gerrit.server.config.GerritInstanceId; -import com.google.gerrit.server.events.Event; -import com.google.gerrit.server.events.EventListener; -import com.google.gerrit.server.events.RefUpdatedEvent; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.inject.Inject; @@ -44,7 +41,8 @@ import org.eclipse.jgit.lib.Repository; @Singleton -public class ProjectVersionRefUpdateImpl implements EventListener, ProjectVersionRefUpdate { +public class ProjectVersionRefUpdateImpl + implements GitBatchRefUpdateListener, ProjectVersionRefUpdate { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final Set<RefUpdate.Result> SUCCESSFUL_RESULTS = ImmutableSet.of(RefUpdate.Result.NEW, RefUpdate.Result.FORCED, RefUpdate.Result.NO_CHANGE); @@ -52,7 +50,6 @@ private final GitRepositoryManager gitRepositoryManager; private final GitReferenceUpdated gitReferenceUpdated; private final ProjectVersionLogger verLogger; - private final String nodeInstanceId; protected final SharedRefDatabaseWrapper sharedRefDb; @@ -61,35 +58,30 @@ GitRepositoryManager gitRepositoryManager, SharedRefDatabaseWrapper sharedRefDb, GitReferenceUpdated gitReferenceUpdated, - ProjectVersionLogger verLogger, - @GerritInstanceId String nodeInstanceId) { + ProjectVersionLogger verLogger) { this.gitRepositoryManager = gitRepositoryManager; this.sharedRefDb = sharedRefDb; this.gitReferenceUpdated = gitReferenceUpdated; this.verLogger = verLogger; - this.nodeInstanceId = nodeInstanceId; } @Override - public void onEvent(Event event) { - logger.atFine().log("Processing event type: %s", event.type); + public void onGitBatchRefUpdate(Event event) { // Producer of the Event use RefUpdatedEvent to trigger the version update - if (nodeInstanceId.equals(event.instanceId) && event instanceof RefUpdatedEvent) { - updateProducerProjectVersionUpdate((RefUpdatedEvent) event); - } + updateProducerProjectVersionUpdate(event); } - private void updateProducerProjectVersionUpdate(RefUpdatedEvent refUpdatedEvent) { - String refName = refUpdatedEvent.getRefName(); - - if (refName.equals(MULTI_SITE_VERSIONING_REF)) { + private void updateProducerProjectVersionUpdate(Event refUpdatedEvent) { + if (refUpdatedEvent.getRefNames().stream() + .allMatch(refName -> refName.equals(MULTI_SITE_VERSIONING_REF))) { logger.atFine().log( "Found a special ref name %s, skipping update for %s", - refName, refUpdatedEvent.getProjectNameKey().get()); + MULTI_SITE_VERSIONING_REF, refUpdatedEvent.getProjectName()); return; } + try { - Project.NameKey projectNameKey = refUpdatedEvent.getProjectNameKey(); + Project.NameKey projectNameKey = Project.nameKey(refUpdatedEvent.getProjectName()); long newVersion = getCurrentGlobalVersionNumber(); RefUpdate newProjectVersionRefUpdate = updateLocalProjectVersion(projectNameKey, newVersion); @@ -102,7 +94,7 @@ } catch (LocalProjectVersionUpdateException | SharedProjectVersionUpdateException e) { logger.atSevere().withCause(e).log( "Issue encountered when updating version for project %s", - refUpdatedEvent.getProjectNameKey()); + refUpdatedEvent.getProjectName()); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java index bceaa4f..e2e3113 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java
@@ -28,10 +28,9 @@ import static org.mockito.Mockito.when; import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDatabaseWrapper; -import com.google.common.base.Suppliers; import com.google.gerrit.entities.Project; -import com.google.gerrit.server.data.RefUpdateAttribute; -import com.google.gerrit.server.events.RefUpdatedEvent; +import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.project.ProjectConfig; import com.google.gerrit.testing.InMemoryRepositoryManager; @@ -43,6 +42,7 @@ import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; import java.util.Optional; +import java.util.Set; import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; @@ -50,6 +50,7 @@ import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.transport.ReceiveCommand; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -60,11 +61,14 @@ @RunWith(MockitoJUnitRunner.class) public class ProjectVersionRefUpdateTest implements RefFixture { - private static final String DEFAULT_INSTANCE_ID = "instance-id"; + private static final int A_TEST_ACCOUNT_ID = 1; + private static final GitReferenceUpdated.UpdatedRef A_TEST_UPDATED_REF = + new GitReferenceUpdated.UpdatedRef( + A_TEST_REF_NAME, ObjectId.zeroId(), ObjectId.zeroId(), ReceiveCommand.Type.UPDATE); @Rule public InMemoryTestEnvironment testEnvironment = new InMemoryTestEnvironment(); - @Mock RefUpdatedEvent refUpdatedEvent; + @Mock GitBatchRefUpdateListener.Event refUpdatedEvent; @Mock SharedRefDatabaseWrapper sharedRefDb; @Mock GitReferenceUpdated gitReferenceUpdated; @Mock ProjectVersionLogger verLogger; @@ -77,7 +81,6 @@ @Before public void setUp() throws Exception { - refUpdatedEvent.instanceId = DEFAULT_INSTANCE_ID; InMemoryRepository inMemoryRepo = repoManager.createRepository(A_TEST_PROJECT_NAME_KEY); project = projectConfigFactory.create(A_TEST_PROJECT_NAME_KEY); project.load(inMemoryRepo); @@ -94,12 +97,11 @@ .thenReturn(Optional.of("" + (masterCommit.getCommitTime() - 1))); when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) .thenReturn(true); - when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -116,19 +118,47 @@ } @Test + public void producerShouldUpdateProjectVersionOnceUponMultipleRefUpdatedEvent() { + when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) + .thenReturn(true); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()) + .thenReturn(Set.of(A_TEST_REF_NAME, A_REF_NAME_OF_A_PATCHSET)); + + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); + + verify(sharedRefDb, atMost(1)) + .compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)); + } + + @Test + public void producerShouldUpdateProjectVersionOnceUponMultipleRefWithOneMetaUpdatedEvent() { + when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) + .thenReturn(true); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()) + .thenReturn(Set.of(A_TEST_REF_NAME, MULTI_SITE_VERSIONING_REF)); + + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); + + verify(sharedRefDb, atMost(1)) + .compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)); + } + + @Test public void producerShouldUsePutInsteadOfCompareAndPutWhenExtendedGlobalRefDb() throws IOException { when(sharedRefDb.isSetOperationSupported()).thenReturn(true); - RefUpdatedEvent refUpdatedEvent = new RefUpdatedEvent(); - refUpdatedEvent.instanceId = DEFAULT_INSTANCE_ID; - RefUpdateAttribute refUpdatedAttribute = new RefUpdateAttribute(); - refUpdatedAttribute.project = A_TEST_PROJECT_NAME_KEY.get(); - refUpdatedAttribute.refName = A_TEST_REF_NAME; - refUpdatedEvent.refUpdate = Suppliers.memoize(() -> refUpdatedAttribute); + GitBatchRefUpdateListener.Event refUpdatedEvent = + new GitReferenceUpdated.GitBatchRefUpdateEvent( + A_TEST_PROJECT_NAME_KEY, + Set.of(A_TEST_UPDATED_REF), + new AccountInfo(A_TEST_ACCOUNT_ID)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -159,13 +189,12 @@ .thenReturn(Optional.of("" + (masterCommit.getCommitTime() - 1))); when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) .thenReturn(true); - when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); ProjectVersionRefUpdateImpl projectVersion = - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID); - projectVersion.onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger); + projectVersion.onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -196,12 +225,11 @@ when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) .thenReturn(true); - when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -224,28 +252,12 @@ } @Test - public void producerShouldNotUpdateProjectVersionUponForwardedRefUpdatedEvent() - throws IOException { - refUpdatedEvent.instanceId = "instance-id-2"; - - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); - - Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); - assertThat(ref).isNull(); - - verifyNoInteractions(verLogger); - } - - @Test public void shouldNotUpdateProjectVersionWhenProjectDoesntExist() throws IOException { - when(refUpdatedEvent.getProjectNameKey()).thenReturn(Project.nameKey("aNonExistentProject")); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn("aNonExistentProject"); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); assertThat(ref).isNull(); @@ -259,8 +271,7 @@ .thenReturn(Optional.of("123")); Optional<Long> version = - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) .getProjectRemoteVersion(A_TEST_PROJECT_NAME); assertThat(version.isPresent()).isTrue();