Skip to content

Add null check for G1 related options and origin field #93197

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

Closed
DmitryNekrasov wants to merge 1 commit into from
Closed

Add null check for G1 related options and origin field #93197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 10 additions & 3 deletions distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmErgonomics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,17 @@ static boolean tuneG1GCForSmallHeap(final long heapSize) {
static boolean tuneG1GCHeapRegion(final Map<String, JvmOption> finalJvmOptions, final boolean tuneG1GCForSmallHeap) {
JvmOption g1GCHeapRegion = finalJvmOptions.get("G1HeapRegionSize");
JvmOption g1GC = finalJvmOptions.get("UseG1GC");
return (tuneG1GCForSmallHeap && g1GC.getMandatoryValue().equals("true") && g1GCHeapRegion.isCommandLineOrigin() == false);
return (tuneG1GCForSmallHeap
&& g1GC != null
&& g1GC.getMandatoryValue().equals("true")
&& g1GCHeapRegion != null
&& g1GCHeapRegion.isCommandLineOrigin() == false);
}

static int tuneG1GCReservePercent(final Map<String, JvmOption> finalJvmOptions, final boolean tuneG1GCForSmallHeap) {
JvmOption g1GC = finalJvmOptions.get("UseG1GC");
JvmOption g1GCReservePercent = finalJvmOptions.get("G1ReservePercent");
if (g1GC.getMandatoryValue().equals("true")) {
if (g1GC != null && g1GC.getMandatoryValue().equals("true") && g1GCReservePercent != null) {
if (g1GCReservePercent.isCommandLineOrigin() == false && tuneG1GCForSmallHeap) {
return 15;
} else if (g1GCReservePercent.isCommandLineOrigin() == false && tuneG1GCForSmallHeap == false) {
Expand All @@ -85,7 +89,10 @@ static int tuneG1GCReservePercent(final Map<String, JvmOption> finalJvmOptions,
static boolean tuneG1GCInitiatingHeapOccupancyPercent(final Map<String, JvmOption> finalJvmOptions) {
JvmOption g1GC = finalJvmOptions.get("UseG1GC");
JvmOption g1GCInitiatingHeapOccupancyPercent = finalJvmOptions.get("InitiatingHeapOccupancyPercent");
return g1GCInitiatingHeapOccupancyPercent.isCommandLineOrigin() == false && g1GC.getMandatoryValue().equals("true");
return g1GCInitiatingHeapOccupancyPercent != null
&& g1GCInitiatingHeapOccupancyPercent.isCommandLineOrigin() == false
&& g1GC != null
&& g1GC.getMandatoryValue().equals("true");
}

private static final Pattern SYSTEM_PROPERTY = Pattern.compile("^-D(?<key>[\\w+].*?)=(?<value>.*)$");
Expand Down
2 changes: 1 addition & 1 deletion distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public String getMandatoryValue() {
}

public boolean isCommandLineOrigin() {
return this.origin.contains("command line");
return this.origin != null && this.origin.contains("command line");
}

private static final Pattern OPTION = Pattern.compile(
Expand Down
15 changes: 15 additions & 0 deletions ...ution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmErgonomicsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,19 @@ public void testMaxDirectMemorySizeChoiceWhenSet() throws Exception {
);
}

public void testTuneG1GCHeapRegionWithEmptyFinalJvmOptionsReturnsFalse() {
Map<String, JvmOption> finalJvmOptions = new HashMap<>();
assertFalse(JvmErgonomics.tuneG1GCHeapRegion(finalJvmOptions, true));
}

public void testTuneG1GCReservePercentWithEmptyFinalJvmOptionsReturnsZero() {
Map<String, JvmOption> finalJvmOptions = new HashMap<>();
assertEquals(JvmErgonomics.tuneG1GCReservePercent(finalJvmOptions, true), 0);
}

public void testTuneG1GCInitiatingHeapOccupancyPercentWithEmptyFinalJvmOptionsReturnsFalse() {
Map<String, JvmOption> finalJvmOptions = new HashMap<>();
assertFalse(JvmErgonomics.tuneG1GCInitiatingHeapOccupancyPercent(finalJvmOptions));
}

}
5 changes: 5 additions & 0 deletions ...on/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,4 +344,9 @@ public void accept(final int lineNumber, final String line) {
assertThat(seenInvalidLines, equalTo(invalidLines));
}

public void testIsCommandLineOriginWithNullOriginReturnsFalse() {
JvmOption jvmOption = new JvmOption("value", null);
assertFalse(jvmOption.isCommandLineOrigin());
}

}
6 changes: 6 additions & 0 deletions docs/changelog/93197.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 93197
summary: Add null check for G1 related options and origin field. This change avoids NPE when starting Elasticsearch on the Azul Platform Prime JDK.
area: Infra/Settings
type: bug
issues:
- 91577