Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always create search context for scroll queries #52078

Merged
merged 1 commit into from
Feb 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,8 @@ public final ShardSearchRequest buildShardSearchRequest(SearchShardIterator shar
// if we already received a search result we can inform the shard that it
// can return a null response if the request rewrites to match none rather
// than creating an empty response in the search thread pool.
shardRequest.canReturnNullResponseIfMatchNoDocs(hasShardResponse.get());
// Note that, we have to disable this shortcut for scroll queries.
shardRequest.canReturnNullResponseIfMatchNoDocs(hasShardResponse.get() && request.scroll() == null);
return shardRequest;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ public void executeQueryPhase(ShardSearchRequest request, SearchShardTask task,
if (rewritten.canReturnNullResponseIfMatchNoDocs()
&& canRewriteToMatchNone(rewritten.source())
&& rewritten.source().query() instanceof MatchNoneQueryBuilder) {
assert request.scroll() == null : "must always create search context for scroll requests";
onMatchNoDocs(context, listener);
} else {
// fork the execution in the search thread pool and wraps the searcher
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.RangeQueryBuilder;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.sort.FieldSortBuilder;
Expand All @@ -53,6 +54,7 @@
import static org.elasticsearch.index.query.QueryBuilders.termQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoSearchHits;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
Expand Down Expand Up @@ -616,6 +618,43 @@ public void testInvalidScrollKeepAlive() throws IOException {
assertThat(illegalArgumentException.getMessage(), containsString("Keep alive for scroll (3h) is too large"));
}

/**
* Ensures that we always create and retain search contexts on every target shards for a scroll request
* regardless whether that query can be written to match_no_docs on some target shards or not.
*/
public void testScrollRewrittenToMatchNoDocs() {
final int numShards = randomIntBetween(3, 5);
assertAcked(
client().admin().indices().prepareCreate("test")
.setSettings(Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, numShards))
.setMapping("{\"properties\":{\"created_date\":{\"type\": \"date\", \"format\": \"yyyy-MM-dd\"}}}"));
client().prepareIndex("test").setId("1").setSource("created_date", "2020-01-01").get();
client().prepareIndex("test").setId("2").setSource("created_date", "2020-01-02").get();
client().prepareIndex("test").setId("3").setSource("created_date", "2020-01-03").get();
client().admin().indices().prepareRefresh("test").get();
SearchResponse resp = null;
try {
int totalHits = 0;
resp = client().prepareSearch("test")
.setQuery(new RangeQueryBuilder("created_date").gte("2020-01-02").lte("2020-01-03"))
.setMaxConcurrentShardRequests(randomIntBetween(1, 3)) // sometimes fan out shard requests one by one
.setSize(randomIntBetween(1, 2))
.setScroll(TimeValue.timeValueMinutes(1))
.get();
assertNoFailures(resp);
while (resp.getHits().getHits().length > 0) {
totalHits += resp.getHits().getHits().length;
resp = client().prepareSearchScroll(resp.getScrollId()).setScroll(TimeValue.timeValueMinutes(1)).get();
assertNoFailures(resp);
}
assertThat(totalHits, equalTo(2));
} finally {
if (resp != null && resp.getScrollId() != null) {
client().prepareClearScroll().addScrollId(resp.getScrollId()).get();
}
}
}

private void assertToXContentResponse(ClearScrollResponse response, boolean succeed, int numFreed) throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder();
response.toXContent(builder, ToXContent.EMPTY_PARAMS);
Expand Down