From 4b7f681c28707aac2d84fa6357a6a30cd5dd5aac Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 28 Sep 2023 12:57:50 -0400 Subject: [PATCH] ESQL: Make sure the request breaker is empty ESQL uses the request breaker to track memory and it's super important that it clear the breaker after every execution, including errors. --- .../esql/qa/server/single-node/build.gradle | 1 + .../esql/qa/single_node/EsqlClientYamlIT.java | 9 +++++++ .../xpack/esql/qa/rest/EsqlSpecTestCase.java | 25 +++++++++++++++++++ .../xpack/esql/qa/rest/RestEsqlTestCase.java | 7 ++++++ .../src/main/resources/keep.csv-spec | 3 ++- .../metadata-ignoreCsvTests.csv-spec | 3 ++- 6 files changed, 46 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/single-node/build.gradle b/x-pack/plugin/esql/qa/server/single-node/build.gradle index 6f913100e0fd7..d5fedad1a537b 100644 --- a/x-pack/plugin/esql/qa/server/single-node/build.gradle +++ b/x-pack/plugin/esql/qa/server/single-node/build.gradle @@ -2,6 +2,7 @@ apply plugin: 'elasticsearch.legacy-yaml-rest-test' dependencies { javaRestTestImplementation project(xpackModule('esql:qa:testFixtures')) + yamlRestTestImplementation project(xpackModule('esql:qa:server')) } restResources { diff --git a/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlClientYamlIT.java b/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlClientYamlIT.java index 64aaf547e5468..38d58644926fe 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlClientYamlIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlClientYamlIT.java @@ -11,6 +11,9 @@ import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate; import org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase; +import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; +import org.junit.After; +import org.junit.Before; public class EsqlClientYamlIT extends ESClientYamlSuiteTestCase { @@ -22,4 +25,10 @@ public EsqlClientYamlIT(final ClientYamlTestCandidate testCandidate) { public static Iterable parameters() throws Exception { return createParameters(); } + + @Before + @After + public void assertRequestBreakerEmpty() throws Exception { + EsqlSpecTestCase.assertRequestBreakerEmpty(); + } } diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java index e9dc848024448..4fc89e11b7b6b 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java @@ -8,8 +8,10 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import org.apache.http.HttpEntity; import org.elasticsearch.client.Request; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; import org.elasticsearch.test.rest.ESRestTestCase; @@ -17,6 +19,7 @@ import org.elasticsearch.xpack.esql.qa.rest.RestEsqlTestCase.RequestObjectBuilder; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; import org.elasticsearch.xpack.ql.SpecReader; +import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -25,6 +28,8 @@ import java.util.List; import java.util.Map; +import static org.elasticsearch.test.MapMatcher.assertMap; +import static org.elasticsearch.test.MapMatcher.matchesMap; import static org.elasticsearch.xpack.esql.CsvAssert.assertData; import static org.elasticsearch.xpack.esql.CsvAssert.assertMetadata; import static org.elasticsearch.xpack.esql.CsvTestUtils.isEnabled; @@ -123,4 +128,24 @@ private Throwable reworkException(Throwable th) { protected boolean preserveClusterUponCompletion() { return true; } + + @Before + @After + public void assertRequestBreakerEmptyAfterTests() throws Exception { + assertRequestBreakerEmpty(); + } + + public static void assertRequestBreakerEmpty() throws Exception { + assertBusy(() -> { + HttpEntity entity = adminClient().performRequest(new Request("GET", "/_nodes/stats")).getEntity(); + Map stats = XContentHelper.convertToMap(XContentType.JSON.xContent(), entity.getContent(), false); + Map nodes = (Map) stats.get("nodes"); + for (Object n : nodes.values()) { + Map node = (Map) n; + Map breakers = (Map) node.get("breakers"); + Map request = (Map) breakers.get("request"); + assertMap(request, matchesMap().extraOk().entry("estimated_size_in_bytes", 0).entry("estimated_size", "0b")); + } + }); + } } diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index 2b8bead0f86bc..14e645f37a659 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -26,6 +26,7 @@ import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; import org.junit.After; +import org.junit.Before; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -556,4 +557,10 @@ protected static String fromIndex() { protected boolean preserveClusterUponCompletion() { return true; } + + @Before + @After + public void assertRequestBreakerEmpty() throws Exception { + EsqlSpecTestCase.assertRequestBreakerEmpty(); + } } diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/keep.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/keep.csv-spec index 3637081c3c4b6..fd87eedf57f2f 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/keep.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/keep.csv-spec @@ -205,7 +205,8 @@ emp_no:integer | languages:integer | gender:keyword | first_name:keyword | abc:i 10100 | 4 | F | Hironobu | 3 ; -projectFromWithStatsAfterLimit +# awaitsfix https://github.com/elastic/elasticsearch/issues/99826 +projectFromWithStatsAfterLimit-Ignore from employees | sort emp_no | keep gender, avg_worked_seconds, first_name, last_name | limit 10 | stats m = max(avg_worked_seconds) by gender; m:long | gender:keyword diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/metadata-ignoreCsvTests.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/metadata-ignoreCsvTests.csv-spec index 82fd27416c526..ee2d3ab6db801 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/metadata-ignoreCsvTests.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/metadata-ignoreCsvTests.csv-spec @@ -95,7 +95,8 @@ emp_no:integer |_version:long |_index:keyword 10002 |1 |employees ; -withMvFunction +# awaitsfix https://github.com/elastic/elasticsearch/issues/99826 +withMvFunction-Ignore from employees [metadata _version] | eval i = mv_avg(_version) + 2 | stats min = min(emp_no) by i; min:integer |i:double