Merge branch 'stable-2.16' into stable-3.0
* stable-2.16:
Add active worker threads check
Change-Id: If48611076b0973f99aa880d51bdb3d2c78da514a
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
index 4e4fab1..a19c934 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
@@ -40,6 +40,7 @@
private static final long HEALTHCHECK_TIMEOUT_DEFAULT = 500L;
private static final String QUERY_DEFAULT = "status:open";
private static final int LIMIT_DEFAULT = 10;
+ private static final int ACTIVE_WORKERS_THRESHOLD_DEFAULT = 80;
private static final String USERNAME_DEFAULT = "healthcheck";
private static final String PASSWORD_DEFAULT = "";
private static final boolean HEALTH_CHECK_ENABLED_DEFAULT = true;
@@ -97,6 +98,14 @@
return config.getInt(HEALTHCHECK, healthCheckName, "limit", defaultLimit);
}
+ public int getActiveWorkersThreshold(String healthCheckName) {
+ int defaultThreshold =
+ healthCheckName == null
+ ? ACTIVE_WORKERS_THRESHOLD_DEFAULT
+ : getActiveWorkersThreshold(null);
+ return config.getInt(HEALTHCHECK, healthCheckName, "threshold", defaultThreshold);
+ }
+
public Set<Project.NameKey> getJGITRepositories(String healthCheckName) {
Set<Project.NameKey> repos =
Stream.of(config.getStringList(HEALTHCHECK, healthCheckName, "project"))
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 a7eb01a..e64b188 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
@@ -17,6 +17,7 @@
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.inject.AbstractModule;
+import com.googlesource.gerrit.plugins.healthcheck.check.ActiveWorkersCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.AuthHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.JGitHealthCheck;
@@ -31,6 +32,7 @@
bindChecker(ProjectsListHealthCheck.class);
bindChecker(QueryChangesHealthCheck.class);
bindChecker(AuthHealthCheck.class);
+ bindChecker(ActiveWorkersCheck.class);
bind(LifecycleListener.class).to(HealthCheckMetrics.class);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java
new file mode 100644
index 0000000..f3afc99
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java
@@ -0,0 +1,83 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.healthcheck.check;
+
+import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.ACTIVEWORKERS;
+
+import com.codahale.metrics.MetricRegistry;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
+import java.util.Optional;
+import org.eclipse.jgit.lib.Config;
+
+public class ActiveWorkersCheck extends AbstractHealthCheck {
+
+ public static final String ACTIVE_WORKERS_METRIC_NAME =
+ "queue/ssh_interactive_worker/active_threads";
+
+ private Integer threshold;
+ private Integer interactiveThreadsMaxPoolSize;
+ private MetricRegistry metricRegistry;
+
+ @Inject
+ public ActiveWorkersCheck(
+ @GerritServerConfig Config gerritConfig,
+ ListeningExecutorService executor,
+ HealthCheckConfig healthCheckConfig,
+ ThreadSettingsConfig threadSettingsConfig,
+ MetricRegistry metricRegistry) {
+ super(executor, healthCheckConfig, ACTIVEWORKERS);
+ this.threshold = healthCheckConfig.getActiveWorkersThreshold(ACTIVEWORKERS);
+ this.metricRegistry = metricRegistry;
+ this.interactiveThreadsMaxPoolSize =
+ getInteractiveThreadsMaxPoolSize(threadSettingsConfig, gerritConfig);
+ }
+
+ @Override
+ protected Result doCheck() throws Exception {
+ return Optional.ofNullable(metricRegistry.getGauges().get(ACTIVE_WORKERS_METRIC_NAME))
+ .map(
+ metric -> {
+ float currentInteractiveThreadsPercentage =
+ ((long) metric.getValue() * 100) / interactiveThreadsMaxPoolSize;
+ return (currentInteractiveThreadsPercentage <= threshold)
+ ? Result.PASSED
+ : Result.FAILED;
+ })
+ .orElse(Result.PASSED);
+ }
+
+ /**
+ * This method is following logic from com.google.gerrit.sshd.CommandExecutorQueueProvider
+ *
+ * <p>We are not able to use "queue/ssh_interactive_worker/max_pool_size" metric because in
+ * current implementation it's always returning Integer.MAX_VALUE.
+ *
+ * @return max number of allowed threads in interactive work queue
+ */
+ private Integer getInteractiveThreadsMaxPoolSize(
+ ThreadSettingsConfig threadSettingsConfig, Config gerritConfig) {
+ int poolSize = threadSettingsConfig.getSshdThreads();
+ int batchThreads =
+ gerritConfig.getInt("sshd", "batchThreads", threadSettingsConfig.getSshdBatchTreads());
+ if (batchThreads > poolSize) {
+ poolSize += batchThreads;
+ }
+ return Math.max(1, poolSize - batchThreads);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheckNames.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheckNames.java
index 143d2b1..4ce705d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheckNames.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheckNames.java
@@ -19,4 +19,5 @@
String PROJECTSLIST = "projectslist";
String QUERYCHANGES = "querychanges";
String AUTH = "auth";
+ String ACTIVEWORKERS = "activeworkers";
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 91d1c0a..1d8c912 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -48,6 +48,7 @@
- `jgit` : check connectivity to the filesystem and ability to open a JGit ref and object
- `projectslist` : check the ability to list projects with their descriptions
- `auth`: check the ability to authenticate with username and password
+- `activeworkers`: check the number of active worker threads and the ability to create a new one
Each check name can be disabled by setting the `enabled` parameter to **false**,
by default this parameter is set to **true**
@@ -81,4 +82,9 @@
- `healthcheck.auth.password` : Password to use for authentication
- Default: no password
\ No newline at end of file
+ Default: no password
+
+ - `healthcheck.activeworkers.threshold` : Percent of queue occupancy above which queue is consider
+ as full.
+
+ Default: 80
\ No newline at end of file
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java
new file mode 100644
index 0000000..71bef43
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java
@@ -0,0 +1,161 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.healthcheck;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig.DEFAULT_CONFIG;
+import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.ACTIVEWORKERS;
+
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.MetricRegistry;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.googlesource.gerrit.plugins.healthcheck.check.ActiveWorkersCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+public class ActiveWorkersCheckTest {
+
+ @Test
+ public void shouldPassCheckWhenNoMetric() {
+
+ Injector injector = testInjector(new TestModule(new Config(), new MetricRegistry()));
+
+ ActiveWorkersCheck check = createCheck(injector);
+ assertThat(check.run().result).isEqualTo(Result.PASSED);
+ }
+
+ @Test
+ public void shouldPassCheckWhenNoActiveWorkers() {
+
+ MetricRegistry metricRegistry = createMetricRegistry(0L);
+
+ Injector injector = testInjector(new TestModule(new Config(), metricRegistry));
+
+ ActiveWorkersCheck check = createCheck(injector);
+ assertThat(check.run().result).isEqualTo(Result.PASSED);
+ }
+
+ @Test
+ public void shouldPassCheckWhenActiveWorkersLessThanDefaultThreshold() {
+
+ MetricRegistry metricRegistry = createMetricRegistry(79L);
+ // By default threshold is 80%
+ Config gerritConfig = new Config();
+ gerritConfig.setInt("sshd", null, "threads", 100);
+ Injector injector = testInjector(new TestModule(gerritConfig, metricRegistry));
+
+ ActiveWorkersCheck check = createCheck(injector);
+ assertThat(check.run().result).isEqualTo(Result.PASSED);
+ }
+
+ @Test
+ public void shouldFailCheckWhenActiveWorkersMoreThanDefaultThreshold() {
+
+ MetricRegistry metricRegistry = createMetricRegistry(90L);
+ // By default threshold is 80%
+ Config gerritConfig = new Config();
+ gerritConfig.setInt("sshd", null, "threads", 100);
+
+ Injector injector = testInjector(new TestModule(gerritConfig, metricRegistry));
+
+ ActiveWorkersCheck check = createCheck(injector);
+ assertThat(check.run().result).isEqualTo(Result.FAILED);
+ }
+
+ @Test
+ public void shouldFailCheckWhenActiveWorkersMoreThanThreshold() {
+
+ MetricRegistry metricRegistry = createMetricRegistry(6L);
+
+ Config gerritConfig = new Config();
+ gerritConfig.setInt("sshd", null, "threads", 12);
+
+ Injector injector = testInjector(new TestModule(gerritConfig, metricRegistry));
+
+ HealthCheckConfig healthCheckConfig =
+ new HealthCheckConfig("[healthcheck \"" + ACTIVEWORKERS + "\"]\n" + " threshold = 50");
+ ActiveWorkersCheck check = createCheck(injector, healthCheckConfig);
+ assertThat(check.run().result).isEqualTo(Result.FAILED);
+ }
+
+ @Test
+ public void shouldPassCheckWhenActiveWorkersLessThanThreshold() {
+
+ MetricRegistry metricRegistry = createMetricRegistry(5L);
+
+ Config gerritConfig = new Config();
+ gerritConfig.setInt("sshd", null, "threads", 12);
+
+ Injector injector = testInjector(new TestModule(gerritConfig, metricRegistry));
+
+ HealthCheckConfig healthCheckConfig =
+ new HealthCheckConfig("[healthcheck \"" + ACTIVEWORKERS + "\"]\n" + " threshold = 50");
+ ActiveWorkersCheck check = createCheck(injector, healthCheckConfig);
+ assertThat(check.run().result).isEqualTo(Result.PASSED);
+ }
+
+ private Injector testInjector(AbstractModule testModule) {
+ return Guice.createInjector(new HealthCheckModule(), testModule);
+ }
+
+ private MetricRegistry createMetricRegistry(Long value) {
+ MetricRegistry metricRegistry = new MetricRegistry();
+ metricRegistry.register(
+ ActiveWorkersCheck.ACTIVE_WORKERS_METRIC_NAME,
+ new Gauge<Long>() {
+ @Override
+ public Long getValue() {
+ return value;
+ }
+ });
+ return metricRegistry;
+ }
+
+ private ActiveWorkersCheck createCheck(Injector injector) {
+ return createCheck(injector, DEFAULT_CONFIG);
+ }
+
+ private ActiveWorkersCheck createCheck(Injector injector, HealthCheckConfig healtchCheckConfig) {
+ return new ActiveWorkersCheck(
+ new Config(),
+ injector.getInstance(ListeningExecutorService.class),
+ healtchCheckConfig,
+ injector.getInstance(ThreadSettingsConfig.class),
+ injector.getInstance(MetricRegistry.class));
+ }
+
+ private class TestModule extends AbstractModule {
+ Config gerritConfig;
+ MetricRegistry metricRegistry;
+
+ public TestModule(Config gerritConfig, MetricRegistry metricRegistry) {
+ this.gerritConfig = gerritConfig;
+ this.metricRegistry = metricRegistry;
+ }
+
+ @Override
+ protected void configure() {
+ bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(gerritConfig);
+ bind(ThreadSettingsConfig.class);
+ bind(MetricRegistry.class).toInstance(metricRegistry);
+ }
+ }
+}
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 148f8f5..199543b 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
@@ -16,6 +16,7 @@
import static com.google.common.net.HttpHeaders.CONTENT_TYPE;
import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.ACTIVEWORKERS;
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.AUTH;
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.JGIT;
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.QUERYCHANGES;
@@ -123,6 +124,34 @@
assertCheckResult(getResponseJson(resp), QUERYCHANGES, "passed");
}
+ @Test
+ public void shouldReturnActiveWorkersCheck() throws Exception {
+ createChange("refs/for/master");
+ RestResponse resp = getHealthCheckStatus();
+ resp.assertOK();
+
+ assertCheckResult(getResponseJson(resp), ACTIVEWORKERS, "passed");
+ }
+
+ @Test
+ public void shouldReturnActiveWorkersCheckAsDisabled() throws Exception {
+ disableCheck(ACTIVEWORKERS);
+ RestResponse resp = getHealthCheckStatus();
+
+ resp.assertOK();
+ assertCheckResult(getResponseJson(resp), ACTIVEWORKERS, "disabled");
+ }
+
+ @Test
+ public void shouldReturnActiveWorkersMultipleTimesCheck() throws Exception {
+ createChange("refs/for/master");
+ getHealthCheckStatus();
+ RestResponse resp = getHealthCheckStatus();
+ resp.assertOK();
+
+ assertCheckResult(getResponseJson(resp), ACTIVEWORKERS, "passed");
+ }
+
private RestResponse getHealthCheckStatus() throws IOException {
return adminRestSession.get("/config/server/healthcheck~status");
}
