Skip to content

Commit

Permalink
SQL: stabilize SqlSearchPageTimeoutIT (elastic#79928)
Browse files Browse the repository at this point in the history
elastic#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.
  • Loading branch information
Lukas Wegmann authored and Luegg committed Oct 28, 2021
1 parent 3e3fe65 commit dced986
Showing 1 changed file with 4 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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());
Expand Down

0 comments on commit dced986

Please sign in to comment.