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;
-        }
-      }
-    };
-  }
 }