Rename RefPair to RefUpdateSnapshot and clean it up
- the old name RefPair was a misnomer, it's not a pair of refs
- the new name RefUpdateSnapshot better describes the purpose of this
class: it's an immutable snapshot of a JGit RefUpdate as the class
javadoc already explained
- reduce visibility of this class to package-visibility since it's only
used within its enclosing package
- add getters for all properties the callers need from a
RefUpdateSnapshot
Change-Id: I78c9d35c685beffbd3bc3d4ab18bd006d98382a6
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java
index 1e45951..cbdb8cd 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java
@@ -140,22 +140,24 @@
return;
}
- List<RefPair> refsToUpdate = getRefPairs(commands).collect(Collectors.toList());
- List<RefPair> refsFailures =
- refsToUpdate.stream().filter(RefPair::hasFailed).collect(Collectors.toList());
+ List<RefUpdateSnapshot> refsToUpdate =
+ getRefUpdateSnapshots(commands).collect(Collectors.toList());
+ List<RefUpdateSnapshot> refsFailures =
+ refsToUpdate.stream().filter(RefUpdateSnapshot::hasFailed).collect(Collectors.toList());
if (!refsFailures.isEmpty()) {
String allFailuresMessage =
refsFailures.stream()
- .map(refPair -> String.format("Failed to fetch ref %s", refPair.compareRef.getName()))
+ .map(refSnapshot -> String.format("Failed to fetch ref %s", refSnapshot.getName()))
.collect(Collectors.joining(", "));
- Exception firstFailureException = refsFailures.get(0).exception;
+ Exception firstFailureException = refsFailures.get(0).getException();
logger.atSevere().withCause(firstFailureException).log("%s", allFailuresMessage);
throw new IOException(allFailuresMessage, firstFailureException);
}
try (CloseableSet<AutoCloseable> locks = new CloseableSet<>()) {
- final List<RefPair> finalRefsToUpdate = compareAndGetLatestLocalRefs(refsToUpdate, locks);
+ final List<RefUpdateSnapshot> finalRefsToUpdate =
+ compareAndGetLatestLocalRefs(refsToUpdate, locks);
delegateUpdate.invoke();
boolean sharedDbUpdateSucceeded = false;
try {
@@ -184,7 +186,7 @@
private void rollback(
OneParameterVoidFunction<List<ReceiveCommand>> delegateUpdateRollback,
- List<RefPair> refsBeforeUpdate,
+ List<RefUpdateSnapshot> refsBeforeUpdate,
List<ReceiveCommand> receiveCommands)
throws IOException {
List<ReceiveCommand> rollbackCommands =
@@ -192,61 +194,62 @@
.map(
refBeforeUpdate ->
new ReceiveCommand(
- refBeforeUpdate.putValue,
- refBeforeUpdate.compareRef.getObjectId(),
+ refBeforeUpdate.getNewValue(),
+ refBeforeUpdate.getOldValue(),
refBeforeUpdate.getName()))
.collect(Collectors.toList());
delegateUpdateRollback.invoke(rollbackCommands);
receiveCommands.forEach(command -> command.setResult(ReceiveCommand.Result.LOCK_FAILURE));
}
- private void updateSharedRefDb(Stream<ReceiveCommand> commandStream, List<RefPair> refsToUpdate)
+ private void updateSharedRefDb(
+ Stream<ReceiveCommand> commandStream, List<RefUpdateSnapshot> refsToUpdate)
throws IOException {
if (commandStream.anyMatch(cmd -> cmd.getResult() != ReceiveCommand.Result.OK)) {
return;
}
- for (RefPair refPair : refsToUpdate) {
- updateSharedDbOrThrowExceptionFor(refPair);
+ for (RefUpdateSnapshot refUpdateSnapshot : refsToUpdate) {
+ updateSharedDbOrThrowExceptionFor(refUpdateSnapshot);
}
}
- private Stream<RefPair> getRefPairs(List<ReceiveCommand> receivedCommands) {
- return receivedCommands.stream().map(this::getRefPairForCommand);
+ private Stream<RefUpdateSnapshot> getRefUpdateSnapshots(List<ReceiveCommand> receivedCommands) {
+ return receivedCommands.stream().map(this::getRefUpdateSnapshotForCommand);
}
- private RefPair getRefPairForCommand(ReceiveCommand command) {
+ private RefUpdateSnapshot getRefUpdateSnapshotForCommand(ReceiveCommand command) {
try {
switch (command.getType()) {
case CREATE:
- return new RefPair(nullRef(command.getRefName()), getNewRef(command));
+ return new RefUpdateSnapshot(nullRef(command.getRefName()), getNewValue(command));
case UPDATE:
case UPDATE_NONFASTFORWARD:
- return new RefPair(getCurrentRef(command.getRefName()), getNewRef(command));
+ return new RefUpdateSnapshot(getCurrentRef(command.getRefName()), getNewValue(command));
case DELETE:
- return new RefPair(getCurrentRef(command.getRefName()), ObjectId.zeroId());
+ return new RefUpdateSnapshot(getCurrentRef(command.getRefName()), ObjectId.zeroId());
default:
- return new RefPair(
+ return new RefUpdateSnapshot(
command.getRef(),
new IllegalArgumentException("Unsupported command type " + command.getType()));
}
} catch (IOException e) {
- return new RefPair(command.getRef(), e);
+ return new RefUpdateSnapshot(command.getRef(), e);
}
}
- private ObjectId getNewRef(ReceiveCommand command) {
+ private ObjectId getNewValue(ReceiveCommand command) {
return command.getNewId();
}
- private List<RefPair> compareAndGetLatestLocalRefs(
- List<RefPair> refsToUpdate, CloseableSet<AutoCloseable> locks) throws IOException {
- List<RefPair> latestRefsToUpdate = new ArrayList<>();
- for (RefPair refPair : refsToUpdate) {
- latestRefsToUpdate.add(compareAndGetLatestLocalRef(refPair, locks));
+ private List<RefUpdateSnapshot> compareAndGetLatestLocalRefs(
+ List<RefUpdateSnapshot> refsToUpdate, CloseableSet<AutoCloseable> locks) throws IOException {
+ List<RefUpdateSnapshot> latestRefsToUpdate = new ArrayList<>();
+ for (RefUpdateSnapshot refUpdateSnapshot : refsToUpdate) {
+ latestRefsToUpdate.add(compareAndGetLatestLocalRef(refUpdateSnapshot, locks));
}
return latestRefsToUpdate;
}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefPair.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefPair.java
deleted file mode 100644
index 4211c0c..0000000
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefPair.java
+++ /dev/null
@@ -1,76 +0,0 @@
-// Copyright (C) 2020 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.gerritforge.gerrit.globalrefdb.validation;
-
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Ref;
-
-/**
- * A convenience object encompassing the old (current) and the new (candidate) value of a {@link
- * Ref}. This is used to snapshot the current status of a ref update so that validations against the
- * global refdb are unaffected by changes on the {@link org.eclipse.jgit.lib.RefDatabase}.
- */
-public class RefPair {
- public final Ref compareRef;
- public final ObjectId putValue;
- public final Exception exception;
-
- /**
- * Constructs a {@code RefPair} with the provided old and new ref values. The oldRef value is
- * required not to be null, in which case an {@link IllegalArgumentException} is thrown.
- *
- * @param oldRef the old ref
- * @param newRefValue the new (candidate) value for this ref.
- */
- RefPair(Ref oldRef, ObjectId newRefValue) {
- if (oldRef == null) {
- throw new IllegalArgumentException("Required not-null ref in RefPair");
- }
- this.compareRef = oldRef;
- this.putValue = newRefValue;
- this.exception = null;
- }
-
- /**
- * Constructs a {@code RefPair} with the current ref and an Exception indicating why the new ref
- * value failed being retrieved.
- *
- * @param newRef
- * @param e
- */
- RefPair(Ref newRef, Exception e) {
- this.compareRef = newRef;
- this.exception = e;
- this.putValue = ObjectId.zeroId();
- }
-
- /**
- * Getter for the current ref
- *
- * @return the current ref value
- */
- public String getName() {
- return compareRef.getName();
- }
-
- /**
- * Whether the new value failed being retrieved
- *
- * @return true when this refPair has failed, false otherwise.
- */
- public boolean hasFailed() {
- return exception != null;
- }
-}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateSnapshot.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateSnapshot.java
new file mode 100644
index 0000000..2e83be3
--- /dev/null
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateSnapshot.java
@@ -0,0 +1,114 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.gerritforge.gerrit.globalrefdb.validation;
+
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+
+/**
+ * A convenience object encompassing the old (current) and the new (candidate) value of a {@link
+ * Ref}. This is used to snapshot the current status of a ref update so that validations against the
+ * global refdb are unaffected by changes on the underlying {@link
+ * org.eclipse.jgit.lib.RefDatabase}.
+ */
+class RefUpdateSnapshot {
+ private final Ref ref;
+ private final ObjectId newValue;
+ private final Exception exception;
+
+ /**
+ * Constructs a {@code RefUpdateSnapshot} with the provided old and new values. The old value of
+ * this Ref must not be null, otherwise an {@link IllegalArgumentException} is thrown.
+ *
+ * @param ref the ref with its old value
+ * @param newRefValue the new (candidate) value for this ref.
+ */
+ RefUpdateSnapshot(Ref ref, ObjectId newRefValue) {
+ if (ref == null) {
+ throw new IllegalArgumentException("RefUpdateSnapshot cannot be created for null Ref");
+ }
+ this.ref = ref;
+ this.newValue = newRefValue;
+ this.exception = null;
+ }
+
+ /**
+ * Constructs a {@code RefUpdateSnapshot} with the current ref and an Exception indicating why the
+ * ref's new value could not be retrieved.
+ *
+ * @param ref the ref with its old value
+ * @param e exception caught when trying to retrieve the ref's new value
+ */
+ RefUpdateSnapshot(Ref ref, Exception e) {
+ this.ref = ref;
+ this.newValue = ObjectId.zeroId();
+ this.exception = e;
+ }
+
+ /**
+ * Get the ref name
+ *
+ * @return the ref name
+ */
+ public String getName() {
+ return ref.getName();
+ }
+
+ /**
+ * Get the ref's old value
+ *
+ * @return the ref's old value
+ */
+ public ObjectId getOldValue() {
+ return ref.getObjectId();
+ }
+
+ /**
+ * Get the ref's new (candidate) value
+ *
+ * @return the ref's new (candidate) value
+ */
+ public ObjectId getNewValue() {
+ return newValue;
+ }
+
+ /**
+ * Get the snapshotted ref with its old value
+ *
+ * @return the snapshotted ref with its old value
+ */
+ public Ref getRef() {
+ return ref;
+ }
+
+ /**
+ * Get the exception which occurred when retrieving the ref's new value
+ *
+ * @return the exception which occurred when retrieving the ref's new value
+ */
+ public Exception getException() {
+ return exception;
+ }
+
+ /**
+ * Whether retrieving the new (candidate) value failed
+ *
+ * @return {@code true} when retrieving the ref's new (candidate) value failed, {@code false}
+ * otherwise.
+ */
+ public boolean hasFailed() {
+ return exception != null;
+ }
+}
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 8823f3d..31d3af1 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
@@ -186,16 +186,16 @@
OneParameterFunction<ObjectId, Result> rollbackFunction)
throws IOException {
try (CloseableSet<AutoCloseable> locks = new CloseableSet<>()) {
- RefPair refPairForUpdate = newRefPairFrom(refUpdate);
- compareAndGetLatestLocalRef(refPairForUpdate, locks);
+ RefUpdateSnapshot refUpdateSnapshot = newSnapshot(refUpdate);
+ compareAndGetLatestLocalRef(refUpdateSnapshot, locks);
RefUpdate.Result result = refUpdateFunction.invoke();
if (!isSuccessful(result)) {
return result;
}
try {
- updateSharedDbOrThrowExceptionFor(refPairForUpdate);
+ updateSharedDbOrThrowExceptionFor(refUpdateSnapshot);
} catch (Exception e) {
- result = rollbackFunction.invoke(refPairForUpdate.compareRef.getObjectId());
+ result = rollbackFunction.invoke(refUpdateSnapshot.getOldValue());
if (isSuccessful(result)) {
result = RefUpdate.Result.LOCK_FAILURE;
}
@@ -207,26 +207,26 @@
} catch (OutOfSyncException e) {
logger.atWarning().withCause(e).log(
"Local node is out of sync with ref-db: %s", e.getMessage());
-
return RefUpdate.Result.LOCK_FAILURE;
}
}
- protected void updateSharedDbOrThrowExceptionFor(RefPair refPair) throws IOException {
+ protected void updateSharedDbOrThrowExceptionFor(RefUpdateSnapshot refSnapshot)
+ throws IOException {
// We are not checking refs that should be ignored
final EnforcePolicy refEnforcementPolicy =
- refEnforcement.getPolicy(projectName, refPair.getName());
+ refEnforcement.getPolicy(projectName, refSnapshot.getName());
if (refEnforcementPolicy == EnforcePolicy.IGNORED) return;
boolean succeeded;
try {
succeeded =
sharedRefDb.compareAndPut(
- Project.nameKey(projectName), refPair.compareRef, refPair.putValue);
+ Project.nameKey(projectName), refSnapshot.getRef(), refSnapshot.getNewValue());
} catch (GlobalRefDbSystemError e) {
logger.atWarning().withCause(e).log(
"Not able to persist the data in global-refdb for project '%s', ref '%s' and value %s, message: %s",
- projectName, refPair.getName(), refPair.putValue, e.getMessage());
+ projectName, refSnapshot.getName(), refSnapshot.getNewValue(), e.getMessage());
throw e;
}
@@ -236,17 +236,18 @@
"Not able to persist the data in SharedRef for project '%s' and ref '%s',"
+ "the cluster is now in Split Brain since the commit has been "
+ "persisted locally but not in global-refdb the value %s",
- projectName, refPair.getName(), refPair.putValue);
+ projectName, refSnapshot.getName(), refSnapshot.getNewValue());
throw new SharedDbSplitBrainException(errorMessage);
}
}
- protected RefPair compareAndGetLatestLocalRef(RefPair refPair, CloseableSet<AutoCloseable> locks)
+ protected RefUpdateSnapshot compareAndGetLatestLocalRef(
+ RefUpdateSnapshot refUpdateSnapshot, CloseableSet<AutoCloseable> locks)
throws GlobalRefDbLockException, OutOfSyncException, IOException {
- String refName = refPair.getName();
+ String refName = refUpdateSnapshot.getName();
EnforcePolicy refEnforcementPolicy = refEnforcement.getPolicy(projectName, refName);
if (refEnforcementPolicy == EnforcePolicy.IGNORED) {
- return refPair;
+ return refUpdateSnapshot;
}
locks.addResourceIfNotExist(
@@ -255,30 +256,33 @@
lockWrapperFactory.create(
projectName, refName, sharedRefDb.lockRef(Project.nameKey(projectName), refName)));
- RefPair latestRefPair = getLatestLocalRef(refPair);
- if (sharedRefDb.isUpToDate(Project.nameKey(projectName), latestRefPair.compareRef)) {
- return latestRefPair;
+ RefUpdateSnapshot latestRefUpdateSnapshot = getLatestLocalRef(refUpdateSnapshot);
+ if (sharedRefDb.isUpToDate(Project.nameKey(projectName), latestRefUpdateSnapshot.getRef())) {
+ return latestRefUpdateSnapshot;
}
- if (isNullRef(latestRefPair.compareRef)
+ if (isNullRef(latestRefUpdateSnapshot.getRef())
|| sharedRefDb.exists(Project.nameKey(projectName), refName)) {
validationMetrics.incrementSplitBrainPrevention();
softFailBasedOnEnforcement(
- new OutOfSyncException(projectName, latestRefPair.compareRef), refEnforcementPolicy);
+ new OutOfSyncException(projectName, latestRefUpdateSnapshot.getRef()),
+ refEnforcementPolicy);
}
- return latestRefPair;
+ return latestRefUpdateSnapshot;
}
private boolean isNullRef(Ref ref) {
return ref.getObjectId().equals(ObjectId.zeroId());
}
- private RefPair getLatestLocalRef(RefPair refPair) throws IOException {
- Ref latestRef = refDb.exactRef(refPair.getName());
- return new RefPair(
- latestRef == null ? nullRef(refPair.getName()) : latestRef, refPair.putValue);
+ private RefUpdateSnapshot getLatestLocalRef(RefUpdateSnapshot refUpdateSnapshot)
+ throws IOException {
+ Ref latestRef = refDb.exactRef(refUpdateSnapshot.getName());
+ return new RefUpdateSnapshot(
+ latestRef == null ? nullRef(refUpdateSnapshot.getName()) : latestRef,
+ refUpdateSnapshot.getNewValue());
}
private Ref nullRef(String name) {
@@ -306,8 +310,8 @@
}
}
- protected RefPair newRefPairFrom(RefUpdate refUpdate) throws IOException {
- return new RefPair(getCurrentRef(refUpdate.getName()), refUpdate.getNewObjectId());
+ protected RefUpdateSnapshot newSnapshot(RefUpdate refUpdate) throws IOException {
+ return new RefUpdateSnapshot(getCurrentRef(refUpdate.getName()), refUpdate.getNewObjectId());
}
protected Ref getCurrentRef(String refName) throws IOException {
