Skip to content

Commit

Permalink
Enhance search tier GC options
Browse files Browse the repository at this point in the history
For small nodes, we need a bit more wiggle room for new size
and concurrent GC threads in order to stay below real memory
circuit breaker limits

ES-8087
  • Loading branch information
henningandersen committed Mar 20, 2024
1 parent c30999b commit 77af821
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

package org.elasticsearch.server.cli;

import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.node.NodeRoleSettings;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
Expand All @@ -31,7 +37,7 @@ private JvmErgonomics() {
* @param userDefinedJvmOptions A list of JVM options that have been defined by the user.
* @return A list of additional JVM options to set.
*/
static List<String> choose(final List<String> userDefinedJvmOptions) throws InterruptedException, IOException {
static List<String> choose(final List<String> userDefinedJvmOptions, Settings nodeSettings) throws InterruptedException, IOException {
final List<String> ergonomicChoices = new ArrayList<>();
final Map<String, JvmOption> finalJvmOptions = JvmOption.findFinalOptions(userDefinedJvmOptions);
final long heapSize = JvmOption.extractMaxHeapSize(finalJvmOptions);
Expand All @@ -55,6 +61,22 @@ static List<String> choose(final List<String> userDefinedJvmOptions) throws Inte
ergonomicChoices.add("-XX:G1ReservePercent=" + tuneG1GCReservePercent);
}

boolean isSearchTier = NodeRoleSettings.NODE_ROLES_SETTING.get(nodeSettings).contains(DiscoveryNodeRole.SEARCH_ROLE);
// override new percentage on small heaps on search tier to increase chance of staying free of the real memory circuit breaker limit
if (isSearchTier && heapSize < ByteSizeUnit.GB.toBytes(5)) {
ergonomicChoices.add("-XX:+UnlockExperimentalVMOptions");
ergonomicChoices.add("-XX:G1NewSizePercent=10");
}

// for dedicated search, using just 1 conc gc thread is not always enough to keep us below real memory breaker limit
// jvm use (2+processsors) / 4 (for small processor counts), so only affects 4/5 processors (for now)
if (EsExecutors.NODE_PROCESSORS_SETTING.exists(nodeSettings)) {
int allocated = EsExecutors.allocatedProcessors(nodeSettings);
if (allocated >= 4 && allocated <= 5 && isSearchTier) {
ergonomicChoices.add("-XX:ConcGCThreads=2");
}
}

return ergonomicChoices;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ private List<String> jvmOptions(
new OverridableSystemMemoryInfo(substitutedJvmOptions, new DefaultSystemMemoryInfo())
);
substitutedJvmOptions.addAll(machineDependentHeap.determineHeapSettings(config, substitutedJvmOptions));
final List<String> ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions);
final List<String> ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions, args.nodeSettings());
final List<String> systemJvmOptions = SystemJvmOptions.systemJvmOptions(args.nodeSettings(), cliSysprops);

final List<String> apmOptions = APMJvmOptions.apmJvmOptions(args.nodeSettings(), args.secrets(), args.logsDir(), tmpDir);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
package org.elasticsearch.server.cli;

import org.apache.lucene.tests.util.LuceneTestCase.SuppressFileSystems;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.node.NodeRoleSettings;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.ESTestCase.WithoutSecurityManager;

Expand All @@ -26,6 +30,7 @@
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasToString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -106,38 +111,40 @@ public void testExtractSystemProperties() {
}

public void testG1GOptionsForSmallHeap() throws Exception {
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseG1GC"));
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseG1GC"), Settings.EMPTY);
assertThat(jvmErgonomics, hasItem("-XX:G1HeapRegionSize=4m"));
assertThat(jvmErgonomics, hasItem("-XX:InitiatingHeapOccupancyPercent=30"));
assertThat(jvmErgonomics, hasItem("-XX:G1ReservePercent=15"));
}

public void testG1GOptionsForSmallHeapWhenTuningSet() throws Exception {
List<String> jvmErgonomics = JvmErgonomics.choose(
Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseG1GC", "-XX:G1HeapRegionSize=4m", "-XX:InitiatingHeapOccupancyPercent=45")
Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseG1GC", "-XX:G1HeapRegionSize=4m", "-XX:InitiatingHeapOccupancyPercent=45"),
Settings.EMPTY
);
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1HeapRegionSize="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:InitiatingHeapOccupancyPercent="))));
assertThat(jvmErgonomics, hasItem("-XX:G1ReservePercent=15"));
}

public void testG1GOptionsForLargeHeap() throws Exception {
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms8g", "-Xmx8g", "-XX:+UseG1GC"));
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms8g", "-Xmx8g", "-XX:+UseG1GC"), Settings.EMPTY);
assertThat(jvmErgonomics, hasItem("-XX:InitiatingHeapOccupancyPercent=30"));
assertThat(jvmErgonomics, hasItem("-XX:G1ReservePercent=25"));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1HeapRegionSize="))));
}

public void testG1GOptionsForSmallHeapWhenOtherGCSet() throws Exception {
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseParallelGC"));
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseParallelGC"), Settings.EMPTY);
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1HeapRegionSize="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:InitiatingHeapOccupancyPercent="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1ReservePercent="))));
}

