From 19ecd415e9cfe7b0dc6cdc9a6c60a07aa7815be2 Mon Sep 17 00:00:00 2001
From: Lukas Wegmann <lukas.wegmann@elastic.co>
Date: Thu, 28 Oct 2021 10:58:05 +0200
Subject: [PATCH] SQL: stabilize SqlSearchPageTimeoutIT (#79928) (#79992)

#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());