Skip to content

Commit 9c09a02

Browse files
authored
FollowUp: Disable GCedMemoryUsage if no concurrent GC MXBean (opensearch-project#4107)
* Disable GCedMemoryUsage if no concurrent GC MXBean Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Ignore q30 when GCedMemoryUsage not initialized Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com>
1 parent fff3e3a commit 9c09a02

3 files changed

Lines changed: 49 additions & 23 deletions

File tree

integ-test/src/test/java/org/opensearch/sql/calcite/clickbench/PPLClickBenchIT.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.junit.Test;
1515
import org.junit.runners.MethodSorters;
1616
import org.opensearch.common.collect.MapBuilder;
17+
import org.opensearch.sql.opensearch.monitor.GCedMemoryUsage;
1718
import org.opensearch.sql.ppl.PPLIntegTestCase;
1819

1920
@FixMethodOrder(MethodSorters.JVM)
@@ -48,14 +49,15 @@ public static void reset() throws IOException {
4849
System.out.println();
4950
}
5051

51-
/**
52-
* Ignore queries that are not supported by Calcite.
53-
*
54-
* <p>q30 is ignored because it will trigger ResourceMonitory health check. TODO: should be
55-
* addressed by: https://github.com/opensearch-project/sql/issues/3981
56-
*/
52+
/** Ignore queries that are not supported by Calcite. */
5753
protected Set<Integer> ignored() {
58-
return Set.of(29);
54+
if (GCedMemoryUsage.initialized()) {
55+
return Set.of(29);
56+
} else {
57+
// Ignore q30 when use RuntimeMemoryUsage,
58+
// because of too much script push down, which will cause ResourceMonitor restriction.
59+
return Set.of(29, 30);
60+
}
5961
}
6062

6163
@Test

integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,33 +5,30 @@
55

66
package org.opensearch.sql.ppl;
77

8-
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG;
9-
import static org.opensearch.sql.util.MatcherUtils.columnName;
10-
import static org.opensearch.sql.util.MatcherUtils.verifyColumn;
8+
import static org.opensearch.sql.util.MatcherUtils.verifyNumOfRows;
119

1210
import java.io.IOException;
1311
import org.hamcrest.Matchers;
1412
import org.json.JSONObject;
1513
import org.junit.Test;
1614
import org.opensearch.client.ResponseException;
1715
import org.opensearch.sql.common.setting.Settings;
16+
import org.opensearch.sql.opensearch.monitor.GCedMemoryUsage;
1817

1918
public class ResourceMonitorIT extends PPLIntegTestCase {
2019

2120
@Override
2221
public void init() throws Exception {
2322
super.init();
24-
loadIndex(Index.DOG);
23+
loadIndex(Index.CLICK_BENCH);
2524
}
2625

2726
@Test
2827
public void queryExceedResourceLimitShouldFail() throws IOException {
2928
// update plugins.ppl.query.memory_limit to 1%
3029
updateClusterSettings(
3130
new ClusterSetting("persistent", Settings.Key.QUERY_MEMORY_LIMIT.getKeyValue(), "1%"));
32-
String query = String.format("search source=%s age=20", TEST_INDEX_DOG);
33-
34-
ResponseException exception = expectThrows(ResponseException.class, () -> executeQuery(query));
31+
ResponseException exception = expectThrows(ResponseException.class, this::executeQuery);
3532
assertEquals(500, exception.getResponse().getStatusLine().getStatusCode());
3633
assertThat(
3734
exception.getMessage(),
@@ -40,7 +37,23 @@ public void queryExceedResourceLimitShouldFail() throws IOException {
4037
// update plugins.ppl.query.memory_limit to default value 85%
4138
updateClusterSettings(
4239
new ClusterSetting("persistent", Settings.Key.QUERY_MEMORY_LIMIT.getKeyValue(), "85%"));
43-
JSONObject result = executeQuery(String.format("search source=%s", TEST_INDEX_DOG));
44-
verifyColumn(result, columnName("dog_name"), columnName("holdersName"), columnName("age"));
40+
executeQuery();
41+
}
42+
43+
private void executeQuery() throws IOException {
44+
if (GCedMemoryUsage.initialized()) {
45+
// ClickBench Q30 is a high memory consumption query. Run 5 times to ensure GC triggered.
46+
String query = sanitize(loadFromFile("clickbench/queries/q30.ppl"));
47+
for (int i = 0; i < 5; i++) {
48+
JSONObject result = executeQuery(query);
49+
verifyNumOfRows(result, 1);
50+
}
51+
} else {
52+
// Q2 is not a high memory consumption query.
53+
// It cannot run in 1% resource but passed in 85%.
54+
String query = sanitize(loadFromFile("clickbench/queries/q2.ppl"));
55+
JSONObject result = executeQuery(query);
56+
verifyNumOfRows(result, 1);
57+
}
4558
}
4659
}

opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public class GCedMemoryUsage implements MemoryUsage {
2424
private static final Logger LOG = LogManager.getLogger();
2525
private static final List<String> OLD_GEN_GC_ACTION_KEYWORDS =
2626
List.of("major", "concurrent", "old", "full", "marksweep");
27+
private static boolean initialized = false;
2728

2829
private GCedMemoryUsage() {
2930
registerGCListener();
@@ -55,13 +56,24 @@ public void setUsage(long value) {
5556
usage.set(value);
5657
}
5758

59+
public static boolean initialized() {
60+
return initialized;
61+
}
62+
5863
private void registerGCListener() {
59-
boolean registered = false;
6064
List<GarbageCollectorMXBean> gcBeans = ManagementFactory.getGarbageCollectorMXBeans();
65+
if (gcBeans.stream()
66+
.filter(b -> b instanceof NotificationEmitter)
67+
.noneMatch(b -> containConcurrentGcBean(b.getName()))) {
68+
// Concurrent Garbage Collector MXBean only existed since Java 21.
69+
// fallback to RuntimeMemoryUsage
70+
LOG.info("No Concurrent Garbage Collector MXBean, fallback to RuntimeMemoryUsage");
71+
throw new OpenSearchMemoryHealthy.MemoryUsageException();
72+
}
6173
for (GarbageCollectorMXBean gcBean : gcBeans) {
6274
if (gcBean instanceof NotificationEmitter && isOldGenGc(gcBean.getName())) {
6375
LOG.info("{} listener registered for memory usage monitor.", gcBean.getName());
64-
registered = true;
76+
initialized = true;
6577
NotificationEmitter emitter = (NotificationEmitter) gcBean;
6678
emitter.addNotificationListener(
6779
new OldGenGCListener(),
@@ -78,11 +90,10 @@ private void registerGCListener() {
7890
null);
7991
}
8092
}
81-
if (!registered) {
82-
// fallback to RuntimeMemoryUsage
83-
LOG.info("No old gen GC listener registered, fallback to RuntimeMemoryUsage");
84-
throw new OpenSearchMemoryHealthy.MemoryUsageException();
85-
}
93+
}
94+
95+
private boolean containConcurrentGcBean(String beanName) {
96+
return beanName.toLowerCase(Locale.ROOT).contains("concurrent");
8697
}
8798

8899
private boolean isOldGenGc(String gcKeyword) {

0 commit comments

Comments
 (0)