Merge branch 'stable-3.1' * stable-3.1: Add active worker threads check Allow using healthcheck on slaves Reformat with GJF 1.7 Feature: Issue 12240 Change-Id: If3d5de5d7a7a76a1fb09b1a155e36cc130881be9
diff --git a/BUILD b/BUILD index 41f1770..e87843c 100644 --- a/BUILD +++ b/BUILD
@@ -11,7 +11,8 @@ srcs = glob(["src/main/java/**/*.java"]), manifest_entries = [ "Gerrit-PluginName: healthcheck", - "Gerrit-Module: com.googlesource.gerrit.plugins.healthcheck.Module" + "Gerrit-Module: com.googlesource.gerrit.plugins.healthcheck.Module", + "Gerrit-HttpModule: com.googlesource.gerrit.plugins.healthcheck.HttpModule", ], resources = glob(["src/main/resources/**/*"]), )
diff --git a/README.md b/README.md index d5cb3db..e5ad599 100644 --- a/README.md +++ b/README.md
@@ -25,6 +25,9 @@ ## How to install Copy the healthcheck.jar into the Gerrit's /plugins directory and wait for the plugin to be automatically loaded. +The healthcheck plugin is compatible with both Gerrit master and slave setups. The only difference to bear in mind +is that some checks may not be successful on slaves (e.g. query changes) because the associated subsystem is switched +off. ## How to use
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 d078857..fc6acc7 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/HttpModule.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HttpModule.java new file mode 100644 index 0000000..c62a261 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HttpModule.java
@@ -0,0 +1,42 @@ +// 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 com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.httpd.AllRequestFilter; +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.inject.Inject; +import com.google.inject.Scopes; +import com.google.inject.servlet.ServletModule; +import com.googlesource.gerrit.plugins.healthcheck.filter.HealthCheckStatusFilter; +import org.eclipse.jgit.lib.Config; + +public class HttpModule extends ServletModule { + private boolean isSlave; + + @Inject + public HttpModule(@GerritServerConfig Config gerritConfig) { + isSlave = gerritConfig.getBoolean("container", "slave", false); + } + + @Override + protected void configureServlets() { + if (isSlave) { + DynamicSet.bind(binder(), AllRequestFilter.class) + .to(HealthCheckStatusFilter.class) + .in(Scopes.SINGLETON); + } + } +}
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/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java new file mode 100644 index 0000000..e19f18a --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.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.filter; + +import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.httpd.AllRequestFilter; +import com.google.gerrit.httpd.restapi.RestApiServlet; +import com.google.gerrit.json.OutputFormat; +import com.google.gerrit.server.config.ConfigResource; +import com.google.gson.Gson; +import com.google.inject.Inject; +import com.googlesource.gerrit.plugins.healthcheck.api.HealthCheckStatusEndpoint; +import java.io.IOException; +import java.io.PrintWriter; +import java.util.Map; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class HealthCheckStatusFilter extends AllRequestFilter { + private final HealthCheckStatusEndpoint statusEndpoint; + private final Gson gson; + + @Inject + public HealthCheckStatusFilter(HealthCheckStatusEndpoint statusEndpoint) { + this.statusEndpoint = statusEndpoint; + this.gson = OutputFormat.JSON.newGsonBuilder().create(); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) { + chain.doFilter(request, response); + return; + } + + HttpServletResponse httpResponse = (HttpServletResponse) response; + HttpServletRequest httpRequest = (HttpServletRequest) request; + + if (isStatusCheck(httpRequest)) { + doStatusCheck(httpResponse); + } else { + chain.doFilter(request, response); + } + } + + private boolean isStatusCheck(HttpServletRequest httpServletRequest) { + return httpServletRequest.getRequestURI().equals("/config/server/healthcheck~status"); + } + + private void doStatusCheck(HttpServletResponse httpResponse) throws ServletException { + try { + Response<Map<String, Object>> healthStatus = + (Response<Map<String, Object>>) statusEndpoint.apply(new ConfigResource()); + String healthStatusJson = gson.toJson(healthStatus); + if (healthStatus.statusCode() == HttpServletResponse.SC_OK) { + PrintWriter writer = httpResponse.getWriter(); + writer.print(new String(RestApiServlet.JSON_MAGIC)); + writer.print(healthStatusJson); + } else { + httpResponse.sendError(healthStatus.statusCode(), healthStatusJson); + } + } catch (Exception e) { + throw new ServletException(e); + } + } +}
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"); }