Improve readability of the refs filtering during replication
Refactor the filtering of refs during replication so that becomes clearer
to read and less prone to bugs in the future.
Change-Id: I9d4b1e66e9628a6f3aef6975c60bf8617013629c
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationPushFilter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationPushFilter.java
index e4bd2e3..b85ec89 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationPushFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationPushFilter.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.multisite.validation;
+import com.google.common.base.Preconditions;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -27,12 +28,10 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
-import java.util.Optional;
import java.util.Random;
import java.util.Set;
import java.util.stream.Collectors;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectIdRef;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.RemoteRefUpdate;
@@ -68,57 +67,16 @@
remoteUpdatesList.stream()
.filter(
refUpdate -> {
- String ref = refUpdate.getSrcRef();
- try {
- if (sharedRefDb.isUpToDate(
- projectName, SharedRefDatabase.newRef(ref, refUpdate.getNewObjectId()))) {
- return true;
- }
- // The ref coming from the event might be old compared to the local version.
- // Valid refs won't be replicated because of this misalignment.
- // Reading the local ref and re-trying the comparison, after a short sleep,
- // could mitigate the issue.
- int waitBeforeReloadLocalVersionMs =
- MIN_WAIT_BEFORE_RELOAD_LOCAL_VERSION_MS
- + new Random().nextInt(RANDOM_WAIT_BEFORE_RELOAD_LOCAL_VERSION_MS);
- repLog.debug(
- String.format(
- "'%s' is not up-to-date for project '%s' [local='%s']. Reload local ref in '%d ms' and re-check",
- ref,
- projectName,
- refUpdate.getNewObjectId(),
- waitBeforeReloadLocalVersionMs));
- Thread.sleep(waitBeforeReloadLocalVersionMs);
- Optional<ObjectId> objectIdVersion =
- getProjectLocalObjectIdVersion(repository, projectName, ref);
- if (objectIdVersion.isPresent()
- && sharedRefDb.isUpToDate(
- projectName,
- new ObjectIdRef.Unpeeled(
- Ref.Storage.NETWORK, ref, objectIdVersion.get()))) {
- repLog.debug("{} is up-to-date after retrying", objectIdVersion);
- return true;
- }
+ boolean refUpToDate = isUpToDateWithRetry(projectName, repository, refUpdate);
+ if (!refUpToDate) {
repLog.warn(
"{} is not up-to-date with the shared-refdb and thus will NOT BE replicated",
refUpdate);
- } catch (SharedLockException e) {
- repLog.warn(
- "{} is locked on shared-refdb and thus will NOT BE replicated",
- refUpdate);
- } catch (InterruptedException ie) {
- String message =
- String.format(
- "Error while waiting for next check for '%s', ref '%s'",
- projectName, ref);
- repLog.error(message);
- logger.atSevere().withCause(ie).log(message);
- return false;
+ if (refUpdate.getSrcRef().endsWith(REF_META_SUFFIX)) {
+ outdatedChanges.add(getRootChangeRefPrefix(refUpdate.getSrcRef()));
+ }
}
- if (ref.endsWith(REF_META_SUFFIX)) {
- outdatedChanges.add(getRootChangeRefPrefix(ref));
- }
- return false;
+ return refUpToDate;
})
.collect(Collectors.toList());
@@ -143,6 +101,54 @@
}
}
+ private boolean isUpToDateWithRetry(
+ String projectName, Repository repository, RemoteRefUpdate refUpdate) {
+ String ref = refUpdate.getSrcRef();
+ try {
+ if (sharedRefDb.isUpToDate(
+ projectName, SharedRefDatabase.newRef(ref, refUpdate.getNewObjectId()))) {
+ return true;
+ }
+
+ randomSleepForMitigatingConditionWhereLocalRefHaveJustBeenChanged(
+ projectName, refUpdate, ref);
+
+ return sharedRefDb.isUpToDate(
+ projectName, SharedRefDatabase.newRef(ref, getNotNullExactRef(repository, ref)));
+ } catch (SharedLockException sle) {
+ String message =
+ String.format("%s is locked on shared-refdb and thus will NOT BE replicated", ref);
+ repLog.error(message);
+ logger.atSevere().withCause(sle).log(message);
+ return false;
+ } catch (IOException ioe) {
+ String message =
+ String.format("Error while extracting ref '%s' for project '%s'", ref, projectName);
+ repLog.error(message);
+ logger.atSevere().withCause(ioe).log(message);
+ return false;
+ }
+ }
+
+ private void randomSleepForMitigatingConditionWhereLocalRefHaveJustBeenChanged(
+ String projectName, RemoteRefUpdate refUpdate, String ref) {
+ int randomSleepTimeMsec =
+ MIN_WAIT_BEFORE_RELOAD_LOCAL_VERSION_MS
+ + new Random().nextInt(RANDOM_WAIT_BEFORE_RELOAD_LOCAL_VERSION_MS);
+ repLog.debug(
+ String.format(
+ "'%s' is not up-to-date for project '%s' [local='%s']. Reload local ref in '%d ms' and re-check",
+ ref, projectName, refUpdate.getNewObjectId(), randomSleepTimeMsec));
+ try {
+ Thread.sleep(randomSleepTimeMsec);
+ } catch (InterruptedException ie) {
+ String message =
+ String.format("Error while waiting for next check for '%s', ref '%s'", projectName, ref);
+ repLog.error(message);
+ logger.atWarning().withCause(ie).log(message);
+ }
+ }
+
private String changePrefix(String changeRef) {
if (changeRef == null || !changeRef.startsWith("refs/changes")) {
return changeRef;
@@ -163,17 +169,9 @@
return changeMetaRef;
}
- private Optional<ObjectId> getProjectLocalObjectIdVersion(
- Repository repository, String projectName, String ref) {
- try {
- return Optional.ofNullable(repository.exactRef(ref).getObjectId());
-
- } catch (IOException ioe) {
- String message =
- String.format("Error while extracting ref '%s' for project '%s'", ref, projectName);
- repLog.error(message);
- logger.atSevere().withCause(ioe).log(message);
- }
- return Optional.empty();
+ private ObjectId getNotNullExactRef(Repository repository, String refName) throws IOException {
+ Ref ref = repository.exactRef(refName);
+ Preconditions.checkNotNull(ref);
+ return ref.getObjectId();
}
}
