Merge "Make sure project is indexed after creation" into stable-3.4
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 1fc3613..43db907 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
@@ -35,8 +35,8 @@
import com.googlesource.gerrit.plugins.replication.pull.api.PullReplicationApiRequestMetrics;
import com.googlesource.gerrit.plugins.replication.pull.fetch.Fetch;
import com.googlesource.gerrit.plugins.replication.pull.fetch.FetchFactory;
+import com.googlesource.gerrit.plugins.replication.pull.fetch.PermanentTransportException;
import com.googlesource.gerrit.plugins.replication.pull.fetch.RefUpdateState;
-import com.jcraft.jsch.JSchException;
import java.io.IOException;
import java.util.Collection;
import java.util.HashSet;
@@ -338,11 +338,11 @@
"Cannot replicate [{}] {}; Remote repository error: {}", taskIdHex, projectName, msg);
} catch (NotSupportedException e) {
stateLog.error("Cannot replicate from " + uri, e, getStatesAsArray());
+ } catch (PermanentTransportException e) {
+ repLog.error(
+ String.format("Terminal failure. Cannot replicate [%s] from %s", taskIdHex, uri), e);
} catch (TransportException e) {
- Throwable cause = e.getCause();
- if (cause instanceof JSchException && cause.getMessage().startsWith("UnknownHostKey:")) {
- repLog.error("Cannot replicate [{}] from {}: {}", taskIdHex, uri, cause.getMessage());
- } else if (e instanceof LockFailureException) {
+ if (e instanceof LockFailureException) {
lockRetryCount++;
// The LockFailureException message contains both URI and reason
// for this failure.
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 89972cf..74ad9fc 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
@@ -21,6 +21,7 @@
import java.io.IOException;
import java.util.List;
import java.util.stream.Collectors;
+import org.eclipse.jgit.errors.TransportException;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.*;
@@ -51,6 +52,13 @@
private FetchResult fetchVia(Transport tn, List<RefSpec> fetchRefSpecs) throws IOException {
repLog.info("Fetch references {} from {}", fetchRefSpecs, uri);
- return tn.fetch(NullProgressMonitor.INSTANCE, fetchRefSpecs);
+ try {
+ return tn.fetch(NullProgressMonitor.INSTANCE, fetchRefSpecs);
+ } catch (TransportException e) {
+ if (PermanentTransportException.isPermanentFailure(e)) {
+ throw new PermanentTransportException("Terminal fetch failure", e);
+ }
+ throw e;
+ }
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/PermanentTransportException.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/PermanentTransportException.java
new file mode 100644
index 0000000..1d96a02
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/PermanentTransportException.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2023 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.jcraft.jsch.JSchException;
+import org.eclipse.jgit.errors.TransportException;
+import org.eclipse.jgit.internal.JGitText;
+
+public class PermanentTransportException extends TransportException {
+ private static final long serialVersionUID = 1L;
+
+ public PermanentTransportException(String msg, Throwable cause) {
+ super(msg, cause);
+ }
+
+ public static boolean isPermanentFailure(TransportException e) {
+ Throwable cause = e.getCause();
+ String message = e.getMessage();
+ return (cause instanceof JSchException && cause.getMessage().startsWith("UnknownHostKey:"))
+ || message.matches(JGitText.get().remoteDoesNotHaveSpec.replaceAll("\\{0\\}", ".+"));
+ }
+}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index eb15c02..b76585e 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -119,6 +119,18 @@
By default, fetches are retried indefinitely.
+ Note that only transient errors will be retried, whilst persistent errors will
+ cause a terminal failure, and the fetch will not be scheduled again. This is
+ only supported for JGit, not cGit. Currently, only the following failures are
+ considered permanent:
+
+ - UnknownHostKey: thrown by Jsch when establishing an SSH connection for an
+ unknown host.
+ - Jgit transport exception when the remote ref does not exist. The assumption
+ here is that the remote ref does not exist so it is not worth retrying. If the
+ exception arisen as a consequence of some ACLs (mis)configuration, then after
+ fixing the ACLs, an explicit replication must be manually triggered.
+
replication.instanceLabel
: Remote configuration name of the current server.
This label is passed as a part of the payload to notify other
@@ -380,6 +392,9 @@
By default, use replication.maxRetries.
+ Note that not all fetch failures are retriable. Please refer
+ to `replication.maxRetries` for more information on this.
+
remote.NAME.threads
: Number of worker threads to dedicate to fetching to the
repositories described by this remote. Each thread can fetch
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 406cd8c..3f40848 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
@@ -22,18 +22,12 @@
import static org.mockito.Mockito.when;
import com.google.common.collect.Lists;
-import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.acceptance.SkipProjectClone;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
-import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.config.FactoryModule;
-import com.google.gerrit.server.config.SitePaths;
-import com.google.inject.Inject;
import com.google.inject.Scopes;
import com.google.inject.assistedinject.FactoryModuleBuilder;
import com.googlesource.gerrit.plugins.replication.AutoReloadSecureCredentialsFactoryDecorator;
@@ -46,12 +40,8 @@
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;
@@ -68,27 +58,8 @@
@TestPlugin(
name = "pull-replication",
sysModule = "com.googlesource.gerrit.plugins.replication.pull.CGitFetchIT$TestModule")
-public class CGitFetchIT extends LightweightPluginDaemonTest {
+public class CGitFetchIT extends FetchITBase {
private static final String TEST_REPLICATION_SUFFIX = "suffix1";
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
- private static final int TEST_REPLICATION_DELAY = 60;
- private static final Duration TEST_TIMEOUT = Duration.ofSeconds(TEST_REPLICATION_DELAY * 2);
-
- @Inject private SitePaths sitePaths;
- @Inject private ProjectOperations projectOperations;
- private FetchFactory fetchFactory;
- private Path gitPath;
- private Path testRepoPath;
-
- @Override
- public void setUpTestPlugin() throws Exception {
- gitPath = sitePaths.site_path.resolve("git");
- testRepoPath = gitPath.resolve(project + TEST_REPLICATION_SUFFIX + ".git");
-
- super.setUpTestPlugin();
- fetchFactory = plugin.getSysInjector().getInstance(FetchFactory.class);
- }
@Test
public void shouldFetchRef() throws Exception {
@@ -245,27 +216,6 @@
}
}
- private void waitUntil(Supplier<Boolean> waitCondition) throws InterruptedException {
- WaitUtil.waitUntil(waitCondition, TEST_TIMEOUT);
- }
-
- private Ref getRef(Repository repo, String branchName) throws IOException {
- return repo.getRefDatabase().exactRef(branchName);
- }
-
- private Ref checkedGetRef(Repository repo, String branchName) {
- try {
- return repo.getRefDatabase().exactRef(branchName);
- } catch (Exception e) {
- logger.atSevere().withCause(e).log("failed to get ref %s in repo %s", branchName, repo);
- return null;
- }
- }
-
- private Project.NameKey createTestProject(String name) throws Exception {
- return projectOperations.newProject().name(name).create();
- }
-
@SuppressWarnings("unused")
private static class TestModule extends FactoryModule {
@Override
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchITBase.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchITBase.java
new file mode 100644
index 0000000..2a11717
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchITBase.java
@@ -0,0 +1,73 @@
+// Copyright (C) 2023 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;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.replication.pull.fetch.FetchFactory;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.function.Supplier;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+
+public abstract class FetchITBase extends LightweightPluginDaemonTest {
+ private static final String TEST_REPLICATION_SUFFIX = "suffix1";
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private static final int TEST_REPLICATION_DELAY = 60;
+ private static final Duration TEST_TIMEOUT = Duration.ofSeconds(TEST_REPLICATION_DELAY * 2);
+
+ @Inject private SitePaths sitePaths;
+ @Inject private ProjectOperations projectOperations;
+ FetchFactory fetchFactory;
+ private Path gitPath;
+ Path testRepoPath;
+
+ @Override
+ public void setUpTestPlugin() throws Exception {
+ gitPath = sitePaths.site_path.resolve("git");
+ testRepoPath = gitPath.resolve(project + TEST_REPLICATION_SUFFIX + ".git");
+
+ super.setUpTestPlugin();
+ fetchFactory = plugin.getSysInjector().getInstance(FetchFactory.class);
+ }
+
+ void waitUntil(Supplier<Boolean> waitCondition) throws InterruptedException {
+ WaitUtil.waitUntil(waitCondition, TEST_TIMEOUT);
+ }
+
+ Ref getRef(Repository repo, String branchName) throws IOException {
+ return repo.getRefDatabase().exactRef(branchName);
+ }
+
+ Ref checkedGetRef(Repository repo, String branchName) {
+ try {
+ return repo.getRefDatabase().exactRef(branchName);
+ } catch (Exception e) {
+ logger.atSevere().withCause(e).log("failed to get ref %s in repo %s", branchName, repo);
+ return null;
+ }
+ }
+
+ Project.NameKey createTestProject(String name) throws Exception {
+ return projectOperations.newProject().name(name).create();
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/JGitFetchIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/JGitFetchIT.java
new file mode 100644
index 0000000..ec695a6
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/JGitFetchIT.java
@@ -0,0 +1,85 @@
+// Copyright (C) 2023 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;
+
+import com.google.common.collect.Lists;
+import com.google.gerrit.acceptance.SkipProjectClone;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.inject.Scopes;
+import com.google.inject.assistedinject.FactoryModuleBuilder;
+import com.googlesource.gerrit.plugins.replication.AutoReloadSecureCredentialsFactoryDecorator;
+import com.googlesource.gerrit.plugins.replication.CredentialsFactory;
+import com.googlesource.gerrit.plugins.replication.ReplicationConfig;
+import com.googlesource.gerrit.plugins.replication.ReplicationFileBasedConfig;
+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 com.googlesource.gerrit.plugins.replication.pull.fetch.PermanentTransportException;
+import java.net.URISyntaxException;
+import org.eclipse.jgit.lib.Config;
+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.junit.Test;
+
+@SkipProjectClone
+@UseLocalDisk
+@TestPlugin(
+ name = "pull-replication",
+ sysModule = "com.googlesource.gerrit.plugins.replication.pull.JGitFetchIT$TestModule")
+public class JGitFetchIT extends FetchITBase {
+ private static final String TEST_REPLICATION_SUFFIX = "suffix1";
+
+ @Test(expected = PermanentTransportException.class)
+ public void shouldThrowPermanentTransportExceptionWhenRefDoesNotExists() throws Exception {
+
+ testRepo = cloneProject(createTestProject(project + TEST_REPLICATION_SUFFIX));
+ String nonExistingRef = "refs/changes/02/20000/1:refs/changes/02/20000/1";
+ try (Repository repo = repoManager.openRepository(project)) {
+ Fetch objectUnderTest = fetchFactory.create(new URIish(testRepoPath.toString()), repo);
+ objectUnderTest.fetch(Lists.newArrayList(new RefSpec(nonExistingRef)));
+ }
+ }
+
+ @SuppressWarnings("unused")
+ private static class TestModule extends FactoryModule {
+ @Override
+ protected void configure() {
+ Config cf = new Config();
+ cf.setInt("remote", "test_config", "timeout", 0);
+ try {
+ RemoteConfig remoteConfig = new RemoteConfig(cf, "test_config");
+ SourceConfiguration sourceConfig = new SourceConfiguration(remoteConfig, cf);
+ bind(ReplicationConfig.class).to(ReplicationFileBasedConfig.class);
+ bind(CredentialsFactory.class)
+ .to(AutoReloadSecureCredentialsFactoryDecorator.class)
+ .in(Scopes.SINGLETON);
+
+ bind(SourceConfiguration.class).toInstance(sourceConfig);
+ install(
+ new FactoryModuleBuilder()
+ .implement(Fetch.class, JGitFetch.class)
+ .implement(Fetch.class, FetchClientImplementation.class, JGitFetch.class)
+ .build(FetchFactory.class));
+ } catch (URISyntaxException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PermanentFailureExceptionTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PermanentFailureExceptionTest.java
new file mode 100644
index 0000000..fb4fb04
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PermanentFailureExceptionTest.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2023 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;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.googlesource.gerrit.plugins.replication.pull.fetch.PermanentTransportException;
+import com.jcraft.jsch.JSchException;
+import org.eclipse.jgit.errors.TransportException;
+import org.junit.Test;
+
+public class PermanentFailureExceptionTest {
+
+ @Test
+ public void shouldConsiderSchUnknownHostAsPermanent() {
+ assertThat(
+ PermanentTransportException.isPermanentFailure(
+ new TransportException(
+ "SSH error", new JSchException("UnknownHostKey: some.place"))))
+ .isTrue();
+ }
+
+ @Test
+ public void shouldConsiderNotExistingRefsAsPermanent() {
+ assertThat(
+ PermanentTransportException.isPermanentFailure(
+ new TransportException("Remote does not have refs/heads/foo available for fetch.")))
+ .isTrue();
+ }
+}
