Allow pushBatchSize when cluster replication is enabled Pushing in batches was disabled when replication.distributionInterval is set because it produced more SEVERE "Error while deleting task" log entries from ReplicationTasksStorage. Those errors are now logged at FINE level when they are expected in a multi-primary setup, so there is no longer a reason to ignore pushBatchSize in that case. Change-Id: Iea59514cb3ee0c15290b095cc2460bb3062b0ee2
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java index 02389d6..cdbf1b1 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.plugins.replication; -import static com.google.common.base.Suppliers.memoize; import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog; import com.google.common.base.MoreObjects; @@ -23,7 +22,6 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; -import java.util.function.Supplier; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; import org.eclipse.jgit.lib.Config; @@ -56,7 +54,7 @@ private final RemoteConfig remoteConfig; private final int maxRetries; private final int slowLatencyThreshold; - private final Supplier<Integer> pushBatchSize; + private final int pushBatchSize; private final ImmutableList<Pattern> excludedRefsPattern; private final boolean storeRefLog; private final String uploadPack; @@ -106,29 +104,9 @@ TimeUnit.SECONDS); pushBatchSize = - memoize( - () -> { - int configuredBatchSize = - Math.max( - 0, - getInt( - remoteConfig, - cfg, - "pushBatchSize", - cfg.getInt("gerrit", "pushBatchSize", 0))); - if (configuredBatchSize > 0) { - int distributionInterval = cfg.getInt("replication", "distributionInterval", 0); - if (distributionInterval > 0) { - repLog.atWarning().log( - "Push in batches cannot be turned on for remote (%s) when 'Cluster" - + " Replication' (replication.distributionInterval) is configured", - name); - return 0; - } - return configuredBatchSize; - } - return 0; - }); + Math.max( + 0, + getInt(remoteConfig, cfg, "pushBatchSize", cfg.getInt("gerrit", "pushBatchSize", 0))); excludedRefsPattern = getExcludedRefsPattern(cfg, name); storeRefLog = cfg.getBoolean("remote", name, "storeRefLog", false); uploadPack = cfg.getString("remote", name, "uploadpack"); @@ -266,7 +244,7 @@ @Override public int getPushBatchSize() { - return pushBatchSize.get(); + return pushBatchSize; } @Override
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 014ca15..b63ae96 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -159,9 +159,6 @@ refs to be transferred in a single push operation. Note that negative values are treated as `0`. - Note that `pushBatchSize` is ignored when *Cluster Replication* is configured - - when `replication.distributionInterval` has value > 0. - gerrit.sshCommandTimeout : Timeout for SSH command execution. If 0, there is no timeout and the client waits indefinitely. By default, 0. @@ -724,9 +721,6 @@ be transferred in a single push operation). Note that negative values are treated as `0`. - Note that `pushBatchSize` is ignored when *Cluster Replication* is configured - - when `replication.distributionInterval` has value > 0. - remote.NAME.replicateNoteDbMetaRefs : Whether to replicate the NoteDb meta refs (`refs/changes/*/meta`, `refs/changes/*/robot-comments`, `refs/draft-comments/*`,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/DestinationConfigurationTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/DestinationConfigurationTest.java index e178ab1..5ae96a4 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/DestinationConfigurationTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/DestinationConfigurationTest.java
@@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.when; import org.eclipse.jgit.lib.Config; @@ -45,38 +46,41 @@ } @Test - public void shouldIgnoreRemotePushBatchSizeWhenClusterReplicationIsConfigured() { + public void shouldReturnRemotePushBatchSizeWhenClusterReplicationIsConfigured() { // given when(cfgMock.getInt("remote", REMOTE, "pushBatchSize", 0)).thenReturn(1); - when(cfgMock.getInt("replication", "distributionInterval", 0)).thenReturn(1); + lenient().when(cfgMock.getInt("replication", "distributionInterval", 0)).thenReturn(1); + objectUnderTest = new DestinationConfiguration(remoteConfigMock, cfgMock); // when int actual = objectUnderTest.getPushBatchSize(); // then - assertThat(actual).isEqualTo(0); + assertThat(actual).isEqualTo(1); } @Test - public void shouldIgnoreGlobalPushBatchSizeWhenClusterReplicationIsConfigured() { + public void shouldReturnGlobalPushBatchSizeWhenClusterReplicationIsConfigured() { // given int globalPushBatchSize = 1; when(cfgMock.getInt("gerrit", "pushBatchSize", 0)).thenReturn(globalPushBatchSize); when(cfgMock.getInt("remote", REMOTE, "pushBatchSize", globalPushBatchSize)) .thenReturn(globalPushBatchSize); - when(cfgMock.getInt("replication", "distributionInterval", 0)).thenReturn(1); + lenient().when(cfgMock.getInt("replication", "distributionInterval", 0)).thenReturn(1); + objectUnderTest = new DestinationConfiguration(remoteConfigMock, cfgMock); // when int actual = objectUnderTest.getPushBatchSize(); // then - assertThat(actual).isEqualTo(0); + assertThat(actual).isEqualTo(globalPushBatchSize); } @Test public void shouldReturnRemotePushBatchSizeWhenClusterReplicationIsNotConfigured() { // given when(cfgMock.getInt("remote", REMOTE, "pushBatchSize", 0)).thenReturn(1); + objectUnderTest = new DestinationConfiguration(remoteConfigMock, cfgMock); // when int actual = objectUnderTest.getPushBatchSize(); @@ -92,6 +96,7 @@ when(cfgMock.getInt("gerrit", "pushBatchSize", 0)).thenReturn(globalPushBatchSize); when(cfgMock.getInt("remote", REMOTE, "pushBatchSize", globalPushBatchSize)) .thenReturn(globalPushBatchSize); + objectUnderTest = new DestinationConfiguration(remoteConfigMock, cfgMock); // when int actual = objectUnderTest.getPushBatchSize();