Merge branch 'stable-3.10'
* stable-3.10:
Short-circuit on failed healthchecks
Simplify construction of map of check results
Simplify check for any failures
Run checks in parallel
Change-Id: I30c16e082fca33af3ae82d610d8e5191fdaf584d
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/GlobalHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/GlobalHealthCheck.java
index fa5f14a..4e4cef7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/GlobalHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/GlobalHealthCheck.java
@@ -22,9 +22,11 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
-import java.util.Arrays;
+import java.util.Collections;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
@@ -33,6 +35,31 @@
private final DynamicSet<HealthCheck> healthChecks;
+ public static class MemoizedStatusSummary implements Supplier<StatusSummary> {
+ private final AtomicReference<StatusSummary> result = new AtomicReference<>();
+ private final HealthCheck check;
+
+ MemoizedStatusSummary(HealthCheck check) {
+ this.check = check;
+ }
+
+ @Override
+ public StatusSummary get() {
+ if (result.get() == null) {
+ result.set(check.run());
+ }
+ return result.get();
+ }
+
+ public StatusSummary getIfCompleted() {
+ StatusSummary completedResult = result.get();
+ return completedResult == null
+ ? new StatusSummary(
+ Result.NOT_RUN, System.currentTimeMillis(), 0L, Collections.emptyMap())
+ : completedResult;
+ }
+ }
+
@Inject
public GlobalHealthCheck(
DynamicSet<HealthCheck> healthChecks,
@@ -47,17 +74,16 @@
public HealthCheck.StatusSummary run() {
Iterable<HealthCheck> iterable = () -> healthChecks.iterator();
long ts = System.currentTimeMillis();
- Map<String, Object> checkToResults =
+ Map<String, MemoizedStatusSummary> checkToResults =
StreamSupport.stream(iterable.spliterator(), false)
- .map(check -> Arrays.asList(check.name(), check.run()))
- .collect(Collectors.toMap(k -> (String) k.get(0), v -> v.get(1)));
+ .collect(Collectors.toMap(HealthCheck::name, MemoizedStatusSummary::new));
long elapsed = System.currentTimeMillis() - ts;
+ Result checkResult = hasAnyFailureOnResults(checkToResults) ? Result.FAILED : Result.PASSED;
+ Map<String, Object> reportedResults =
+ checkToResults.entrySet().stream()
+ .collect(Collectors.toMap(Map.Entry::getKey, v -> v.getValue().getIfCompleted()));
StatusSummary globalStatus =
- new HealthCheck.StatusSummary(
- hasAnyFailureOnResults(checkToResults) ? Result.FAILED : Result.PASSED,
- ts,
- elapsed,
- checkToResults);
+ new HealthCheck.StatusSummary(checkResult, ts, elapsed, reportedResults);
if (globalStatus.isFailure()) {
failureCounterMetric.increment();
}
@@ -70,13 +96,7 @@
return run().result;
}
- public static boolean hasAnyFailureOnResults(Map<String, Object> results) {
- return results.values().stream()
- .filter(
- res ->
- res instanceof HealthCheck.StatusSummary
- && ((HealthCheck.StatusSummary) res).isFailure())
- .findAny()
- .isPresent();
+ public static boolean hasAnyFailureOnResults(Map<String, MemoizedStatusSummary> results) {
+ return results.values().stream().parallel().anyMatch(res -> res.get().isFailure());
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
index 7c4f8a1..db07320 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
@@ -30,6 +30,8 @@
FAILED,
@SerializedName("timeout")
TIMEOUT,
+ @SerializedName("not_run")
+ NOT_RUN,
@SerializedName("disabled")
DISABLED;
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckIntegrationTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckIntegrationTest.java
index 56f96fe..1b1cd70 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckIntegrationTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckIntegrationTest.java
@@ -16,13 +16,20 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.metrics.MetricMaker;
import com.google.gson.Gson;
import com.google.gson.JsonObject;
import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
import com.google.inject.Key;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.healthcheck.check.AbstractHealthCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -34,6 +41,33 @@
protected void configure() {
install(new HealthCheckExtensionApiModule());
install(new Module());
+ DynamicSet.bind(binder(), HealthCheck.class).to(FakeHealthCheck.class);
+ }
+ }
+
+ @Singleton
+ public static class FakeHealthCheck extends AbstractHealthCheck {
+ private long sleep = 0L;
+ private Result result = Result.PASSED;
+
+ @Inject
+ public FakeHealthCheck(
+ ListeningExecutorService executor, HealthCheckConfig config, MetricMaker metricMaker) {
+ super(executor, config, "fake-check", metricMaker);
+ }
+
+ public void setSleep(long sleep) {
+ this.sleep = sleep;
+ }
+
+ public void setResult(Result result) {
+ this.result = result;
+ }
+
+ @Override
+ protected Result doCheck() throws Exception {
+ Thread.sleep(sleep);
+ return result;
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
index 86ce328..621ba17 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
@@ -26,9 +26,15 @@
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import java.io.File;
import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Before;
@@ -114,6 +120,25 @@
}
@Test
+ public void shouldNotRunChecksWhenOneFails() throws Exception {
+ FakeHealthCheck fakeHealthCheck = plugin.getSysInjector().getInstance(FakeHealthCheck.class);
+ fakeHealthCheck.setResult(HealthCheck.Result.FAILED);
+
+ RestResponse resp = getHealthCheckStatus();
+ resp.assertStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ JsonObject respJson = getResponseJson(resp);
+ Set<Map.Entry<String, JsonElement>> checkResults = respJson.entrySet();
+ List<String> results =
+ checkResults.stream()
+ .map(Map.Entry::getValue)
+ .filter(JsonElement::isJsonObject)
+ .map(j -> j.getAsJsonObject().get("result"))
+ .map(JsonElement::getAsString)
+ .collect(Collectors.toList());
+ assertThat(results).contains("not_run");
+ }
+
+ @Test
public void shouldReturnAuthCheckAsDisabled() throws Exception {
disableCheck(AUTH);
RestResponse resp = getHealthCheckStatus();
