From 025857d949dfd25464dfa18f53fbddea8c19039f Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 24 Mar 2020 18:01:50 +0100 Subject: [PATCH] Fix ShardSearchRequest cache key (#54071) 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 --- .../search/internal/ShardSearchRequest.java | 2 +- .../indices/IndicesRequestCacheIT.java | 52 ++++++++++++------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java index 9e20389f940..81e4c0085f7 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java @@ -229,7 +229,7 @@ public class ShardSearchRequest extends TransportRequest implements IndicesReque 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); } diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java b/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java index 56ebd8e068c..ec3d5d2a908 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java @@ -97,7 +97,6 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { } } - @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") @@ -128,7 +127,7 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { .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); @@ -136,7 +135,7 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { 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); @@ -144,7 +143,6 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { 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)); @@ -220,29 +218,35 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { 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) @@ -280,8 +284,10 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { 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); @@ -291,16 +297,22 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { // 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);