Fix BlockedThreadsCheck failing with un-blocked threads The introduction of BlockedThreadsCheck introduced a P0 regression because of the accumulatio of the counter of blocked threads caused any Gerrit instance to eventually fail once the counter of blocked threads reached the 50% of the total number. Remove the @Singleton annotation misplaced on the check and make sure that similar issues aren't passed unnoticed by using the actual Guice injection for creating the BlockedThreadsCheck in tests rather than calling the constructor directly. Bug: Issue 14736 Change-Id: I430a7a576f8c41594c59d71cced6d43d7acf985a
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java index f2cbb70..4e21771 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java
@@ -23,7 +23,6 @@ import com.google.inject.Inject; import com.google.inject.Module; import com.google.inject.Provider; -import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.lang.management.ManagementFactory; @@ -35,7 +34,6 @@ import java.util.function.Supplier; import java.util.stream.Stream; -@Singleton public class BlockedThreadsCheck extends AbstractHealthCheck { public static Module SUB_CHECKS = new FactoryModule() {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java index 641a060..c52d646 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java
@@ -93,6 +93,27 @@ } @Test + public void shouldFailThenPassCheck() { + String prefix = "some-prefix"; + + List<ThreadInfo> failingThreadInfo = new ArrayList<>(); + failingThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.RUNNABLE, prefix))); + failingThreadInfo.addAll(nCopies(3, mockInfo(Thread.State.BLOCKED, prefix))); + + List<ThreadInfo> successThreadInfo = new ArrayList<>(); + successThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.RUNNABLE, prefix))); + successThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.BLOCKED, prefix))); + + when(beanMock.getThreadInfo(null, 0)) + .thenReturn( + failingThreadInfo.toArray(new ThreadInfo[0]), + successThreadInfo.toArray(new ThreadInfo[0])); + + assertThat(createCheck().run().result).isEqualTo(Result.FAILED); + assertThat(createCheck().run().result).isEqualTo(Result.PASSED); + } + + @Test public void shouldPassCheckWhenBlockedThreadsAreLessThenThreshold() { int running = 3; int blocked = 1;