From dced986ad52008b7c2f9f26c16978d25744ad45a Mon Sep 17 00:00:00 2001 From: Lukas Wegmann Date: Thu, 28 Oct 2021 10:06:22 +0200 Subject: [PATCH] SQL: stabilize SqlSearchPageTimeoutIT (#79928) #79360 introduced a [flaky test](https://gradle-enterprise.elastic.co/scans/tests?search.relativeStartTime=P7D&search.timeZoneId=Europe/Zurich&tests.container=org.elasticsearch.xpack.sql.action.SqlSearchPageTimeoutIT&tests.sortField=FAILED&tests.test=testSearchContextIsCleanedUpAfterPageTimeoutForHitsQueries&tests.unstableOnly=true). Increasing the page timeout used in the initial query should ensure that line 44 should always see the search context (I suspect that it has been removed before `getNumberOfSearchContexts()` reads the search contexts). I've run the test with a 1000 iterations locally on both master and 7.16 and it didn't fail. --- .../xpack/sql/action/SqlSearchPageTimeoutIT.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/sql/src/internalClusterTest/java/org/elasticsearch/xpack/sql/action/SqlSearchPageTimeoutIT.java b/x-pack/plugin/sql/src/internalClusterTest/java/org/elasticsearch/xpack/sql/action/SqlSearchPageTimeoutIT.java index 144528defea02..479c9042e2d95 100644 --- a/x-pack/plugin/sql/src/internalClusterTest/java/org/elasticsearch/xpack/sql/action/SqlSearchPageTimeoutIT.java +++ b/x-pack/plugin/sql/src/internalClusterTest/java/org/elasticsearch/xpack/sql/action/SqlSearchPageTimeoutIT.java @@ -16,7 +16,6 @@ import org.elasticsearch.search.SearchService; import java.util.Arrays; -import java.util.concurrent.TimeUnit; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.contains; @@ -28,24 +27,22 @@ public class SqlSearchPageTimeoutIT extends AbstractSqlIntegTestCase { protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) { Settings.Builder settings = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings)); // use static low keepAlive interval to ensure obsolete search contexts are pruned soon enough - settings.put(SearchService.KEEPALIVE_INTERVAL_SETTING.getKey(), TimeValue.timeValueMillis(200)); + settings.put(SearchService.KEEPALIVE_INTERVAL_SETTING.getKey(), TimeValue.timeValueMillis(100)); return settings.build(); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/pull/79928") public void testSearchContextIsCleanedUpAfterPageTimeoutForHitsQueries() throws Exception { setupTestIndex(); SqlQueryResponse response = new SqlQueryRequestBuilder(client(), SqlQueryAction.INSTANCE).query("SELECT field FROM test") .fetchSize(1) - .pageTimeout(TimeValue.timeValueMillis(100)) + .pageTimeout(TimeValue.timeValueMillis(500)) .get(); - assertEquals(1, response.size()); assertTrue(response.hasCursor()); assertEquals(1, getNumberOfSearchContexts()); - assertBusy(() -> assertEquals(0, getNumberOfSearchContexts()), 3, TimeUnit.SECONDS); + assertBusy(() -> assertEquals(0, getNumberOfSearchContexts())); SearchPhaseExecutionException exception = expectThrows( SearchPhaseExecutionException.class, @@ -60,7 +57,7 @@ public void testNoSearchContextForAggregationQueries() throws InterruptedExcepti SqlQueryResponse response = new SqlQueryRequestBuilder(client(), SqlQueryAction.INSTANCE).query( "SELECT COUNT(*) FROM test GROUP BY field" - ).fetchSize(1).pageTimeout(TimeValue.timeValueMillis(100)).get(); + ).fetchSize(1).pageTimeout(TimeValue.timeValueMillis(500)).get(); assertEquals(1, response.size()); assertTrue(response.hasCursor());