Skip to content

Fix line length for monitor and remove suppressions #37456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 15, 2019
Merged

Fix line length for monitor and remove suppressions #37456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e935b5
Fix line length for `monitor` and remove suppressions.
dliappis Jan 14, 2019
12d1de2
Addressing PR comment
dliappis Jan 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions buildSrc/src/main/resources/checkstyle_suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
12 changes: 8 additions & 4 deletions server/src/main/java/org/elasticsearch/monitor/jvm/GcNames.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions server/src/main/java/org/elasticsearch/monitor/jvm/HotThreads.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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]));
}
Expand Down
24 changes: 16 additions & 8 deletions server/src/test/java/org/elasticsearch/monitor/jvm/JvmGcMonitorServiceSettingsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down