Add sentinel to prevent out of sync with global-refdb
Make sure the the global refdb is never updated to a value that is
different from the local refdb. It may happen that a local operation has
an undetected failure that causes the global-refdb to be updated to a
non-existent value. By checking that the target SHA1 always corresponds
to the local refdb value, any discrepancy would result in an error to
the client and avoid to update the global refdb.
This sentinel should never be needed in Gerrit, however it sometimes
happens that the operation fails at Git level and the ref isn't updated
as expected. Prevent nasty out of sync that would require a Gerrit admin
to action and solve them one-by-one.
Change-Id: If30079abd39f17f3859aa3f8b300b65cc0b7f117
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
index 54eed1b..3ecec22 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
@@ -30,6 +30,7 @@
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.util.HashMap;
+import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectIdRef;
import org.eclipse.jgit.lib.Ref;
@@ -220,6 +221,22 @@
boolean succeeded;
try {
+ if (!sharedRefDb.isNoop()) {
+ ObjectId localObjectId =
+ Optional.ofNullable(refDb.findRef(refPair.compareRef.getName()))
+ .map(Ref::getObjectId)
+ .orElse(ObjectId.zeroId());
+ if (!localObjectId.equals(refPair.putValue)) {
+ String error =
+ String.format(
+ "Aborting the global-refdb update of %s = %s: local ref value is %s instead of"
+ + " the expected value %s",
+ refPair.getName(), refPair.putValue, localObjectId.name(), refPair.putValue);
+ logger.atSevere().log("%s", error);
+ throw new IOException(error);
+ }
+ }
+
succeeded =
sharedRefDb.compareAndPut(
Project.nameKey(projectName), refPair.compareRef, refPair.putValue);
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
index c9a6c7d..b973602 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
@@ -152,6 +152,10 @@
}
}
+ boolean isNoop() {
+ return sharedRefDbDynamicItem == null || sharedRefDbDynamicItem.get() == null;
+ }
+
private GlobalRefDatabase sharedRefDb() {
if (sharedRefDbDynamicItem == null) {
log.atWarning().log("DynamicItem<GlobalRefDatabase> has not been injected");
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
index b99749f..f26f749 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
@@ -241,11 +241,24 @@
}
@Test
- public void shouldNotUpdateSharedRefDbWhenProjectIsLocal() throws Exception {
+ public void shouldSucceedButNotUpdateSharedRefDbWhenProjectIsLocal() throws Exception {
when(projectsFilter.matches(anyString())).thenReturn(false);
- refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, this::defaultRollback);
+ Result result =
+ refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, this::defaultRollback);
+ assertThat(result).isEqualTo(Result.NEW);
+ verify(sharedRefDb, never())
+ .compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
+ }
+
+ @Test
+ public void shouldReturnLockFailureAndNotUpdateSharedRefDbWhenLocalRefUpdateNotExecuted()
+ throws Exception {
+ Result result =
+ refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, this::defaultRollback);
+
+ assertThat(result).isEqualTo(Result.LOCK_FAILURE);
verify(sharedRefDb, never())
.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
}
