Do not use QueryProcessor multiple times
The QueryProcessor object is non-threadsafe and it should not be reused.
This is been enforced with runtime state checks from stable-2.15 onwards.
Rather than using the same object through multiple requests, the
QueryChanges healthcheck now will be instantiated with a _provider_ of
QueryChanges, so that a new QueryProcessor is used each time the
healthcheck is executed.
Change-Id: Ifa4e621eaf87a409c428a618e2a7f2dcd0bb2ca7
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java
index ce6b168..a8e4d2e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java
@@ -21,6 +21,7 @@
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
import java.util.List;
@@ -30,7 +31,8 @@
@Singleton
public class QueryChangesHealthCheck extends AbstractHealthCheck {
private static final Logger log = LoggerFactory.getLogger(QueryChangesHealthCheck.class);
- private final QueryChanges queryChanges;
+ private final Provider<QueryChanges> queryChangesProvider;
+ private final HealthCheckConfig config;
private final int limit;
private final OneOffRequestContext oneOffCtx;
@@ -38,20 +40,25 @@
public QueryChangesHealthCheck(
ListeningExecutorService executor,
HealthCheckConfig config,
- QueryChanges queryChanges,
+ Provider<QueryChanges> queryChangesProvider,
OneOffRequestContext oneOffCtx) {
super(executor, config, QUERYCHANGES);
- this.queryChanges = queryChanges;
+ this.queryChangesProvider = queryChangesProvider;
+ this.config = config;
this.limit = config.getLimit(QUERYCHANGES);
- queryChanges.setLimit(limit);
- queryChanges.addQuery(config.getQuery(QUERYCHANGES));
- queryChanges.setStart(0);
+
this.oneOffCtx = oneOffCtx;
}
@Override
protected Result doCheck() throws Exception {
try (ManualRequestContext ctx = oneOffCtx.open()) {
+
+ QueryChanges queryChanges = this.queryChangesProvider.get();
+ queryChanges.setLimit(limit);
+ queryChanges.addQuery(config.getQuery(QUERYCHANGES));
+ queryChanges.setStart(0);
+
List<?> changes = queryChanges.apply(null);
if (changes == null) {
log.warn("Cannot query changes: received a null list of results");
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 38f807e..2c63ed7 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
@@ -102,6 +102,18 @@
assertCheckResult(respPayload, QUERYCHANGES, "passed");
}
+ @Test
+ public void shouldReturnQueryChangesMultipleTimesCheck() throws Exception {
+ createChange("refs/for/master");
+ getHealthCheckStatus();
+ RestResponse resp = getHealthCheckStatus();
+ resp.assertOK();
+
+ JsonObject respPayload = gson.fromJson(resp.getReader(), JsonObject.class);
+
+ assertCheckResult(respPayload, QUERYCHANGES, "passed");
+ }
+
private RestResponse getHealthCheckStatus() throws IOException {
return adminRestSession.get("/config/server/healthcheck~status");
}
