Execute refs fetch in batches
If number of refs to fetch is greater then
replication.refsBatchSize, refs are going to be split into
a separate fetch operations.
Feature: Issue 11605
Change-Id: If5f192a95bf5f0945b503f66dcdc012ce71539eb
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java
index bf776e6..d4e22d9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java
@@ -98,7 +98,7 @@
FetchOne(
GitRepositoryManager grm,
Source s,
- RemoteConfig c,
+ SourceConfiguration c,
PerThreadRequestScope.Scoper ts,
IdGenerator ig,
ReplicationStateListeners sl,
@@ -108,7 +108,7 @@
@Assisted URIish u) {
gitManager = grm;
pool = s;
- config = c;
+ config = c.getRemoteConfig();
threadScoper = ts;
projectName = d;
uri = u;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/Source.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/Source.java
index 1daf92b..e9bb620 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/Source.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/Source.java
@@ -59,9 +59,11 @@
import com.google.inject.servlet.RequestScoped;
import com.googlesource.gerrit.plugins.replication.RemoteSiteUser;
import com.googlesource.gerrit.plugins.replication.ReplicationFilter;
+import com.googlesource.gerrit.plugins.replication.pull.fetch.BatchFetchClient;
import com.googlesource.gerrit.plugins.replication.pull.fetch.CGitFetch;
import com.googlesource.gerrit.plugins.replication.pull.fetch.CGitFetchValidator;
import com.googlesource.gerrit.plugins.replication.pull.fetch.Fetch;
+import com.googlesource.gerrit.plugins.replication.pull.fetch.FetchClientImplementation;
import com.googlesource.gerrit.plugins.replication.pull.fetch.FetchFactory;
import com.googlesource.gerrit.plugins.replication.pull.fetch.JGitFetch;
import java.io.IOException;
@@ -85,7 +87,6 @@
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.RefSpec;
-import org.eclipse.jgit.transport.RemoteConfig;
import org.eclipse.jgit.transport.URIish;
import org.slf4j.Logger;
@@ -175,13 +176,14 @@
bind(PerThreadRequestScope.Propagator.class);
bind(Source.class).toInstance(Source.this);
- bind(RemoteConfig.class).toInstance(config.getRemoteConfig());
+ bind(SourceConfiguration.class).toInstance(config);
install(new FactoryModuleBuilder().build(FetchOne.Factory.class));
Class<? extends Fetch> clientClass =
cfg.useCGitClient() ? CGitFetch.class : JGitFetch.class;
install(
new FactoryModuleBuilder()
- .implement(Fetch.class, clientClass)
+ .implement(Fetch.class, BatchFetchClient.class)
+ .implement(Fetch.class, FetchClientImplementation.class, clientClass)
.build(FetchFactory.class));
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfiguration.java
index 01d46b1..e50461d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfiguration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfiguration.java
@@ -51,6 +51,7 @@
private final int maxRetries;
private int slowLatencyThreshold;
private boolean useCGitClient;
+ private int refsBatchSize;
public SourceConfiguration(RemoteConfig remoteConfig, Config cfg) {
this.remoteConfig = remoteConfig;
@@ -76,6 +77,9 @@
replicatePermissions = cfg.getBoolean("remote", name, "replicatePermissions", true);
replicateHiddenProjects = cfg.getBoolean("remote", name, "replicateHiddenProjects", false);
useCGitClient = cfg.getBoolean("replication", "useCGitClient", false);
+ refsBatchSize = cfg.getInt("replication", "refsBatchSize", 50);
+ if (refsBatchSize <= 0)
+ throw new IllegalArgumentException("refsBatchSize must be greater than zero");
remoteNameStyle =
MoreObjects.firstNonNull(cfg.getString("remote", name, "remoteNameStyle"), "slash");
maxRetries =
@@ -173,6 +177,10 @@
return useCGitClient;
}
+ public int getRefsBatchSize() {
+ return refsBatchSize;
+ }
+
@Override
public RemoteConfig getRemoteConfig() {
return remoteConfig;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/BatchFetchClient.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/BatchFetchClient.java
new file mode 100644
index 0000000..f2fbcd1
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/BatchFetchClient.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.replication.pull.fetch;
+
+import com.google.common.collect.Lists;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.googlesource.gerrit.plugins.replication.pull.SourceConfiguration;
+import java.io.IOException;
+import java.util.List;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.transport.URIish;
+
+public class BatchFetchClient implements Fetch {
+ private int batchSize;
+ private Fetch fetchClient;
+
+ @Inject
+ public BatchFetchClient(
+ SourceConfiguration config,
+ FetchFactory factory,
+ @Assisted URIish uri,
+ @Assisted Repository git) {
+ this.batchSize = config.getRefsBatchSize();
+ this.fetchClient = factory.createPlainImpl(uri, git);
+ }
+
+ @Override
+ public List<RefUpdateState> fetch(List<RefSpec> refs) throws IOException {
+ List<RefUpdateState> results = Lists.newArrayList();
+ for (List<RefSpec> refsBatch : Lists.partition(refs, batchSize)) {
+ results.addAll(fetchClient.fetch(refsBatch));
+ }
+ return results;
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/CGitFetch.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/CGitFetch.java
index 4f8abd6..9f055c8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/CGitFetch.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/CGitFetch.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Lists;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import com.googlesource.gerrit.plugins.replication.pull.SourceConfiguration;
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
@@ -30,7 +31,6 @@
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.RefSpec;
-import org.eclipse.jgit.transport.RemoteConfig;
import org.eclipse.jgit.transport.URIish;
public class CGitFetch implements Fetch {
@@ -40,11 +40,11 @@
private int timeout;
@Inject
- public CGitFetch(RemoteConfig config, @Assisted URIish uri, @Assisted Repository git) {
+ public CGitFetch(SourceConfiguration config, @Assisted URIish uri, @Assisted Repository git) {
this.localProjectDirectory = git.getDirectory();
this.uri = uri;
- this.timeout = config.getTimeout();
+ this.timeout = config.getRemoteConfig().getTimeout();
}
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/FetchClientImplementation.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/FetchClientImplementation.java
new file mode 100644
index 0000000..77317b7
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/FetchClientImplementation.java
@@ -0,0 +1,23 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.replication.pull.fetch;
+
+import com.google.inject.BindingAnnotation;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@BindingAnnotation
+@Retention(RetentionPolicy.RUNTIME)
+public @interface FetchClientImplementation {}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/FetchFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/FetchFactory.java
index 3d65594..d356477 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/FetchFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/FetchFactory.java
@@ -19,4 +19,7 @@
public interface FetchFactory {
Fetch create(URIish uri, Repository git);
+ // Return implementation without any decorators
+ @FetchClientImplementation
+ Fetch createPlainImpl(URIish uri, Repository git);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/JGitFetch.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/JGitFetch.java
index 4467752..5de82f1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/JGitFetch.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/JGitFetch.java
@@ -19,6 +19,7 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.googlesource.gerrit.plugins.replication.CredentialsFactory;
+import com.googlesource.gerrit.plugins.replication.pull.SourceConfiguration;
import java.io.IOException;
import java.util.List;
import java.util.stream.Collectors;
@@ -39,11 +40,11 @@
@Inject
public JGitFetch(
- RemoteConfig config,
+ SourceConfiguration sourceConfig,
CredentialsFactory cpFactory,
@Assisted URIish uri,
@Assisted Repository git) {
- this.config = config;
+ this.config = sourceConfig.getRemoteConfig();
this.credentialsProvider = cpFactory.create(config.getName());
this.uri = uri;
this.git = git;
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index cc0c0ee..c5d13a0 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -138,6 +138,15 @@
Default: false
+replication.refsBatchSize
+: Number of refs that are fetched in a single fetch call.
+ If number of refs to fetch is greater then this param,
+ refs are going to be split into a separate fetch operations.
+
+ Value must be greater than zero.
+
+ Default: 50
+
remote.NAME.url
: Address of the remote server to fetch from. Single URL can be
specified within a single remote block. A remote node can request
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/CGitFetchIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/CGitFetchIT.java
index 8134ba4..a02f43f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/CGitFetchIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/CGitFetchIT.java
@@ -15,6 +15,11 @@
package com.googlesource.gerrit.plugins.replication.pull;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import com.google.common.collect.Lists;
import com.google.common.flogger.FluentLogger;
@@ -30,17 +35,22 @@
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import com.google.inject.assistedinject.FactoryModuleBuilder;
+import com.googlesource.gerrit.plugins.replication.pull.fetch.BatchFetchClient;
import com.googlesource.gerrit.plugins.replication.pull.fetch.CGitFetch;
import com.googlesource.gerrit.plugins.replication.pull.fetch.Fetch;
+import com.googlesource.gerrit.plugins.replication.pull.fetch.FetchClientImplementation;
import com.googlesource.gerrit.plugins.replication.pull.fetch.FetchFactory;
+import com.googlesource.gerrit.plugins.replication.pull.fetch.RefUpdateState;
import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.time.Duration;
+import java.util.List;
import java.util.function.Supplier;
import org.eclipse.jgit.errors.TransportException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.RefSpec;
@@ -117,7 +127,6 @@
try (Repository repo = repoManager.openRepository(project)) {
Result pushResult = createChange();
- RevCommit sourceCommit = pushResult.getCommit();
String sourceRef = pushResult.getPatchSet().refName();
Fetch objectUnderTest = fetchFactory.create(new URIish("/not_existing_path/"), repo);
@@ -160,6 +169,34 @@
}
@Test
+ public void shouldFetchMultipleRefsInMultipleBatches() throws Exception {
+ Config cf = new Config();
+ cf.setInt("remote", "test_config", "timeout", 0);
+ cf.setInt("replication", null, "refsBatchSize", 2);
+ URIish uri = new URIish(testRepoPath.toString());
+ List<RefUpdateState> fetchResultList =
+ Lists.newArrayList(new RefUpdateState("test_config", RefUpdate.Result.NEW));
+ RemoteConfig remoteConfig = new RemoteConfig(cf, "test_config");
+ SourceConfiguration sourceConfig = new SourceConfiguration(remoteConfig, cf);
+
+ Repository repo = mock(Repository.class);
+ FetchFactory fetchFactory = mock(FetchFactory.class);
+ Fetch fetchClient = mock(Fetch.class);
+ when(fetchFactory.createPlainImpl(uri, repo)).thenReturn(fetchClient);
+ when(fetchClient.fetch(any())).thenReturn(fetchResultList);
+
+ Fetch objectUnderTest =
+ new BatchFetchClient(sourceConfig, fetchFactory, new URIish(testRepoPath.toString()), repo);
+
+ objectUnderTest.fetch(
+ Lists.newArrayList(
+ new RefSpec("refs/changes/01/1/1:refs/changes/01/1/1"),
+ new RefSpec("refs/changes/02/2/1:refs/changes/02/2/1"),
+ new RefSpec("refs/changes/03/3/1:refs/changes/03/3/1")));
+ verify(fetchClient, times(2)).fetch(any());
+ }
+
+ @Test
public void shouldFetchNewBranch() throws Exception {
String testProjectName = project + TEST_REPLICATION_SUFFIX;
createTestProject(testProjectName);
@@ -232,10 +269,13 @@
cf.setInt("remote", "test_config", "timeout", 0);
try {
RemoteConfig remoteConfig = new RemoteConfig(cf, "test_config");
- bind(RemoteConfig.class).toInstance(remoteConfig);
+ SourceConfiguration sourceConfig = new SourceConfiguration(remoteConfig, cf);
+
+ bind(SourceConfiguration.class).toInstance(sourceConfig);
install(
new FactoryModuleBuilder()
.implement(Fetch.class, CGitFetch.class)
+ .implement(Fetch.class, FetchClientImplementation.class, CGitFetch.class)
.build(FetchFactory.class));
} catch (URISyntaxException e) {
throw new RuntimeException(e);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchReplicationTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchReplicationTest.java
index c1c0981..3257fda 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchReplicationTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchReplicationTest.java
@@ -15,10 +15,13 @@
package com.googlesource.gerrit.plugins.replication.pull;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.googlesource.gerrit.plugins.replication.pull.Source.encode;
import static com.googlesource.gerrit.plugins.replication.pull.Source.needsUrlEncoding;
import java.net.URISyntaxException;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.transport.RemoteConfig;
import org.eclipse.jgit.transport.URIish;
import org.junit.Test;
@@ -43,4 +46,14 @@
assertThat(encode("name/with a space")).isEqualTo("name/with%20a%20space");
assertThat(encode("name\nwith-LF")).isEqualTo("name%0Awith-LF");
}
+
+ @Test
+ public void testRefsBatchSizeMustBeGreaterThanZero() throws URISyntaxException {
+ Config cf = new Config();
+ cf.setInt("remote", "test_config", "timeout", 0);
+ cf.setInt("replication", null, "refsBatchSize", 0);
+ RemoteConfig remoteConfig = new RemoteConfig(cf, "test_config");
+
+ assertThrows(IllegalArgumentException.class, () -> new SourceConfiguration(remoteConfig, cf));
+ }
}
