Skip to content

Commit

Permalink
ESQL: Make sure the request breaker is empty
Browse files Browse the repository at this point in the history
ESQL uses the request breaker to track memory and it's super important
that it clear the breaker after every execution, including errors.
  • Loading branch information
nik9000 committed Sep 28, 2023
1 parent 0e21930 commit 4b7f681
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 2 deletions.
1 change: 1 addition & 0 deletions x-pack/plugin/esql/qa/server/single-node/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -22,4 +25,10 @@ public EsqlClientYamlIT(final ClientYamlTestCandidate testCandidate) {
public static Iterable<Object[]> parameters() throws Exception {
return createParameters();
}

@Before
@After
public void assertRequestBreakerEmpty() throws Exception {
EsqlSpecTestCase.assertRequestBreakerEmpty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@

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;
import org.elasticsearch.xcontent.XContentType;
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;

Expand All @@ -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;
Expand Down Expand Up @@ -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"));
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -556,4 +557,10 @@ protected static String fromIndex() {
protected boolean preserveClusterUponCompletion() {
return true;
}

@Before
@After
public void assertRequestBreakerEmpty() throws Exception {
EsqlSpecTestCase.assertRequestBreakerEmpty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4b7f681

Please sign in to comment.