Listen to OnlineUpgradeListener.onSuccess event
Issue:
During the Gerrit upgrade sometimes there is also an index upgrade
therefore Gerrit starts with the old index version set as ready and
issues an on-line reindex. It finally switches to the new version and
marks it as ready (marking the previous one as not-ready).
Solution:
Upon the Gerrit start detect the index version as it was before (IOW
monitor the old version's lock files). But in addition listen to the
`OnlineUpgradeListener.onSuccess` event and in case when `changes`
on-line reindex gets finished and its version changed switch to monitor
the new changes index version lock files.
The following scenarios were tested:
* gerrit 3.8.3 site was initiated and ~200 changes were created
* gerrit 3.9.1 init was performed and healthcheck plugin was dropped to
plugins
* gerrit 3.9.1 was started and `healthcheck~status` endpoint was called
a few times while reindex of changes was performed - all `passed`
* switch to newer changes index version locks was confirmed with the
following log entry:
QueryChangesHealthCheck : Changes index healthcheck switched from index version 82 to 84
but also by removing `write.lock` from old version (healtcheck passed)
and from new version (healtheck failed) as expected
* calling manual reindex on the same version was ignored by healtcheck
- again as expected
Bug: Issue 40015289
Change-Id: I6c2f8feebadafeb89f4ac8c17bce4b96ab8a5685
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
index d88c445..bff5c13 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
@@ -16,6 +16,7 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.index.OnlineUpgradeListener;
import com.googlesource.gerrit.plugins.healthcheck.check.ActiveWorkersCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.AuthHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsCheck;
@@ -39,6 +40,8 @@
bindChecker(BlockedThreadsCheck.class);
bindChecker(ChangesIndexHealthCheck.class);
+ DynamicSet.bind(binder(), OnlineUpgradeListener.class).to(ChangesIndexHealthCheck.class);
+
install(BlockedThreadsCheck.SUB_CHECKS);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ChangesIndexHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ChangesIndexHealthCheck.java
index 176cea9..bd16b7b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ChangesIndexHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ChangesIndexHealthCheck.java
@@ -21,6 +21,7 @@
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.index.OnlineUpgradeListener;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
@@ -28,6 +29,7 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.Optional;
+import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -36,12 +38,13 @@
import org.slf4j.LoggerFactory;
@Singleton
-public class ChangesIndexHealthCheck extends AbstractHealthCheck {
+public class ChangesIndexHealthCheck extends AbstractHealthCheck implements OnlineUpgradeListener {
private static final Logger log = LoggerFactory.getLogger(QueryChangesHealthCheck.class);
private static final String lockFilename = "write.lock";
+ private final SitePaths sitePaths;
private final boolean isLuceneIndex;
- private final Optional<ChangesIndexLockFiles> changes;
+ private final AtomicReference<Optional<ChangesIndexLockFiles>> changes;
@Inject
ChangesIndexHealthCheck(
@@ -51,17 +54,46 @@
MetricMaker metricMaker,
SitePaths sitePaths) {
super(executor, config, CHANGES_INDEX, metricMaker);
+ this.sitePaths = sitePaths;
this.isLuceneIndex = isIndexTypeLucene(cfg);
- this.changes = getChangesIndexLockFiles(sitePaths.index_dir);
+ this.changes = new AtomicReference<>(getChangesIndexLockFiles(sitePaths.index_dir));
}
@Override
protected Result doCheck() throws Exception {
return isLuceneIndex
- ? changes.map(locks -> locks.valid() ? Result.PASSED : Result.FAILED).orElse(Result.FAILED)
+ ? changes
+ .get()
+ .map(locks -> locks.valid() ? Result.PASSED : Result.FAILED)
+ .orElse(Result.FAILED)
: Result.DISABLED;
}
+ @Override
+ public void onStart(String name, int oldVersion, int newVersion) {}
+
+ @Override
+ public void onSuccess(String name, int oldVersion, int newVersion) {
+ if (!name.equals("changes") || oldVersion == newVersion) {
+ return;
+ }
+
+ Optional<ChangesIndexLockFiles> newLockFiles =
+ Optional.of(
+ getChangesLockFiles(sitePaths.index_dir, String.format("changes_%04d", newVersion)));
+ if (!changes.compareAndSet(changes.get(), newLockFiles)) {
+ log.info(
+ "New version {} of changes index healthcheck lock files was set already by another thread",
+ newVersion);
+ } else {
+ log.info(
+ "Changes index healthcheck switched from index version {} to {}", oldVersion, newVersion);
+ }
+ }
+
+ @Override
+ public void onFailure(String name, int oldVersion, int newVersion) {}
+
private static boolean isIndexTypeLucene(Config cfg) {
IndexType indexType = new IndexType(cfg.getString("index", null, "type"));
boolean isLucene = indexType.isLucene();
@@ -73,7 +105,7 @@
return isLucene;
}
- private static Optional<ChangesIndexLockFiles> getChangesIndexLockFiles(Path indexDir) {
+ private Optional<ChangesIndexLockFiles> getChangesIndexLockFiles(Path indexDir) {
FileBasedConfig cfg =
new FileBasedConfig(indexDir.resolve("gerrit_index.config").toFile(), FS.detect());
try {
@@ -96,7 +128,7 @@
.findAny();
}
- private static ChangesIndexLockFiles getChangesLockFiles(Path indexDir, String indexVersion) {
+ private ChangesIndexLockFiles getChangesLockFiles(Path indexDir, String indexVersion) {
Path versionDir = indexDir.resolve(indexVersion);
return new ChangesIndexLockFiles(
versionDir.resolve("open").resolve(lockFilename).toFile(),
