Merge branch 'stable-3.0'
* stable-3.0:
Make sure to always compare the latest local ref
Adapt to the latest metrics interface in master using the
new Field.of() builder syntax.
Change-Id: I9294778973da73805e214acb6b5072ad8c60b03a
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/broker/BrokerMetrics.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/broker/BrokerMetrics.java
index f6be65a..3f8aa8d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/broker/BrokerMetrics.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/broker/BrokerMetrics.java
@@ -38,14 +38,18 @@
new Description("Number of messages published by the broker publisher")
.setRate()
.setUnit("messages"),
- Field.ofString(PUBLISHER_SUCCESS_COUNTER, "Broker message published count"));
+ Field.ofString(PUBLISHER_SUCCESS_COUNTER)
+ .description("Broker message published count")
+ .build());
this.brokerPublisherFailureCounter =
metricMaker.newCounter(
"multi_site/broker/broker_message_publisher_failure_counter",
new Description("Number of messages failed to publish by the broker publisher")
.setRate()
.setUnit("errors"),
- Field.ofString(PUBLISHER_FAILURE_COUNTER, "Broker failed to publish message count"));
+ Field.ofString(PUBLISHER_FAILURE_COUNTER)
+ .description("Broker failed to publish message count")
+ .build());
}
public void incrementBrokerPublishedMessage() {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidator.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidator.java
index 59b5753..33be190 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidator.java
@@ -21,6 +21,7 @@
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -92,7 +93,7 @@
}
try (CloseableSet<AutoCloseable> locks = new CloseableSet<>()) {
- checkIfLocalRefIsUpToDateWithSharedRefDb(refsToUpdate, locks);
+ refsToUpdate = compareAndGetLatestLocalRefs(refsToUpdate, locks);
delegateUpdate.invoke();
updateSharedRefDb(batchRefUpdate.getCommands().stream(), refsToUpdate);
}
@@ -143,10 +144,12 @@
return command.getNewId();
}
- private void checkIfLocalRefIsUpToDateWithSharedRefDb(
+ private List<RefPair> compareAndGetLatestLocalRefs(
List<RefPair> refsToUpdate, CloseableSet<AutoCloseable> locks) throws IOException {
+ List<RefPair> latestRefsToUpdate = new ArrayList<>();
for (RefPair refPair : refsToUpdate) {
- checkIfLocalRefIsUpToDateWithSharedRefDb(refPair, locks);
+ latestRefsToUpdate.add(compareAndGetLatestLocalRef(refPair, locks));
}
+ return latestRefsToUpdate;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidator.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidator.java
index 18f8654..dec6ae4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidator.java
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.multisite.validation;
+import static com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefDatabase.nullRef;
+
import com.google.common.base.MoreObjects;
import com.google.common.flogger.FluentLogger;
import com.google.inject.Inject;
@@ -26,6 +28,7 @@
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy;
import java.io.IOException;
import java.util.HashMap;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.RefUpdate;
@@ -113,7 +116,7 @@
throws IOException {
try (CloseableSet<AutoCloseable> locks = new CloseableSet<>()) {
RefPair refPairForUpdate = newRefPairFrom(refUpdate);
- checkIfLocalRefIsUpToDateWithSharedRefDb(refPairForUpdate, locks);
+ compareAndGetLatestLocalRef(refPairForUpdate, locks);
RefUpdate.Result result = refUpdateFunction.invoke();
if (isSuccessful(result)) {
updateSharedDbOrThrowExceptionFor(refPairForUpdate);
@@ -146,32 +149,39 @@
}
}
- protected void checkIfLocalRefIsUpToDateWithSharedRefDb(
+ protected RefPair compareAndGetLatestLocalRef(
RefPair refPair, CloseableSet<AutoCloseable> locks)
throws SharedLockException, OutOfSyncException, IOException {
String refName = refPair.getName();
EnforcePolicy refEnforcementPolicy = refEnforcement.getPolicy(projectName, refName);
if (refEnforcementPolicy == EnforcePolicy.IGNORED) {
- return;
+ return refPair;
}
- Ref localRef = refPair.compareRef;
-
locks.addResourceIfNotExist(
String.format("%s-%s", projectName, refName),
() -> sharedRefDb.lockRef(projectName, refName));
+ RefPair latestRefPair = getLatestLocalRef(refPair);
boolean isInSync =
- (localRef != null)
- ? sharedRefDb.isUpToDate(projectName, localRef)
- : !sharedRefDb.exists(projectName, refName);
+ (latestRefPair.compareRef.getObjectId().equals(ObjectId.zeroId()))
+ ? !sharedRefDb.exists(projectName, refName)
+ : sharedRefDb.isUpToDate(projectName, latestRefPair.compareRef);
if (!isInSync) {
validationMetrics.incrementSplitBrainPrevention();
softFailBasedOnEnforcement(
- new OutOfSyncException(projectName, localRef), refEnforcementPolicy);
+ new OutOfSyncException(projectName, latestRefPair.compareRef), refEnforcementPolicy);
}
+
+ return latestRefPair;
+ }
+
+ private RefPair getLatestLocalRef(RefPair refPair) throws IOException {
+ Ref latestRef = refDb.exactRef(refPair.getName());
+ return new RefPair(
+ latestRef == null ? nullRef(refPair.getName()) : latestRef, refPair.putValue);
}
protected boolean isSuccessful(RefUpdate.Result result) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationMetrics.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationMetrics.java
index ee9c5e5..7f5c637 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationMetrics.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationMetrics.java
@@ -35,17 +35,17 @@
metricMaker.newCounter(
"multi_site/validation/git_update_split_brain_prevented",
new Description("Rate of REST API error responses").setRate().setUnit("errors"),
- Field.ofString(
- GIT_UPDATE_SPLIT_BRAIN_PREVENTED,
- "Ref-update operations, split-brain detected and prevented"));
+ Field.ofString(GIT_UPDATE_SPLIT_BRAIN_PREVENTED)
+ .description("Ref-update operations, split-brain detected and prevented")
+ .build());
this.splitBrainCounter =
metricMaker.newCounter(
"multi_site/validation/git_update_split_brain",
new Description("Rate of REST API error responses").setRate().setUnit("errors"),
- Field.ofString(
- GIT_UPDATE_SPLIT_BRAIN,
- "Ref-update operation left node in a split-brain scenario"));
+ Field.ofString(GIT_UPDATE_SPLIT_BRAIN)
+ .description("Ref-update operation left node in a split-brain scenario")
+ .build());
}
public void incrementSplitBrainPrevention() {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
index 45600d0..50c1c90 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
@@ -99,6 +99,7 @@
.thenReturn(asList(successReceiveCommandAfterExecution));
doReturn(oldRef).when(refDatabase).getRef(A_TEST_REF_NAME);
+ doReturn(oldRef).when(refDatabase).exactRef(A_TEST_REF_NAME);
multiSiteRefUpdate = getMultiSiteBatchRefUpdateWithDefaultPolicyEnforcement();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidatorTest.java
index e7624bd..d95860c 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidatorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidatorTest.java
@@ -67,6 +67,7 @@
localRef = newRef(refName, AN_OBJECT_ID_3);
doReturn(localRef).when(localRefDb).getRef(refName);
+ doReturn(localRef).when(localRefDb).exactRef(refName);
doReturn(oldUpdateRef).when(refUpdate).getRef();
doReturn(newUpdateRef.getObjectId()).when(refUpdate).getNewObjectId();
doReturn(refName).when(refUpdate).getName();
