Fix line length for monitor and remove suppressions (#37456)

Relates #34884
elastic · Jan 15, 2019 · 19fc59f · 19fc59f
1 parent 23ae980
commit 19fc59f
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 17 deletions.
diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml
@@ -48,11 +48,8 @@
 
   <!-- Hopefully temporary suppression of LineLength on files that don't pass it. We should remove these when we the
     files start to pass. -->
-  <suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]monitor[/\\]jvm[/\\]GcNames.java" checks="LineLength" />
-  <suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]monitor[/\\]jvm[/\\]HotThreads.java" checks="LineLength" />
   <suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]node[/\\]Node.java" checks="LineLength" />
   <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]aliases[/\\]IndexAliasesIT.java" checks="LineLength" />
-  <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]monitor[/\\]jvm[/\\]JvmGcMonitorServiceSettingsTests.java" checks="LineLength" />
 
   <!-- Gradle requires inputs to be seriablizable -->
   <suppress files="buildSrc[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]gradle[/\\]precommit[/\\]TestingConventionRule.java" checks="RegexpSinglelineJava" />

diff --git a/server/src/main/java/org/elasticsearch/monitor/jvm/GcNames.java b/server/src/main/java/org/elasticsearch/monitor/jvm/GcNames.java
@@ -29,13 +29,16 @@ public class GcNames {
      * Resolves the GC type by its memory pool name ({@link java.lang.management.MemoryPoolMXBean#getName()}.
      */
     public static String getByMemoryPoolName(String poolName, String defaultName) {
-        if ("Eden Space".equals(poolName) || "PS Eden Space".equals(poolName) || "Par Eden Space".equals(poolName) || "G1 Eden Space".equals(poolName)) {
+        if ("Eden Space".equals(poolName) || "PS Eden Space".equals(poolName)
+            || "Par Eden Space".equals(poolName) || "G1 Eden Space".equals(poolName)) {
             return YOUNG;
         }
-        if ("Survivor Space".equals(poolName) || "PS Survivor Space".equals(poolName) || "Par Survivor Space".equals(poolName) || "G1 Survivor Space".equals(poolName)) {
+        if ("Survivor Space".equals(poolName) || "PS Survivor Space".equals(poolName)
+            || "Par Survivor Space".equals(poolName) || "G1 Survivor Space".equals(poolName)) {
             return SURVIVOR;
         }
-        if ("Tenured Gen".equals(poolName) || "PS Old Gen".equals(poolName) || "CMS Old Gen".equals(poolName) || "G1 Old Gen".equals(poolName)) {
+        if ("Tenured Gen".equals(poolName) || "PS Old Gen".equals(poolName)
+            || "CMS Old Gen".equals(poolName) || "G1 Old Gen".equals(poolName)) {
             return OLD;
         }
         return defaultName;
@@ -45,7 +48,8 @@ public static String getByGcName(String gcName, String defaultName) {
         if ("Copy".equals(gcName) || "PS Scavenge".equals(gcName) || "ParNew".equals(gcName) || "G1 Young Generation".equals(gcName)) {
             return YOUNG;
         }
-        if ("MarkSweepCompact".equals(gcName) || "PS MarkSweep".equals(gcName) || "ConcurrentMarkSweep".equals(gcName) || "G1 Old Generation".equals(gcName)) {
+        if ("MarkSweepCompact".equals(gcName) || "PS MarkSweep".equals(gcName)
+            || "ConcurrentMarkSweep".equals(gcName) || "G1 Old Generation".equals(gcName)) {
             return OLD;
         }
         return defaultName;

diff --git a/server/src/main/java/org/elasticsearch/monitor/jvm/HotThreads.java b/server/src/main/java/org/elasticsearch/monitor/jvm/HotThreads.java
@@ -234,7 +234,8 @@ private String innerDetect() throws Exception {
                 continue; // thread is not alive yet or died before the first snapshot - ignore it!
             }
             double percent = (((double) time) / interval.nanos()) * 100;
-            sb.append(String.format(Locale.ROOT, "%n%4.1f%% (%s out of %s) %s usage by thread '%s'%n", percent, TimeValue.timeValueNanos(time), interval, type, threadName));
+            sb.append(String.format(Locale.ROOT, "%n%4.1f%% (%s out of %s) %s usage by thread '%s'%n",
+                percent, TimeValue.timeValueNanos(time), interval, type, threadName));
             // for each snapshot (2nd array index) find later snapshot for same thread with max number of
             // identical StackTraceElements (starting from end of each)
             boolean[] done = new boolean[threadElementsSnapshotCount];
@@ -267,7 +268,8 @@ private String innerDetect() throws Exception {
                             sb.append(String.format(Locale.ROOT, "    %s%n", show[l]));
                         }
                     } else {
-                        sb.append(String.format(Locale.ROOT, "  %d/%d snapshots sharing following %d elements%n", count, threadElementsSnapshotCount, maxSim));
+                        sb.append(String.format(Locale.ROOT, "  %d/%d snapshots sharing following %d elements%n",
+                            count, threadElementsSnapshotCount, maxSim));
                         for (int l = show.length - maxSim; l < show.length; l++) {
                             sb.append(String.format(Locale.ROOT, "    %s%n", show[l]));
                         }

diff --git a/server/src/test/java/org/elasticsearch/monitor/jvm/JvmGcMonitorServiceSettingsTests.java b/server/src/test/java/org/elasticsearch/monitor/jvm/JvmGcMonitorServiceSettingsTests.java
@@ -65,7 +65,8 @@ public void testNegativeSetting() throws InterruptedException {
         Settings settings = Settings.builder().put("monitor.jvm.gc.collector." + collector + ".warn", "-" + randomTimeValue()).build();
         execute(settings, (command, interval, name) -> null, e -> {
             assertThat(e, instanceOf(IllegalArgumentException.class));
-            assertThat(e.getMessage(), allOf(containsString("invalid gc_threshold"), containsString("for [monitor.jvm.gc.collector." + collector + ".")));
+            assertThat(e.getMessage(), allOf(containsString("invalid gc_threshold"),
+                containsString("for [monitor.jvm.gc.collector." + collector + ".")));
         }, true, null);
     }
 
@@ -78,8 +79,9 @@ public void testMissingSetting() throws InterruptedException {
         Settings.Builder builder = Settings.builder();
 
         // drop a random setting or two
-        for (@SuppressWarnings("unchecked") AbstractMap.SimpleEntry<String, String> entry : randomSubsetOf(randomIntBetween(1, 2), entries.toArray(new AbstractMap.SimpleEntry[0]))) {
-            builder.put(entry.getKey(), entry.getValue());
+        for (@SuppressWarnings("unchecked") AbstractMap.SimpleEntry<String, String> entry : randomSubsetOf(randomIntBetween(1, 2),
+            entries.toArray(new AbstractMap.SimpleEntry[0]))) {
+                builder.put(entry.getKey(), entry.getValue());
         }
 
         // we should get an exception that a setting is missing
@@ -115,25 +117,31 @@ public void testIllegalOverheadSettings() throws InterruptedException {
         infoWarnOutOfOrderBuilder.put("monitor.jvm.gc.overhead.warn", warn);
         execute(infoWarnOutOfOrderBuilder.build(), (command, interval, name) -> null, e -> {
             assertThat(e, instanceOf(IllegalArgumentException.class));
-            assertThat(e.getMessage(), containsString("[monitor.jvm.gc.overhead.warn] must be greater than [monitor.jvm.gc.overhead.info] [" + info + "] but was [" + warn + "]"));
+            assertThat(e.getMessage(), containsString("[monitor.jvm.gc.overhead.warn] must be greater than "
+                + "[monitor.jvm.gc.overhead.info] [" + info + "] but was [" + warn + "]"));
         }, true, null);
 
         final Settings.Builder debugInfoOutOfOrderBuilder = Settings.builder();
         debugInfoOutOfOrderBuilder.put("monitor.jvm.gc.overhead.info", info);
         final int debug = randomIntBetween(info + 1, 99);
         debugInfoOutOfOrderBuilder.put("monitor.jvm.gc.overhead.debug", debug);
-        debugInfoOutOfOrderBuilder.put("monitor.jvm.gc.overhead.warn", randomIntBetween(debug + 1, 100)); // or the test will fail for the wrong reason
+        debugInfoOutOfOrderBuilder.put("monitor.jvm.gc.overhead.warn",
+            randomIntBetween(debug + 1, 100)); // or the test will fail for the wrong reason
         execute(debugInfoOutOfOrderBuilder.build(), (command, interval, name) -> null, e -> {
             assertThat(e, instanceOf(IllegalArgumentException.class));
-            assertThat(e.getMessage(), containsString("[monitor.jvm.gc.overhead.info] must be greater than [monitor.jvm.gc.overhead.debug] [" + debug + "] but was [" + info + "]"));
+            assertThat(e.getMessage(), containsString("[monitor.jvm.gc.overhead.info] must be greater than "
+                + "[monitor.jvm.gc.overhead.debug] [" + debug + "] but was [" + info + "]"));
         }, true, null);
     }
 
-    private static void execute(Settings settings, TriFunction<Runnable, TimeValue, String, Cancellable> scheduler, Runnable asserts) throws InterruptedException {
+    private static void execute(Settings settings, TriFunction<Runnable, TimeValue, String, Cancellable> scheduler,
+                                Runnable asserts) throws InterruptedException {
         execute(settings, scheduler, null, false, asserts);
     }
 
-    private static void execute(Settings settings, TriFunction<Runnable, TimeValue, String, Cancellable> scheduler, Consumer<Throwable> consumer, boolean constructionShouldFail, Runnable asserts) throws InterruptedException {
+    private static void execute(Settings settings, TriFunction<Runnable, TimeValue, String, Cancellable> scheduler,
+                                Consumer<Throwable> consumer, boolean constructionShouldFail,
+                                Runnable asserts) throws InterruptedException {
         assert constructionShouldFail == (consumer != null);
         assert constructionShouldFail == (asserts == null);
         ThreadPool threadPool = null;