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)); }