From f86475c3abafdc7dfdb14abe6b05539d199b33de Mon Sep 17 00:00:00 2001 From: Lantao Jin Date: Tue, 19 Aug 2025 11:19:55 +0800 Subject: [PATCH 1/3] Add flaky retry on CalcitePPLTpchIT.testQ7 Signed-off-by: Lantao Jin --- .../sql/calcite/tpch/CalcitePPLTpchIT.java | 4 ++ .../opensearch/sql/ppl/PPLIntegTestCase.java | 3 ++ .../java/org/opensearch/sql/util/Retry.java | 17 ++++++++ .../opensearch/sql/util/RetryProcessor.java | 39 +++++++++++++++++++ 4 files changed, 63 insertions(+) create mode 100644 integ-test/src/test/java/org/opensearch/sql/util/Retry.java create mode 100644 integ-test/src/test/java/org/opensearch/sql/util/RetryProcessor.java diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java index 27fe8c49edf..13ab23a8233 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java @@ -18,6 +18,7 @@ import org.junit.Ignore; import org.junit.Test; import org.opensearch.sql.ppl.PPLIntegTestCase; +import org.opensearch.sql.util.Retry; public class CalcitePPLTpchIT extends PPLIntegTestCase { @@ -173,6 +174,8 @@ public void testQ6() throws IOException { verifyDataRows(actual, rows(77949.9186)); } + @Test + @Retry public void testQ7() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q7.ppl")); JSONObject actual = executeQuery(ppl); @@ -185,6 +188,7 @@ public void testQ7() throws IOException { verifyNumOfRows(actual, 0); } + @Test public void testQ8() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q8.ppl")); JSONObject actual = executeQuery(ppl); diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java index 68330272767..43aad2d10a3 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java @@ -21,6 +21,7 @@ import org.json.JSONException; import org.json.JSONObject; import org.junit.Assert; +import org.junit.Rule; import org.opensearch.client.Request; import org.opensearch.client.RequestOptions; import org.opensearch.client.Response; @@ -29,10 +30,12 @@ import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.common.setting.Settings.Key; import org.opensearch.sql.legacy.SQLIntegTestCase; +import org.opensearch.sql.util.RetryProcessor; /** OpenSearch Rest integration test base for PPL testing. */ public abstract class PPLIntegTestCase extends SQLIntegTestCase { private static final Logger LOG = LogManager.getLogger(); + @Rule public final RetryProcessor retryProcessor = new RetryProcessor(); @Override protected void init() throws Exception { diff --git a/integ-test/src/test/java/org/opensearch/sql/util/Retry.java b/integ-test/src/test/java/org/opensearch/sql/util/Retry.java new file mode 100644 index 00000000000..725c3d15cc7 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/util/Retry.java @@ -0,0 +1,17 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.util; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ElementType.METHOD}) +@Retention(RetentionPolicy.RUNTIME) +public @interface Retry { + int value() default 3; +} diff --git a/integ-test/src/test/java/org/opensearch/sql/util/RetryProcessor.java b/integ-test/src/test/java/org/opensearch/sql/util/RetryProcessor.java new file mode 100644 index 00000000000..2383c486dbc --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/util/RetryProcessor.java @@ -0,0 +1,39 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.util; + +import org.junit.rules.TestWatcher; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +public class RetryProcessor extends TestWatcher { + + @Override + public Statement apply(Statement base, Description description) { + Retry retry = description.getAnnotation(Retry.class); + if (retry == null) { + return base; + } + return new Statement() { + @Override + public void evaluate() throws Throwable { + Throwable lastException = null; + for (int i = 0; i < retry.value(); i++) { + try { + base.evaluate(); + return; + } catch (Throwable t) { + lastException = t; + System.out.println( + "Retrying " + description.getDisplayName() + " " + (i + 1) + " times"); + } + } + assert lastException != null; + throw lastException; + } + }; + } +} From 8553c212774f1428fe451ff6dff25c7af31dd051 Mon Sep 17 00:00:00 2001 From: Lantao Jin Date: Tue, 19 Aug 2025 14:04:33 +0800 Subject: [PATCH 2/3] Add retry to all tpch queries Signed-off-by: Lantao Jin --- .../sql/calcite/tpch/CalcitePPLTpchIT.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java index 13ab23a8233..139d4b46d9d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java @@ -38,6 +38,7 @@ public void init() throws Exception { } @Test + @Retry public void testQ1() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q1.ppl")); JSONObject actual = executeQuery(ppl); @@ -102,6 +103,7 @@ public void testQ1() throws IOException { } @Test + @Retry public void testQ2() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q2.ppl")); JSONObject actual = executeQuery(ppl); @@ -119,6 +121,7 @@ public void testQ2() throws IOException { } @Test + @Retry public void testQ3() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q3.ppl")); JSONObject actual = executeQuery(ppl); @@ -143,7 +146,6 @@ public void testQ3() throws IOException { // TODO: Aggregation push down has a hard-coded limit of 1000 buckets for output, so this query // will not return the correct results with aggregation push down and it's unstable @Ignore - @Test public void testQ4() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q4.ppl")); JSONObject actual = executeQuery(ppl); @@ -159,6 +161,7 @@ public void testQ4() throws IOException { } @Test + @Retry public void testQ5() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q5.ppl")); JSONObject actual = executeQuery(ppl); @@ -167,6 +170,7 @@ public void testQ5() throws IOException { } @Test + @Retry public void testQ6() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q6.ppl")); JSONObject actual = executeQuery(ppl); @@ -189,6 +193,7 @@ public void testQ7() throws IOException { } @Test + @Retry public void testQ8() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q8.ppl")); JSONObject actual = executeQuery(ppl); @@ -197,6 +202,7 @@ public void testQ8() throws IOException { } @Test + @Retry public void testQ9() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q9.ppl")); JSONObject actual = executeQuery(ppl); @@ -209,6 +215,7 @@ public void testQ9() throws IOException { } @Test + @Retry public void testQ10() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q10.ppl")); JSONObject actual = executeQuery(ppl); @@ -238,6 +245,7 @@ public void testQ10() throws IOException { } @Test + @Retry public void testQ11() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q11.ppl")); JSONObject actual = executeQuery(ppl); @@ -246,6 +254,7 @@ public void testQ11() throws IOException { } @Test + @Retry public void testQ12() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q12.ppl")); JSONObject actual = executeQuery(ppl); @@ -258,6 +267,7 @@ public void testQ12() throws IOException { } @Test + @Retry public void testQ13() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q13.ppl")); JSONObject actual = executeQuery(ppl); @@ -294,6 +304,7 @@ public void testQ13() throws IOException { } @Test + @Retry public void testQ14() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q14.ppl")); JSONObject actual = executeQuery(ppl); @@ -302,6 +313,7 @@ public void testQ14() throws IOException { } @Test + @Retry public void testQ15() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q15.ppl")); JSONObject actual = executeQuery(ppl); @@ -318,6 +330,7 @@ public void testQ15() throws IOException { } @Test + @Retry public void testQ16() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q16.ppl")); JSONObject actual = executeQuery(ppl); @@ -366,6 +379,7 @@ public void testQ16() throws IOException { } @Test + @Retry public void testQ17() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q17.ppl")); String actual = executeQuery(ppl).toString(); @@ -389,6 +403,7 @@ public void testQ17() throws IOException { } @Test + @Retry public void testQ18() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q18.ppl")); JSONObject actual = executeQuery(ppl); @@ -404,6 +419,7 @@ public void testQ18() throws IOException { } @Test + @Retry public void testQ19() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q19.ppl")); String actual = executeQuery(ppl).toString(); @@ -427,6 +443,7 @@ public void testQ19() throws IOException { } @Test + @Retry public void testQ20() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q20.ppl")); JSONObject actual = executeQuery(ppl); @@ -435,6 +452,7 @@ public void testQ20() throws IOException { } @Test + @Retry public void testQ21() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q21.ppl")); JSONObject actual = executeQuery(ppl); @@ -443,6 +461,7 @@ public void testQ21() throws IOException { } @Test + @Retry public void testQ22() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q22.ppl")); JSONObject actual = executeQuery(ppl); From 95f32d0d8ea5e67abdb27523820ddca42b960538 Mon Sep 17 00:00:00 2001 From: Lantao Jin Date: Wed, 20 Aug 2025 13:31:47 +0800 Subject: [PATCH 3/3] address comments Signed-off-by: Lantao Jin --- .../sql/calcite/tpch/CalcitePPLTpchIT.java | 22 +------------------ .../java/org/opensearch/sql/util/Retry.java | 7 +++++- .../opensearch/sql/util/RetryProcessor.java | 7 ++++-- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java index 139d4b46d9d..c440432b7ea 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java @@ -20,6 +20,7 @@ import org.opensearch.sql.ppl.PPLIntegTestCase; import org.opensearch.sql.util.Retry; +@Retry public class CalcitePPLTpchIT extends PPLIntegTestCase { @Override @@ -38,7 +39,6 @@ public void init() throws Exception { } @Test - @Retry public void testQ1() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q1.ppl")); JSONObject actual = executeQuery(ppl); @@ -103,7 +103,6 @@ public void testQ1() throws IOException { } @Test - @Retry public void testQ2() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q2.ppl")); JSONObject actual = executeQuery(ppl); @@ -121,7 +120,6 @@ public void testQ2() throws IOException { } @Test - @Retry public void testQ3() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q3.ppl")); JSONObject actual = executeQuery(ppl); @@ -161,7 +159,6 @@ public void testQ4() throws IOException { } @Test - @Retry public void testQ5() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q5.ppl")); JSONObject actual = executeQuery(ppl); @@ -170,7 +167,6 @@ public void testQ5() throws IOException { } @Test - @Retry public void testQ6() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q6.ppl")); JSONObject actual = executeQuery(ppl); @@ -179,7 +175,6 @@ public void testQ6() throws IOException { } @Test - @Retry public void testQ7() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q7.ppl")); JSONObject actual = executeQuery(ppl); @@ -193,7 +188,6 @@ public void testQ7() throws IOException { } @Test - @Retry public void testQ8() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q8.ppl")); JSONObject actual = executeQuery(ppl); @@ -202,7 +196,6 @@ public void testQ8() throws IOException { } @Test - @Retry public void testQ9() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q9.ppl")); JSONObject actual = executeQuery(ppl); @@ -215,7 +208,6 @@ public void testQ9() throws IOException { } @Test - @Retry public void testQ10() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q10.ppl")); JSONObject actual = executeQuery(ppl); @@ -245,7 +237,6 @@ public void testQ10() throws IOException { } @Test - @Retry public void testQ11() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q11.ppl")); JSONObject actual = executeQuery(ppl); @@ -254,7 +245,6 @@ public void testQ11() throws IOException { } @Test - @Retry public void testQ12() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q12.ppl")); JSONObject actual = executeQuery(ppl); @@ -267,7 +257,6 @@ public void testQ12() throws IOException { } @Test - @Retry public void testQ13() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q13.ppl")); JSONObject actual = executeQuery(ppl); @@ -304,7 +293,6 @@ public void testQ13() throws IOException { } @Test - @Retry public void testQ14() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q14.ppl")); JSONObject actual = executeQuery(ppl); @@ -313,7 +301,6 @@ public void testQ14() throws IOException { } @Test - @Retry public void testQ15() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q15.ppl")); JSONObject actual = executeQuery(ppl); @@ -330,7 +317,6 @@ public void testQ15() throws IOException { } @Test - @Retry public void testQ16() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q16.ppl")); JSONObject actual = executeQuery(ppl); @@ -379,7 +365,6 @@ public void testQ16() throws IOException { } @Test - @Retry public void testQ17() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q17.ppl")); String actual = executeQuery(ppl).toString(); @@ -403,7 +388,6 @@ public void testQ17() throws IOException { } @Test - @Retry public void testQ18() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q18.ppl")); JSONObject actual = executeQuery(ppl); @@ -419,7 +403,6 @@ public void testQ18() throws IOException { } @Test - @Retry public void testQ19() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q19.ppl")); String actual = executeQuery(ppl).toString(); @@ -443,7 +426,6 @@ public void testQ19() throws IOException { } @Test - @Retry public void testQ20() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q20.ppl")); JSONObject actual = executeQuery(ppl); @@ -452,7 +434,6 @@ public void testQ20() throws IOException { } @Test - @Retry public void testQ21() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q21.ppl")); JSONObject actual = executeQuery(ppl); @@ -461,7 +442,6 @@ public void testQ21() throws IOException { } @Test - @Retry public void testQ22() throws IOException { String ppl = sanitize(loadFromFile("tpch/queries/q22.ppl")); JSONObject actual = executeQuery(ppl); diff --git a/integ-test/src/test/java/org/opensearch/sql/util/Retry.java b/integ-test/src/test/java/org/opensearch/sql/util/Retry.java index 725c3d15cc7..5ed873ddcb2 100644 --- a/integ-test/src/test/java/org/opensearch/sql/util/Retry.java +++ b/integ-test/src/test/java/org/opensearch/sql/util/Retry.java @@ -10,7 +10,12 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -@Target({ElementType.METHOD}) +/** + * Retry annotation to indicate a test should be retried when exception happens. The default retry + * count is 3. You can specify the retry count by passing a value to the annotation. For + * example: @Retry(5) + */ +@Target({ElementType.TYPE, ElementType.METHOD}) @Retention(RetentionPolicy.RUNTIME) public @interface Retry { int value() default 3; diff --git a/integ-test/src/test/java/org/opensearch/sql/util/RetryProcessor.java b/integ-test/src/test/java/org/opensearch/sql/util/RetryProcessor.java index 2383c486dbc..d747ebfed7d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/util/RetryProcessor.java +++ b/integ-test/src/test/java/org/opensearch/sql/util/RetryProcessor.java @@ -5,11 +5,15 @@ package org.opensearch.sql.util; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.junit.rules.TestWatcher; import org.junit.runner.Description; import org.junit.runners.model.Statement; +/** Retry processor to retry a test when exception happens. Retry a test by adding @Retry. */ public class RetryProcessor extends TestWatcher { + private static final Logger LOG = LogManager.getLogger(); @Override public Statement apply(Statement base, Description description) { @@ -27,8 +31,7 @@ public void evaluate() throws Throwable { return; } catch (Throwable t) { lastException = t; - System.out.println( - "Retrying " + description.getDisplayName() + " " + (i + 1) + " times"); + LOG.info("Retrying {} {} times", description.getDisplayName(), (i + 1)); } } assert lastException != null;