Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: 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 Change-Id: I623b58397d4ae9b9cc6c7038c992225eb33bc3b7
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 0d3939a..029dc83 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -649,32 +649,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.debug("Unknown remoteNameStyle: {}, 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.debug("Unknown remoteNameStyle: {}, 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 f910a40..66130f9 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 883cb23..2cbd627 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -103,6 +103,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() @@ -316,6 +317,7 @@ // MDC.put(ID_MDC_KEY, HexFormat.fromInt(id)); RunwayStatus status = pool.requestRunway(this); + isCollision = false; if (!status.isAllowed()) { if (status.isCanceled()) { repLog.info("PushOp for replication to {} was canceled and thus won't be rescheduled", uri); @@ -325,6 +327,7 @@ 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/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();