Merge branch 'stable-3.1'
* stable-3.1:
Fix log statement formatting
GerritRestApi: Use UTF_8 from StandardCharsets instead of Guava
Fix NPE in PushResultProcessing
Fix start --wait to track in-flight collisions and to not fail
Improve URLmatching to match real Urls
Convert PushResultProcessing to an interface
ReplicationQueue: Revert back from Flogger to slf4j logging
The change "Revert back from Flogger to slf4j logging" is omitted from
this merge. Since change I0d18f8931 which was submitted earlier on the
master branch, Flogger logging is fixed.
Change-Id: Idb497116ee621b2fc2e984d1be679412c002fb48
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
index 7e0bff4..f485c95 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -320,6 +320,10 @@
"Error reading project %s from cache", project);
return false;
}
+ if (projectState == null) {
+ repLog.atFine().log("Project %s does not exist", project);
+ throw new NoSuchProjectException(project);
+ }
if (!projectState.statePermitsRead()) {
repLog.atFine().log("Project %s does not permit read", project);
return false;
@@ -661,33 +665,34 @@
List<URIish> getURIs(Project.NameKey project, String urlMatch) {
List<URIish> r = Lists.newArrayListWithCapacity(config.getRemoteConfig().getURIs().size());
- for (URIish uri : config.getRemoteConfig().getURIs()) {
- if (matches(uri, urlMatch)) {
- String name = project.get();
- if (needsUrlEncoding(uri)) {
- name = encode(name);
- }
- String remoteNameStyle = config.getRemoteNameStyle();
- if (remoteNameStyle.equals("dash")) {
- name = name.replace("/", "-");
- } else if (remoteNameStyle.equals("underscore")) {
- name = name.replace("/", "_");
- } else if (remoteNameStyle.equals("basenameOnly")) {
- name = FilenameUtils.getBaseName(name);
- } else if (!remoteNameStyle.equals("slash")) {
- repLog.atFine().log(
- "Unknown remoteNameStyle: %s, falling back to slash", remoteNameStyle);
- }
- String replacedPath = replaceName(uri.getPath(), name, config.isSingleProjectMatch());
- if (replacedPath != null) {
- uri = uri.setPath(replacedPath);
- r.add(uri);
- }
+ for (URIish configUri : config.getRemoteConfig().getURIs()) {
+ URIish uri = getURI(configUri, project);
+ if (matches(configUri, urlMatch) || matches(uri, urlMatch)) {
+ r.add(uri);
}
}
return r;
}
+ URIish getURI(URIish template, Project.NameKey project) {
+ String name = project.get();
+ if (needsUrlEncoding(template)) {
+ name = encode(name);
+ }
+ String remoteNameStyle = config.getRemoteNameStyle();
+ if (remoteNameStyle.equals("dash")) {
+ name = name.replace("/", "-");
+ } else if (remoteNameStyle.equals("underscore")) {
+ name = name.replace("/", "_");
+ } else if (remoteNameStyle.equals("basenameOnly")) {
+ name = FilenameUtils.getBaseName(name);
+ } else if (!remoteNameStyle.equals("slash")) {
+ repLog.atFine().log("Unknown remoteNameStyle: %s, falling back to slash", remoteNameStyle);
+ }
+ String replacedPath = replaceName(template.getPath(), name, isSingleProjectMatch());
+ return (replacedPath != null) ? template.setPath(replacedPath) : template;
+ }
+
static boolean needsUrlEncoding(URIish uri) {
return "http".equalsIgnoreCase(uri.getScheme())
|| "https".equalsIgnoreCase(uri.getScheme())
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java b/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
index 9e9da9a..fa81dd0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
@@ -16,8 +16,8 @@
import static com.googlesource.gerrit.plugins.replication.GerritSshApi.GERRIT_ADMIN_PROTOCOL_PREFIX;
import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
+import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.base.Charsets;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.restapi.Url;
import com.google.inject.Inject;
@@ -93,8 +93,7 @@
String url = String.format("%s/a/projects/%s/HEAD", toHttpUri(uri), Url.encode(project.get()));
try {
HttpPut req = new HttpPut(url);
- req.setEntity(
- new StringEntity(String.format("{\"ref\": \"%s\"}", newHead), Charsets.UTF_8.name()));
+ req.setEntity(new StringEntity(String.format("{\"ref\": \"%s\"}", newHead), UTF_8.name()));
req.addHeader(new BasicHeader("Content-Type", "application/json"));
httpClient.execute(req, new HttpResponseHandler(), getContext());
return true;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
index ff6306f..982e19e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -104,6 +104,7 @@
private final Set<String> delta = Sets.newHashSetWithExpectedSize(4);
private boolean pushAllRefs;
private Repository git;
+ private boolean isCollision;
private boolean retrying;
private int retryCount;
private final int maxRetries;
@@ -281,7 +282,7 @@
}
private void statesCleanUp() {
- if (!stateMap.isEmpty() && !isRetrying()) {
+ if (!stateMap.isEmpty() && !isRetrying() && !isCollision) {
for (Map.Entry<String, ReplicationState> entry : stateMap.entries()) {
entry
.getValue()
@@ -321,6 +322,7 @@
// created and scheduled for a future point in time.)
//
RunwayStatus status = pool.requestRunway(this);
+ isCollision = false;
if (!status.isAllowed()) {
if (status.isCanceled()) {
repLog.atInfo().log(
@@ -332,6 +334,7 @@
"Rescheduling replication to %s to avoid collision with the in-flight push [%s].",
uri, HexFormat.fromInt(status.getInFlightPushId()));
pool.reschedule(this, Destination.RetryReason.COLLISION);
+ isCollision = true;
}
return;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushResultProcessing.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushResultProcessing.java
index ad68d42..92ba4be 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushResultProcessing.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushResultProcessing.java
@@ -25,36 +25,54 @@
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.eclipse.jgit.transport.URIish;
-public abstract class PushResultProcessing {
+public interface PushResultProcessing {
+ public static final PushResultProcessing NO_OP = new PushResultProcessing() {};
- abstract void onRefReplicatedToOneNode(
+ /**
+ * Invoked when a ref has been replicated to one node.
+ *
+ * @param project
+ * @param ref
+ * @param uri
+ * @param status
+ * @param refStatus
+ */
+ default void onRefReplicatedToOneNode(
String project,
String ref,
URIish uri,
RefPushResult status,
- RemoteRefUpdate.Status refStatus);
+ RemoteRefUpdate.Status refStatus) {}
- abstract void onRefReplicatedToAllNodes(String project, String ref, int nodesCount);
+ /**
+ * Invoked when a ref has been replicated to all nodes.
+ *
+ * @param project
+ * @param ref
+ * @param nodesCount
+ */
+ default void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {}
- abstract void onAllRefsReplicatedToAllNodes(int totalPushTasksCount);
+ /**
+ * Invoked when all refs have been replicated to all nodes.
+ *
+ * @param totalPushTasksCount
+ */
+ default void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {}
/**
* Write message to standard out.
*
* @param message message text.
*/
- void writeStdOut(String message) {
- // Default doing nothing
- }
+ default void writeStdOut(String message) {}
/**
* Write message to standard error.
*
* @param message message text.
*/
- void writeStdErr(String message) {
- // Default doing nothing
- }
+ default void writeStdErr(String message) {}
static String resolveNodeName(URIish uri) {
StringBuilder sb = new StringBuilder();
@@ -70,7 +88,7 @@
return sb.toString();
}
- public static class CommandProcessing extends PushResultProcessing {
+ public static class CommandProcessing implements PushResultProcessing {
private WeakReference<StartCommand> sshCommand;
private AtomicBoolean hasError = new AtomicBoolean();
@@ -79,7 +97,7 @@
}
@Override
- void onRefReplicatedToOneNode(
+ public void onRefReplicatedToOneNode(
String project,
String ref,
URIish uri,
@@ -109,13 +127,13 @@
break;
}
sb.append(" (");
- sb.append(refStatus.toString());
+ sb.append(refStatus == null ? "unknown" : refStatus.toString());
sb.append(")");
writeStdOut(sb.toString());
}
@Override
- void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {
+ public void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {
StringBuilder sb = new StringBuilder();
sb.append("Replication of ");
sb.append(project);
@@ -128,7 +146,7 @@
}
@Override
- void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {
+ public void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {
if (totalPushTasksCount == 0) {
return;
}
@@ -141,7 +159,7 @@
}
@Override
- void writeStdOut(String message) {
+ public void writeStdOut(String message) {
StartCommand command = sshCommand.get();
if (command != null) {
command.writeStdOutSync(message);
@@ -149,7 +167,7 @@
}
@Override
- void writeStdErr(String message) {
+ public void writeStdErr(String message) {
StartCommand command = sshCommand.get();
if (command != null) {
command.writeStdErrSync(message);
@@ -157,7 +175,7 @@
}
}
- public static class GitUpdateProcessing extends PushResultProcessing {
+ public static class GitUpdateProcessing implements PushResultProcessing {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final EventDispatcher dispatcher;
@@ -167,7 +185,7 @@
}
@Override
- void onRefReplicatedToOneNode(
+ public void onRefReplicatedToOneNode(
String project,
String ref,
URIish uri,
@@ -177,13 +195,10 @@
}
@Override
- void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {
+ public void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {
postEvent(new RefReplicationDoneEvent(project, ref, nodesCount));
}
- @Override
- void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {}
-
private void postEvent(RefEvent event) {
try {
dispatcher.postEvent(event);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteSsh.java b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteSsh.java
index b8da2e4..f96c157 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteSsh.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteSsh.java
@@ -44,7 +44,7 @@
sshHelper.executeRemoteSsh(uri, cmd, errStream);
repLog.atInfo().log("Created remote repository: %s", uri);
} catch (IOException e) {
- repLog.atSevere().withCause(e).log(
+ repLog.atSevere().log(
"Error creating remote repository at %s:\n"
+ " Exception: %s\n"
+ " Command: %s\n"
@@ -64,8 +64,8 @@
sshHelper.executeRemoteSsh(uri, cmd, errStream);
repLog.atInfo().log("Deleted remote repository: %s", uri);
} catch (IOException e) {
- repLog.atSevere().withCause(e).log(
- "Error deleting remote repository at %s}:\n"
+ repLog.atSevere().log(
+ "Error deleting remote repository at %s:\n"
+ " Exception: %s\n"
+ " Command: %s\n"
+ " Output: %s",
@@ -84,7 +84,7 @@
try {
sshHelper.executeRemoteSsh(uri, cmd, errStream);
} catch (IOException e) {
- repLog.atSevere().withCause(e).log(
+ repLog.atSevere().log(
"Error updating HEAD of remote repository at %s to %s:\n"
+ " Exception: %s\n"
+ " Command: %s\n"
diff --git a/src/main/resources/Documentation/cmd-start.md b/src/main/resources/Documentation/cmd-start.md
index 6af73af..8291421 100644
--- a/src/main/resources/Documentation/cmd-start.md
+++ b/src/main/resources/Documentation/cmd-start.md
@@ -97,10 +97,10 @@
: Schedule replication for all projects.
`--url <PATTERN>`
-: Replicate only to replication destinations whose URL contains
- the substring `PATTERN`. This can be useful to replicate
- only to a previously down node, which has been brought back
- online.
+: Replicate only to replication destinations whose configuration
+ URL contains the substring `PATTERN`, or whose expanded project
+ URL contains `PATTERN`. This can be useful to replicate only to
+ a previously down node, which has been brought back online.
EXAMPLES
--------
@@ -136,6 +136,12 @@
$ ssh -p @SSH_PORT@ @SSH_HOST@ @PLUGIN@ start --url slave1 ^(|.*/)vendor(|/.*)
```
+Replicate to only one specific destination URL:
+
+```
+ $ ssh -p @SSH_PORT@ @SSH_HOST@ @PLUGIN@ start --url https://example.com/tools/gerrit.git
+```
+
SEE ALSO
--------
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
index 9cf5489..19948db 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static com.googlesource.gerrit.plugins.replication.PushResultProcessing.NO_OP;
import static java.util.stream.Collectors.toList;
import com.google.common.flogger.FluentLogger;
@@ -47,8 +48,6 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.storage.file.FileBasedConfig;
-import org.eclipse.jgit.transport.RemoteRefUpdate;
-import org.eclipse.jgit.transport.URIish;
import org.eclipse.jgit.util.FS;
import org.junit.Test;
@@ -206,25 +205,7 @@
@Test
public void shouldCreateOneReplicationTaskWhenSchedulingRepoFullSync() throws Exception {
- PushResultProcessing pushResultProcessing =
- new PushResultProcessing() {
-
- @Override
- void onRefReplicatedToOneNode(
- String project,
- String ref,
- URIish uri,
- ReplicationState.RefPushResult status,
- RemoteRefUpdate.Status refStatus) {}
-
- @Override
- void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {}
-
- @Override
- void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {}
- };
-
- createTestProject("replica");
+ createTestProject(project + "replica");
setReplicationDestination("foo", "replica", ALL_PROJECTS);
reloadConfig();
@@ -232,12 +213,55 @@
plugin
.getSysInjector()
.getInstance(ReplicationQueue.class)
- .scheduleFullSync(project, null, new ReplicationState(pushResultProcessing), true);
+ .scheduleFullSync(project, null, new ReplicationState(NO_OP), true);
assertThat(listReplicationTasks(".*all.*")).hasSize(1);
}
@Test
+ public void shouldMatchTemplatedURL() throws Exception {
+ createTestProject(project + "replica");
+
+ setReplicationDestination("foo", "replica", ALL_PROJECTS);
+ reloadConfig();
+
+ String urlMatch = gitPath.resolve("${name}" + "replica" + ".git").toString();
+ String expectedURI = gitPath.resolve(project + "replica" + ".git").toString();
+
+ plugin
+ .getSysInjector()
+ .getInstance(ReplicationQueue.class)
+ .scheduleFullSync(project, urlMatch, new ReplicationState(NO_OP), true);
+
+ assertThat(listReplicationTasks(".*all.*")).hasSize(1);
+ for (ReplicationTasksStorage.ReplicateRefUpdate task : tasksStorage.list()) {
+ assertThat(task.uri).isEqualTo(expectedURI);
+ }
+ }
+
+ @Test
+ public void shouldMatchRealURL() throws Exception {
+ createTestProject(project + "replica");
+
+ setReplicationDestination("foo", "replica", ALL_PROJECTS);
+ reloadConfig();
+
+ String urlMatch = gitPath.resolve(project + "replica" + ".git").toString();
+ String expectedURI = urlMatch;
+
+ plugin
+ .getSysInjector()
+ .getInstance(ReplicationQueue.class)
+ .scheduleFullSync(project, urlMatch, new ReplicationState(NO_OP), true);
+
+ assertThat(listReplicationTasks(".*")).hasSize(1);
+ for (ReplicationTasksStorage.ReplicateRefUpdate task : tasksStorage.list()) {
+ assertThat(task.uri).isEqualTo(expectedURI);
+ }
+ assertThat(tasksStorage.list()).isNotEmpty();
+ }
+
+ @Test
public void shouldReplicateHeadUpdate() throws Exception {
setReplicationDestination("foo", "replica", ALL_PROJECTS);
reloadConfig();
