Skip to content

Commit

Permalink
Fix ShardSearchRequest cache key (#54071)
Browse files Browse the repository at this point in the history 
This commit ensures that we don't use the non-deterministic canReturnNullResponseIfMatchNoDocs
boolean in the cache key of the ShardSearchRequest. The value of this boolean has no influence
on the cacheability of the request.

Closes #32827
  • Loading branch information
@jimczi
jimczi committed Mar 24, 2020
1 parent 627ca03 commit 025857d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 21 deletions.
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ protected final void innerWriteTo(StreamOutput out, boolean asKey) throws IOExce
out.writeOptionalString(preference);
}
}
if (out.getVersion().onOrAfter(Version.V_7_7_0)) {
if (out.getVersion().onOrAfter(Version.V_7_7_0) && asKey == false) {
out.writeBoolean(canReturnNullResponseIfMatchNoDocs);
out.writeOptionalWriteable(bottomSortValues);
}
Expand Down
52 changes: 32 additions & 20 deletions server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ public void testCacheAggs() throws Exception {
}
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/32827")
public void testQueryRewrite() throws Exception {
Client client = client();
assertAcked(client.admin().indices().prepareCreate("index").addMapping("type", "s", "type=date")
Expand Down Expand Up @@ -128,23 +127,22 @@ public void testQueryRewrite() throws Exception {
.setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-19").lte("2016-03-25"))
// to ensure that query is executed even if it rewrites to match_no_docs
.addAggregation(new GlobalAggregationBuilder("global"))
.setPreFilterShardSize(Integer.MAX_VALUE).get();
.get();
ElasticsearchAssertions.assertAllSuccessful(r1);
assertThat(r1.getHits().getTotalHits().value, equalTo(7L));
assertCacheState(client, "index", 0, 5);

final SearchResponse r2 = client.prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0)
.setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-20").lte("2016-03-26"))
.addAggregation(new GlobalAggregationBuilder("global"))
.setPreFilterShardSize(Integer.MAX_VALUE).get();
.get();
ElasticsearchAssertions.assertAllSuccessful(r2);
assertThat(r2.getHits().getTotalHits().value, equalTo(7L));
assertCacheState(client, "index", 3, 7);

final SearchResponse r3 = client.prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0)
.setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-21").lte("2016-03-27"))
.addAggregation(new GlobalAggregationBuilder("global"))
.setPreFilterShardSize(Integer.MAX_VALUE)
.get();
ElasticsearchAssertions.assertAllSuccessful(r3);
assertThat(r3.getHits().getTotalHits().value, equalTo(7L));
Expand Down Expand Up @@ -220,29 +218,35 @@ public void testQueryRewriteDates() throws Exception {

assertCacheState(client, "index", 0, 0);

final SearchResponse r1 = client.prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("2013-01-01T00:00:00").lte("now"))
.get();
final SearchResponse r1 = client.prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH)
.setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("2013-01-01T00:00:00").lte("now"))
// to ensure that query is executed even if it rewrites to match_no_docs
.addAggregation(new GlobalAggregationBuilder("global"))
.get();
ElasticsearchAssertions.assertAllSuccessful(r1);
assertThat(r1.getHits().getTotalHits().value, equalTo(9L));
assertCacheState(client, "index", 0, 1);

final SearchResponse r2 = client.prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("2013-01-01T00:00:00").lte("now"))
.get();
final SearchResponse r2 = client.prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH)
.setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("2013-01-01T00:00:00").lte("now"))
.addAggregation(new GlobalAggregationBuilder("global"))
.get();
ElasticsearchAssertions.assertAllSuccessful(r2);
assertThat(r2.getHits().getTotalHits().value, equalTo(9L));
assertCacheState(client, "index", 1, 1);

final SearchResponse r3 = client.prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("2013-01-01T00:00:00").lte("now"))
.get();
final SearchResponse r3 = client.prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH)
.setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("2013-01-01T00:00:00").lte("now"))
.addAggregation(new GlobalAggregationBuilder("global"))
.get();
ElasticsearchAssertions.assertAllSuccessful(r3);
assertThat(r3.getHits().getTotalHits().value, equalTo(9L));
assertCacheState(client, "index", 2, 1);
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/32827")
public void testQueryRewriteDatesWithNow() throws Exception {
Client client = client();
Settings settings = Settings.builder().put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), true)
Expand Down Expand Up @@ -280,8 +284,10 @@ public void testQueryRewriteDatesWithNow() throws Exception {
assertCacheState(client, "index-2", 0, 0);
assertCacheState(client, "index-3", 0, 0);

final SearchResponse r1 = client.prepareSearch("index-*").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now")).get();
final SearchResponse r1 = client.prepareSearch("index-*")
.setSearchType(SearchType.QUERY_THEN_FETCH)
.setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now")).get();
ElasticsearchAssertions.assertAllSuccessful(r1);
assertThat(r1.getHits().getTotalHits().value, equalTo(8L));
assertCacheState(client, "index-1", 0, 1);
Expand All @@ -291,16 +297,22 @@ public void testQueryRewriteDatesWithNow() throws Exception {
// cache miss or cache hit since queries containing now can't be cached
assertCacheState(client, "index-3", 0, 0);

final SearchResponse r2 = client.prepareSearch("index-*").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now")).get();
final SearchResponse r2 = client.prepareSearch("index-*")
.setSearchType(SearchType.QUERY_THEN_FETCH)
.setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now"))
.get();
ElasticsearchAssertions.assertAllSuccessful(r2);
assertThat(r2.getHits().getTotalHits().value, equalTo(8L));
assertCacheState(client, "index-1", 1, 1);
assertCacheState(client, "index-2", 1, 1);
assertCacheState(client, "index-3", 0, 0);

final SearchResponse r3 = client.prepareSearch("index-*").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now")).get();
final SearchResponse r3 = client.prepareSearch("index-*")
.setSearchType(SearchType.QUERY_THEN_FETCH)
.setSize(0)
.setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now"))
.get();
ElasticsearchAssertions.assertAllSuccessful(r3);
assertThat(r3.getHits().getTotalHits().value, equalTo(8L));
assertCacheState(client, "index-1", 2, 1);
Expand Down

0 comments on commit 025857d

Please sign in to comment.