From 257afc629a91af71fb17fb468e7025dff3443f94 Mon Sep 17 00:00:00 2001 From: Lantao Jin Date: Fri, 22 Aug 2025 16:56:11 +0800 Subject: [PATCH 1/3] Disable GCedMemoryUsage if no concurrent GC MXBean Signed-off-by: Lantao Jin --- .../opensearch/sql/ppl/ResourceMonitorIT.java | 23 ++++++++++++------- .../opensearch/monitor/GCedMemoryUsage.java | 19 +++++++++------ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java index 53544607fd6..b26ba867827 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java @@ -5,9 +5,7 @@ package org.opensearch.sql.ppl; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG; -import static org.opensearch.sql.util.MatcherUtils.columnName; -import static org.opensearch.sql.util.MatcherUtils.verifyColumn; +import static org.opensearch.sql.util.MatcherUtils.verifyNumOfRows; import java.io.IOException; import org.hamcrest.Matchers; @@ -21,7 +19,7 @@ public class ResourceMonitorIT extends PPLIntegTestCase { @Override public void init() throws Exception { super.init(); - loadIndex(Index.DOG); + loadIndex(Index.CLICK_BENCH); } @Test @@ -29,9 +27,11 @@ public void queryExceedResourceLimitShouldFail() throws IOException { // update plugins.ppl.query.memory_limit to 1% updateClusterSettings( new ClusterSetting("persistent", Settings.Key.QUERY_MEMORY_LIMIT.getKeyValue(), "1%")); - String query = String.format("search source=%s age=20", TEST_INDEX_DOG); + // ClickBench Q30 is high memory consumption query. + String query = sanitize(loadFromFile("clickbench/queries/q30.ppl")); - ResponseException exception = expectThrows(ResponseException.class, () -> executeQuery(query)); + ResponseException exception = + expectThrows(ResponseException.class, () -> executeQueryMultipleTimes(query)); assertEquals(500, exception.getResponse().getStatusLine().getStatusCode()); assertThat( exception.getMessage(), @@ -40,7 +40,14 @@ public void queryExceedResourceLimitShouldFail() throws IOException { // update plugins.ppl.query.memory_limit to default value 85% updateClusterSettings( new ClusterSetting("persistent", Settings.Key.QUERY_MEMORY_LIMIT.getKeyValue(), "85%")); - JSONObject result = executeQuery(String.format("search source=%s", TEST_INDEX_DOG)); - verifyColumn(result, columnName("dog_name"), columnName("holdersName"), columnName("age")); + executeQueryMultipleTimes(query); + } + + /** Run the same query multiple times in case GC can be triggered in integration test */ + private void executeQueryMultipleTimes(String query) throws IOException { + for (int i = 0; i < 10; i++) { + JSONObject result = executeQuery(query); + verifyNumOfRows(result, 1); + } } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java index a94209b0ef3..3284ac81ca4 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java @@ -56,12 +56,18 @@ public void setUsage(long value) { } private void registerGCListener() { - boolean registered = false; List gcBeans = ManagementFactory.getGarbageCollectorMXBeans(); + if (gcBeans.stream() + .filter(b -> b instanceof NotificationEmitter) + .noneMatch(b -> containConcurrentGcBean(b.getName()))) { + // Concurrent Garbage Collector MXBean only existed since Java 21. + // fallback to RuntimeMemoryUsage + LOG.info("No Concurrent Garbage Collector MXBean, fallback to RuntimeMemoryUsage"); + throw new OpenSearchMemoryHealthy.MemoryUsageException(); + } for (GarbageCollectorMXBean gcBean : gcBeans) { if (gcBean instanceof NotificationEmitter && isOldGenGc(gcBean.getName())) { LOG.info("{} listener registered for memory usage monitor.", gcBean.getName()); - registered = true; NotificationEmitter emitter = (NotificationEmitter) gcBean; emitter.addNotificationListener( new OldGenGCListener(), @@ -78,11 +84,10 @@ private void registerGCListener() { null); } } - if (!registered) { - // fallback to RuntimeMemoryUsage - LOG.info("No old gen GC listener registered, fallback to RuntimeMemoryUsage"); - throw new OpenSearchMemoryHealthy.MemoryUsageException(); - } + } + + private boolean containConcurrentGcBean(String beanName) { + return beanName.toLowerCase(Locale.ROOT).contains("concurrent"); } private boolean isOldGenGc(String gcKeyword) { From 69a60cd93df28d14f21bfe02e41942aa1c8c906b Mon Sep 17 00:00:00 2001 From: Lantao Jin Date: Fri, 22 Aug 2025 17:47:00 +0800 Subject: [PATCH 2/3] Fix IT Signed-off-by: Lantao Jin --- .../opensearch/sql/ppl/ResourceMonitorIT.java | 24 ++++++++++++------- .../opensearch/monitor/GCedMemoryUsage.java | 6 +++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java index b26ba867827..ea1fde77493 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/ResourceMonitorIT.java @@ -13,6 +13,7 @@ import org.junit.Test; import org.opensearch.client.ResponseException; import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.opensearch.monitor.GCedMemoryUsage; public class ResourceMonitorIT extends PPLIntegTestCase { @@ -27,11 +28,7 @@ public void queryExceedResourceLimitShouldFail() throws IOException { // update plugins.ppl.query.memory_limit to 1% updateClusterSettings( new ClusterSetting("persistent", Settings.Key.QUERY_MEMORY_LIMIT.getKeyValue(), "1%")); - // ClickBench Q30 is high memory consumption query. - String query = sanitize(loadFromFile("clickbench/queries/q30.ppl")); - - ResponseException exception = - expectThrows(ResponseException.class, () -> executeQueryMultipleTimes(query)); + ResponseException exception = expectThrows(ResponseException.class, this::executeQuery); assertEquals(500, exception.getResponse().getStatusLine().getStatusCode()); assertThat( exception.getMessage(), @@ -40,12 +37,21 @@ public void queryExceedResourceLimitShouldFail() throws IOException { // update plugins.ppl.query.memory_limit to default value 85% updateClusterSettings( new ClusterSetting("persistent", Settings.Key.QUERY_MEMORY_LIMIT.getKeyValue(), "85%")); - executeQueryMultipleTimes(query); + executeQuery(); } - /** Run the same query multiple times in case GC can be triggered in integration test */ - private void executeQueryMultipleTimes(String query) throws IOException { - for (int i = 0; i < 10; i++) { + private void executeQuery() throws IOException { + if (GCedMemoryUsage.initialized()) { + // ClickBench Q30 is a high memory consumption query. Run 5 times to ensure GC triggered. + String query = sanitize(loadFromFile("clickbench/queries/q30.ppl")); + for (int i = 0; i < 5; i++) { + JSONObject result = executeQuery(query); + verifyNumOfRows(result, 1); + } + } else { + // Q2 is not a high memory consumption query. + // It cannot run in 1% resource but passed in 85%. + String query = sanitize(loadFromFile("clickbench/queries/q2.ppl")); JSONObject result = executeQuery(query); verifyNumOfRows(result, 1); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java index 3284ac81ca4..6a2e18b30cf 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java @@ -24,6 +24,7 @@ public class GCedMemoryUsage implements MemoryUsage { private static final Logger LOG = LogManager.getLogger(); private static final List OLD_GEN_GC_ACTION_KEYWORDS = List.of("major", "concurrent", "old", "full", "marksweep"); + private static boolean initialized = false; private GCedMemoryUsage() { registerGCListener(); @@ -55,6 +56,10 @@ public void setUsage(long value) { usage.set(value); } + public static boolean initialized() { + return initialized; + } + private void registerGCListener() { List gcBeans = ManagementFactory.getGarbageCollectorMXBeans(); if (gcBeans.stream() @@ -68,6 +73,7 @@ private void registerGCListener() { for (GarbageCollectorMXBean gcBean : gcBeans) { if (gcBean instanceof NotificationEmitter && isOldGenGc(gcBean.getName())) { LOG.info("{} listener registered for memory usage monitor.", gcBean.getName()); + initialized = true; NotificationEmitter emitter = (NotificationEmitter) gcBean; emitter.addNotificationListener( new OldGenGCListener(), From 427891b0cb60280437cd6856cf0e37dffaa822ea Mon Sep 17 00:00:00 2001 From: Lantao Jin Date: Mon, 25 Aug 2025 13:19:24 +0800 Subject: [PATCH 3/3] Ignore q30 when GCedMemoryUsage not initialized Signed-off-by: Lantao Jin --- .../sql/calcite/clickbench/PPLClickBenchIT.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/clickbench/PPLClickBenchIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/clickbench/PPLClickBenchIT.java index 5d520cd4b44..a2ce3d7841f 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/clickbench/PPLClickBenchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/clickbench/PPLClickBenchIT.java @@ -14,6 +14,7 @@ import org.junit.Test; import org.junit.runners.MethodSorters; import org.opensearch.common.collect.MapBuilder; +import org.opensearch.sql.opensearch.monitor.GCedMemoryUsage; import org.opensearch.sql.ppl.PPLIntegTestCase; @FixMethodOrder(MethodSorters.JVM) @@ -48,14 +49,15 @@ public static void reset() throws IOException { System.out.println(); } - /** - * Ignore queries that are not supported by Calcite. - * - *

q30 is ignored because it will trigger ResourceMonitory health check. TODO: should be - * addressed by: https://github.com/opensearch-project/sql/issues/3981 - */ + /** Ignore queries that are not supported by Calcite. */ protected Set ignored() { - return Set.of(29); + if (GCedMemoryUsage.initialized()) { + return Set.of(29); + } else { + // Ignore q30 when use RuntimeMemoryUsage, + // because of too much script push down, which will cause ResourceMonitor restriction. + return Set.of(29, 30); + } } @Test