public void testG1GOptionsForLargeHeapWhenTuningSet() throws Exception {
List<String> jvmErgonomics = JvmErgonomics.choose(
Arrays.asList("-Xms8g", "-Xmx8g", "-XX:+UseG1GC", "-XX:InitiatingHeapOccupancyPercent=60", "-XX:G1ReservePercent=10")
Arrays.asList("-Xms8g", "-Xmx8g", "-XX:+UseG1GC", "-XX:InitiatingHeapOccupancyPercent=60", "-XX:G1ReservePercent=10"),
Settings.EMPTY
);
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:InitiatingHeapOccupancyPercent="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1ReservePercent="))));
Expand Down Expand Up @@ -168,18 +175,79 @@ public void testMaxDirectMemorySizeChoice() throws Exception {
);
final String heapSize = randomFrom(heapMaxDirectMemorySize.keySet().toArray(String[]::new));
assertThat(
JvmErgonomics.choose(Arrays.asList("-Xms" + heapSize, "-Xmx" + heapSize)),
JvmErgonomics.choose(Arrays.asList("-Xms" + heapSize, "-Xmx" + heapSize), Settings.EMPTY),
hasItem("-XX:MaxDirectMemorySize=" + heapMaxDirectMemorySize.get(heapSize))
);
}

public void testMaxDirectMemorySizeChoiceWhenSet() throws Exception {
assertThat(
JvmErgonomics.choose(Arrays.asList("-Xms1g", "-Xmx1g", "-XX:MaxDirectMemorySize=1g")),
JvmErgonomics.choose(Arrays.asList("-Xms1g", "-Xmx1g", "-XX:MaxDirectMemorySize=1g"), Settings.EMPTY),
everyItem(not(startsWith("-XX:MaxDirectMemorySize=")))
);
}

public void testConcGCThreadsNotSetBasedOnProcessors() throws Exception {
Settings.Builder nodeSettingsBuilder = Settings.builder()
.put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), DiscoveryNodeRole.SEARCH_ROLE.roleName());
if (randomBoolean()) {
nodeSettingsBuilder.put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), randomBoolean() ? between(1, 3) : between(6, 100));
}
assertThat(JvmErgonomics.choose(List.of(), nodeSettingsBuilder.build()), everyItem(not(startsWith("-XX:ConcGCThreads="))));
}

public void testConcGCThreadsNotSetBasedOnRoles() throws Exception {
Settings.Builder nodeSettingsBuilder = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), between(4, 5));
if (randomBoolean()) {
nodeSettingsBuilder.put(
NodeRoleSettings.NODE_ROLES_SETTING.getKey(),
randomValueOtherThan(DiscoveryNodeRole.SEARCH_ROLE, () -> randomFrom(DiscoveryNodeRole.roles())).roleName()
);
}
assertThat(JvmErgonomics.choose(List.of(), nodeSettingsBuilder.build()), everyItem(not(startsWith("-XX:ConcGCThreads="))));

}

public void testConcGCThreadsSet() throws Exception {
Settings nodeSettings = Settings.builder()
.put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), between(4, 5))
.put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), DiscoveryNodeRole.SEARCH_ROLE.roleName())
.build();
assertThat(JvmErgonomics.choose(List.of(), nodeSettings), hasItem("-XX:ConcGCThreads=2"));
}

public void testMinimumNewSizeNotSetBasedOnHeap() throws Exception {
Settings nodeSettings = Settings.builder()
.put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), DiscoveryNodeRole.SEARCH_ROLE.roleName())
.build();
List<String> chosen = JvmErgonomics.choose(List.of("-Xmx" + between(5, 31) + "g"), nodeSettings);
assertThat(chosen, everyItem(not(is("-XX:+UnlockExperimentalVMOptions"))));
assertThat(chosen, everyItem(not(startsWith("-XX:G1NewSizePercent="))));
}

public void testMinimumNewSizeNotSetBasedOnRoles() throws Exception {
Settings nodeSettings = randomBoolean()
? Settings.EMPTY
: Settings.builder()
.put(
NodeRoleSettings.NODE_ROLES_SETTING.getKey(),
randomValueOtherThan(DiscoveryNodeRole.SEARCH_ROLE, () -> randomFrom(DiscoveryNodeRole.roles())).roleName()
)
.build();
List<String> chosen = JvmErgonomics.choose(List.of("-Xmx" + between(1, 4) + "g"), nodeSettings);
assertThat(chosen, everyItem(not(is("-XX:+UnlockExperimentalVMOptions"))));
assertThat(chosen, everyItem(not(startsWith("-XX:G1NewSizePercent="))));
}

public void testMinimumNewSizeSet() throws Exception {
Settings nodeSettings = Settings.builder()
.put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), DiscoveryNodeRole.SEARCH_ROLE.roleName())
.build();
List<String> chosen = JvmErgonomics.choose(List.of("-Xmx" + between(1, 4) + "g"), nodeSettings);
assertThat(chosen, hasItem("-XX:+UnlockExperimentalVMOptions"));
assertThat(chosen, hasItem("-XX:G1NewSizePercent=10"));
}

@SuppressWarnings("ConstantConditions")
public void testMissingOptionHandling() {
final Map<String, JvmOption> g1GcOn = Map.of("UseG1GC", new JvmOption("true", ""));
Expand Down

0 comments on commit 77af821

Please sign in to comment.