Add a remote config to exclude replicating desired refs Allow admins to exclude specific refs from replication when they are not needed on replicas. For instance, refs created by certain plugins can now be skipped. This config helps improve replication performance and stability, especially on busy servers that create a large number of such unwanted refs. Change-Id: I42366148cfb1f9da35904ad8e07d3fc103966a53
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 c186493..ceaf073 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -83,6 +83,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.function.Function; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Ref; @@ -835,6 +836,10 @@ return config.replicateNoteDbMetaRefs(); } + ImmutableList<Pattern> excludedRefsPattern() { + return config.excludedRefsPattern(); + } + private static boolean matches(URIish uri, String urlMatch) { if (urlMatch == null || urlMatch.equals("") || urlMatch.equals("*")) { return true;
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 4d72ff0..41ab4c4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java
@@ -20,8 +20,12 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; import com.google.gerrit.server.config.ConfigUtil; +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; import org.eclipse.jgit.transport.RemoteConfig; @@ -51,6 +55,7 @@ private final int maxRetries; private final int slowLatencyThreshold; private final Supplier<Integer> pushBatchSize; + private final ImmutableList<Pattern> excludedRefsPattern; protected DestinationConfiguration(RemoteConfig remoteConfig, Config cfg) { this.remoteConfig = remoteConfig; @@ -116,6 +121,7 @@ } return 0; }); + excludedRefsPattern = getExcludedRefsPattern(cfg, name); } @Override @@ -215,4 +221,21 @@ public int getPushBatchSize() { return pushBatchSize.get(); } + + @Override + public ImmutableList<Pattern> excludedRefsPattern() { + return excludedRefsPattern; + } + + private ImmutableList<Pattern> getExcludedRefsPattern(Config cfg, String name) { + List<Pattern> patterns = new ArrayList<>(); + for (String regex : cfg.getStringList("remote", name, "excludedRefsPattern")) { + try { + patterns.add(Pattern.compile(regex)); + } catch (PatternSyntaxException e) { + repLog.atWarning().log("Invalid excludedRefsPattern '%s' is ignored", regex); + } + } + return ImmutableList.copyOf(patterns); + } }
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 2b60145..b516fc7 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -755,7 +755,8 @@ private boolean canPushRef(String ref, boolean noPerms) { return !(noPerms && RefNames.REFS_CONFIG.equals(ref)) && !ref.startsWith(RefNames.REFS_CACHE_AUTOMERGE) - && !(!pool.replicateNoteDbMetaRefs() && RefNames.isNoteDbMetaRef(ref)); + && !(!pool.replicateNoteDbMetaRefs() && RefNames.isNoteDbMetaRef(ref)) + && pool.excludedRefsPattern().stream().noneMatch(p -> p.matcher(ref).matches()); } private Map<String, Ref> listRemote(Transport tn)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java index e39afd6..307017b 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java
@@ -14,6 +14,7 @@ package com.googlesource.gerrit.plugins.replication; import com.google.common.collect.ImmutableList; +import java.util.regex.Pattern; import org.eclipse.jgit.transport.RemoteConfig; /** Remote configuration for a replication endpoint */ @@ -142,4 +143,11 @@ } return ret; } + + /** + * List of patterns that will be used to exclude refs from being replicated + * + * @return list of successfully compiled patterns + */ + ImmutableList<Pattern> excludedRefsPattern(); }
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 6f9a49d..197c0d0 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -596,6 +596,11 @@ By default, true. +remote.NAME.excludedRefsPattern +: Refs that match the pattern provided using this config will not be replicated. + This option is useful when admins want to skip replicating certain refs, for + example refs created by plugins. Multiple excludedRefsPattern keys can be + supplied, to specify multiple patterns to match against. Directory `replication` --------------------
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java index aa29846..3b07f49 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java
@@ -53,6 +53,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; import org.eclipse.jgit.errors.NotSupportedException; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.errors.TransportException; @@ -75,6 +76,7 @@ import org.eclipse.jgit.util.FS; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -294,6 +296,40 @@ } @Test + public void skipPushingExcludedRefs() throws InterruptedException, IOException { + when(destinationMock.excludedRefsPattern()) + .thenReturn( + ImmutableList.of(Pattern.compile("refs/foo/.*"), Pattern.compile("refs/bar/.*"))); + PushOne pushOne = Mockito.spy(createPushOne(null)); + + Ref ref1 = + new ObjectIdRef.Unpeeled( + NEW, + "refs/heads/master", + ObjectId.fromString("0000000000000000000000000000000000000002")); + Ref ref2 = + new ObjectIdRef.Unpeeled( + NEW, "refs/foo/test", ObjectId.fromString("0000000000000000000000000000000000000003")); + Ref ref3 = + new ObjectIdRef.Unpeeled( + NEW, "refs/bar/test", ObjectId.fromString("0000000000000000000000000000000000000004")); + + localRefs.add(ref1); + localRefs.add(ref2); + localRefs.add(ref3); + + pushOne.addRefBatch(ImmutableSet.of(ref1.getName(), ref2.getName(), ref3.getName())); + pushOne.run(); + + isCallFinished.await(10, TimeUnit.SECONDS); + + ArgumentCaptor<Ref> refCaptor = ArgumentCaptor.forClass(Ref.class); + verify(transportMock, atLeastOnce()).push(any(), any()); + verify(pushOne, times(1)).push(any(), any(), refCaptor.capture()); + assertThat(refCaptor.getValue().getName()).isEqualTo("refs/heads/master"); + } + + @Test public void shouldNotAttemptDuplicateRemoteRefUpdate() throws InterruptedException, IOException { PushOne pushOne = Mockito.spy(createPushOne(null)); @@ -459,6 +495,7 @@ private void setupDestinationMock() { destinationMock = mock(Destination.class); when(destinationMock.requestRunway(any())).thenReturn(RunwayStatus.allowed()); + when(destinationMock.excludedRefsPattern()).thenReturn(ImmutableList.of()); } private void setupPermissionBackedMock() {