Execute checks asynchronously with timeout
Do not block during the execution of the health-checks. Wait up to
500 msec and fail for timeout if not completed yet.
Change-Id: I1b282e9de339ace631262bd16b6e13cc1ffa35e4
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckModule.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckModule.java
index 6971282..0fa02f2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckModule.java
@@ -14,14 +14,21 @@
package com.googlesource.gerrit.plugins.healthcheck;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.inject.AbstractModule;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
+import java.util.concurrent.Executors;
public class HealthCheckModule extends AbstractModule {
+ public static final int CHECK_THREADS_DEFAULT = 10;
@Override
protected void configure() {
+ bind(ListeningExecutorService.class)
+ .toInstance(
+ MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(CHECK_THREADS_DEFAULT)));
DynamicSet.setOf(binder(), HealthCheck.class);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java
index f186092..5c8d410 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java
@@ -14,14 +14,22 @@
package com.googlesource.gerrit.plugins.healthcheck.check;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public abstract class AbstractHealthCheck implements HealthCheck {
private static final Logger log = LoggerFactory.getLogger(AbstractHealthCheck.class);
+ public static final long CHECK_TIMEOUT = 500L;
private final String name;
+ private final ListeningExecutorService executor;
- protected AbstractHealthCheck(String name) {
+ protected AbstractHealthCheck(ListeningExecutorService executor, String name) {
+ this.executor = executor;
this.name = name;
}
@@ -32,15 +40,29 @@
@Override
public Status run() {
- Result healthy;
- long ts = System.currentTimeMillis();
+ final long ts = System.currentTimeMillis();
+ ListenableFuture<Status> resultFuture =
+ executor.submit(
+ () -> {
+ Result healthy;
+ try {
+ healthy = doCheck();
+ } catch (Exception e) {
+ log.warn("Check {} failed", name, e);
+ healthy = Result.FAILED;
+ }
+ return new Status(healthy, ts, System.currentTimeMillis() - ts);
+ });
+
try {
- healthy = doCheck();
- } catch (Exception e) {
- log.warn("Check {} failed", name, e);
- healthy = Result.FAILED;
+ return resultFuture.get(CHECK_TIMEOUT, TimeUnit.MILLISECONDS);
+ } catch (TimeoutException e) {
+ log.warn("Check {} timed out", name, e);
+ return new Status(Result.TIMEOUT, ts, System.currentTimeMillis() - ts);
+ } catch (InterruptedException | ExecutionException e) {
+ log.warn("Check {} failed while waiting for its future result", name, e);
+ return new Status(Result.FAILED, ts, System.currentTimeMillis() - ts);
}
- return new Status(healthy, ts, System.currentTimeMillis() - ts);
}
protected abstract Result doCheck() throws Exception;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java
index be0a373..a23e0d1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java
@@ -16,6 +16,7 @@
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.JGIT;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
@@ -27,8 +28,11 @@
private final AllProjectsName allProjectsName;
@Inject
- public JGitHealthCheck(GitRepositoryManager repositoryManager, AllProjectsName allProjectsName) {
- super(JGIT);
+ public JGitHealthCheck(
+ ListeningExecutorService executor,
+ GitRepositoryManager repositoryManager,
+ AllProjectsName allProjectsName) {
+ super(executor, JGIT);
this.repositoryManager = repositoryManager;
this.allProjectsName = allProjectsName;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java
index 669ae62..580f3a0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java
@@ -16,6 +16,7 @@
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.PROJECTSLIST;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.server.project.ListProjects;
import com.google.inject.Inject;
@@ -29,8 +30,8 @@
private final ListProjects listProjects;
@Inject
- public ProjectsListHealthCheck(ListProjects listProjects) {
- super(PROJECTSLIST);
+ public ProjectsListHealthCheck(ListeningExecutorService executor, ListProjects listProjects) {
+ super(executor, PROJECTSLIST);
this.listProjects = listProjects;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ReviewDbHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ReviewDbHealthCheck.java
index 10aad07..138997b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ReviewDbHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ReviewDbHealthCheck.java
@@ -16,27 +16,24 @@
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.REVIEWDB;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.reviewdb.client.CurrentSchemaVersion;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
-import com.google.inject.Provider;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class ReviewDbHealthCheck extends AbstractHealthCheck {
- private static final Logger log = LoggerFactory.getLogger(ReviewDbHealthCheck.class);
-
- private final Provider<ReviewDb> reviewDb;
+ private final SchemaFactory<ReviewDb> reviewDb;
@Inject
- public ReviewDbHealthCheck(Provider<ReviewDb> reviewDb) {
- super(REVIEWDB);
+ public ReviewDbHealthCheck(ListeningExecutorService executor, SchemaFactory<ReviewDb> reviewDb) {
+ super(executor, REVIEWDB);
this.reviewDb = reviewDb;
}
@Override
protected Result doCheck() throws Exception {
- try (ReviewDb db = reviewDb.get()) {
+ try (ReviewDb db = reviewDb.open()) {
db.schemaVersion().get(new CurrentSchemaVersion.Key());
return Result.PASSED;
}
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 f5c6036..86a0965 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
@@ -21,6 +21,7 @@
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gson.Gson;
import com.google.gson.JsonObject;
@@ -28,6 +29,7 @@
import org.junit.Test;
@TestPlugin(name = "healthcheck", sysModule = "com.googlesource.gerrit.plugins.healthcheck.Module")
+@Sandboxed
public class HealthCheckIT extends LightweightPluginDaemonTest {
Gson gson = new Gson();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java
index 677d22a..990b4cb 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java
@@ -17,11 +17,14 @@
import static com.google.common.truth.Truth.assertThat;
import static org.eclipse.jgit.lib.RefUpdate.Result.NEW;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.RepositoryCaseMismatchException;
import com.google.gerrit.testutil.InMemoryRepositoryManager;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
import com.googlesource.gerrit.plugins.healthcheck.check.JGitHealthCheck;
import java.io.IOException;
@@ -41,10 +44,14 @@
public class JGitHealthCheckTest {
private AllProjectsName allProjectsName = new AllProjectsName("All-Projects");
private InMemoryRepositoryManager inMemoryRepositoryManager = new InMemoryRepositoryManager();
- PersonIdent personIdent = new PersonIdent("Gerrit Rietveld", "gerrit@rietveld.nl");
+ private PersonIdent personIdent = new PersonIdent("Gerrit Rietveld", "gerrit@rietveld.nl");
+
+ @Inject private ListeningExecutorService executor;
@Before
public void setupAllProjects() throws Exception {
+ Guice.createInjector(new HealthCheckModule()).injectMembers(this);
+
InMemoryRepositoryManager.Repo repo =
inMemoryRepositoryManager.createRepository(allProjectsName);
createCommit(repo, "refs/meta/config");
@@ -53,14 +60,14 @@
@Test
public void shouldBeHealthyWhenJGitIsWorking() {
JGitHealthCheck reviewDbCheck =
- new JGitHealthCheck(getWorkingRepositoryManager(), allProjectsName);
+ new JGitHealthCheck(executor, getWorkingRepositoryManager(), allProjectsName);
assertThat(reviewDbCheck.run().result).isEqualTo(Result.PASSED);
}
@Test
public void shouldBeUnhealthyWhenJGitIsFailing() {
JGitHealthCheck jGitHealthCheck =
- new JGitHealthCheck(getFailingGitRepositoryManager(), allProjectsName);
+ new JGitHealthCheck(executor, getFailingGitRepositoryManager(), allProjectsName);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
index 668dbdf..a45ea32 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
@@ -16,29 +16,50 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.server.project.ListProjects;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.healthcheck.check.AbstractHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
import com.googlesource.gerrit.plugins.healthcheck.check.ProjectsListHealthCheck;
import java.util.SortedMap;
import java.util.TreeMap;
+import org.junit.Before;
import org.junit.Test;
public class ProjectsListHealthCheckTest {
+ @Inject private ListeningExecutorService executor;
+
+ @Before
+ public void setUp() {
+ Guice.createInjector(new HealthCheckModule()).injectMembers(this);
+ }
@Test
public void shouldBeHealthyWhenListProjectsWorks() {
- ProjectsListHealthCheck jGitHealthCheck = new ProjectsListHealthCheck(getWorkingProjectList());
+ ProjectsListHealthCheck jGitHealthCheck =
+ new ProjectsListHealthCheck(executor, getWorkingProjectList(0));
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.PASSED);
}
@Test
public void shouldBeUnhealthyWhenListProjectsIsFailing() {
- ProjectsListHealthCheck jGitHealthCheck = new ProjectsListHealthCheck(getFailingProjectList());
+ ProjectsListHealthCheck jGitHealthCheck =
+ new ProjectsListHealthCheck(executor, getFailingProjectList());
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED);
}
+ @Test
+ public void shouldBeUnhealthyWhenListProjectsIsTimingOut() {
+ ProjectsListHealthCheck jGitHealthCheck =
+ new ProjectsListHealthCheck(
+ executor, getWorkingProjectList(AbstractHealthCheck.CHECK_TIMEOUT * 2));
+ assertThat(jGitHealthCheck.run().result).isEqualTo(Result.TIMEOUT);
+ }
+
private ListProjects getFailingProjectList() {
return new ListProjects(null, null, null, null, null, null, null, null) {
@Override
@@ -48,12 +69,17 @@
};
}
- private ListProjects getWorkingProjectList() {
+ private ListProjects getWorkingProjectList(long execTime) {
return new ListProjects(null, null, null, null, null, null, null, null) {
@Override
public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
SortedMap<String, ProjectInfo> projects = new TreeMap<>();
projects.put("testproject", new ProjectInfo());
+ try {
+ Thread.sleep(execTime);
+ } catch (InterruptedException e) {
+ throw new IllegalStateException(e);
+ }
return projects;
}
};
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ReviewDbHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ReviewDbHealthCheckTest.java
index aae2082..1e5d712 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ReviewDbHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ReviewDbHealthCheckTest.java
@@ -16,52 +16,49 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.testutil.DisabledReviewDb;
import com.google.gerrit.testutil.InMemoryDatabase;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Provider;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.ReviewDbHealthCheck;
+import org.junit.Before;
import org.junit.Test;
public class ReviewDbHealthCheckTest {
+ private SchemaFactory<ReviewDb> workingReviewDbFactory;
+
+ @Inject private ListeningExecutorService executor;
+
+ @Before
+ public void setUp() throws Exception {
+ Guice.createInjector(new HealthCheckModule()).injectMembers(this);
+ workingReviewDbFactory = InMemoryDatabase.newDatabase(new LifecycleManager()).create();
+ }
@Test
- public void shouldBeHealthyWhenReviewDbIsWorking() throws OrmException {
- ReviewDbHealthCheck reviewDbCheck = new ReviewDbHealthCheck(getWorkingReviewDbProvider());
+ public void shouldBeHealthyWhenReviewDbIsWorking() {
+ ReviewDbHealthCheck reviewDbCheck = new ReviewDbHealthCheck(executor, workingReviewDbFactory);
assertThat(reviewDbCheck.run().result).isEqualTo(HealthCheck.Result.PASSED);
}
@Test
public void shouldBeUnhealthyWhenReviewDbIsFailing() {
- ReviewDbHealthCheck reviewDbCheck = new ReviewDbHealthCheck(getFailingReviewDbProvider());
+ ReviewDbHealthCheck reviewDbCheck =
+ new ReviewDbHealthCheck(executor, getFailingReviewDbProvider());
assertThat(reviewDbCheck.run().result).isEqualTo(HealthCheck.Result.FAILED);
}
- private Provider<ReviewDb> getFailingReviewDbProvider() {
- return new Provider<ReviewDb>() {
+ private SchemaFactory<ReviewDb> getFailingReviewDbProvider() {
+ return new SchemaFactory<ReviewDb>() {
@Override
- public ReviewDb get() {
+ public ReviewDb open() {
return new DisabledReviewDb();
}
};
}
-
- private Provider<ReviewDb> getWorkingReviewDbProvider() throws OrmException {
- InMemoryDatabase inMemoryDatabase =
- InMemoryDatabase.newDatabase(new LifecycleManager()).create();
- return new Provider<ReviewDb>() {
- @Override
- public ReviewDb get() {
- try {
- return inMemoryDatabase.open();
- } catch (OrmException e) {
- e.printStackTrace();
- return null;
- }
- }
- };
- }
}
