Bust the request cache when the mapping changes (backport of #66295) (#66795)

This makes sure that we only serve a hit from the request cache if it
was build using the same mapping and that the same mapping is used for
the entire "query phase" of the search.

Closes #62033

Author: nik9000

docs/reference/indices/apis/reload-analyzers.asciidoc

@@ -13,9 +13,17 @@ stream's backing indices.
 [source,console]
 --------------------------------------------------
 POST /my-index-000001/_reload_search_analyzers
+POST /my-index-000001/_cache/clear?request=true
 --------------------------------------------------
 // TEST[setup:my_index]
 
+IMPORTANT: After reloading the search analyzers you should clear the request
+           cache to make sure it doesn't contain responses derived from the
+           previous versions of the analyzer.
+// the need for this is tracked in https://github.com/elastic/elasticsearch/issues/66722
+
+
+
 [discrete]
 [[indices-reload-analyzers-api-request]]
 === {api-request-title}

docs/reference/modules/indices/request_cache.asciidoc

@@ -32,14 +32,14 @@ Scripted queries that use the API calls which are non-deterministic, such as
 The cache is smart -- it keeps the same _near real-time_ promise as uncached
 search.
 
-Cached results are invalidated automatically whenever the shard refreshes, but
-only if the data in the shard has actually changed.  In other words, you will
-always get the same results from the cache as you would for an uncached search
-request.
+Cached results are invalidated automatically whenever the shard refreshes to
+pick up changes to the documents or when you update the mapping. In other
+words you will always get the same results from the cache as you would for an
+uncached search request.
 
 The longer the refresh interval, the longer that cached entries will remain
-valid. If the cache is full, the least recently used cache keys will be
-evicted.
+valid even if there are changes to the documents. If the cache is full, the
+least recently used cache keys will be evicted.
 
 The cache can be expired manually with the <<indices-clearcache,`clear-cache` API>>:
 

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java

@@ -544,8 +544,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
             }
         }
         for (BytesReference document : documents) {
-            docs.add(context.parseDocument(type,
-                new SourceToParse(context.index().getName(), type, "_temp_id", document, documentXContentType)));
+            docs.add(context.parseDocument(new SourceToParse(context.index().getName(), type, "_temp_id", document, documentXContentType)));
         }
 
         // We need this custom analyzer because the default index analyzer is strict and the percolator sometimes isn't when

rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/220_filters_bucket.yml

@@ -272,3 +272,72 @@ setup:
             the_filter:
               filters:
                 filters: []
+
+
+---
+"cache":
+  - skip:
+      version: " - 7.11.99"
+      reason:  cache fixed in 7.12.0 to be backported to 7.11.0
+
+  - do:
+       bulk:
+         refresh: true
+         body:
+           - index:
+               _index: test_1
+               _id:    100
+           - int_field: 1
+             double_field: 1.0
+             string_field: foo bar
+
+  - do:
+      search:
+        index: test_1
+        body:
+          size: 0
+          aggs:
+            f:
+              filters:
+                filters:
+                  foo:
+                    match:
+                      string_field: foo
+                  foo_bar:
+                    match:
+                      string_field: foo bar
+  - match: { hits.total.value: 5 }
+  - length: { aggregations.f.buckets: 2 }
+  - match: { aggregations.f.buckets.foo.doc_count: 4 }
+  - match: { aggregations.f.buckets.foo_bar.doc_count: 1 }
+
+  # Modify the mapping configuration that generates queries. This should bust the cache.
+  - do:
+      indices.put_mapping:
+        index: test_1
+        body:
+          properties:
+            string_field:
+              type: keyword
+              split_queries_on_whitespace: true
+
+  # This should be entirely fresh because updating the mapping busted the cache.
+  - do:
+      search:
+        index: test_1
+        body:
+          size: 0
+          aggs:
+            f:
+              filters:
+                filters:
+                  foo:
+                    match:
+                      string_field: foo
+                  foo_bar:
+                    match:
+                      string_field: foo bar
+  - match: { hits.total.value: 5 }
+  - length: { aggregations.f.buckets: 2 }
+  - match: { aggregations.f.buckets.foo.doc_count: 4 }
+  - match: { aggregations.f.buckets.foo_bar.doc_count: 4 }

rest-api-spec/src/main/resources/rest-api-spec/test/search/340_type_query.yml

@@ -0,0 +1,88 @@
+type query:
+  - do:
+      index:
+          index:   test1
+          id:      1
+          type:    cat
+          refresh: true
+          body:
+            foo: bar
+
+  - do:
+      index:
+          index:   test2
+          id:      1
+          type:    _doc
+          refresh: true
+          body:
+            foo: bar
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test1
+        body:
+          query:
+            type:
+              value: cat
+  - match: { hits.total: 1 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test1
+        body:
+          query:
+            type:
+              value: dog
+  - match: { hits.total: 0 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test1
+        body:
+          query:
+            type:
+              value: _doc
+  - match: { hits.total: 0 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test1
+        body:
+          query:
+            type:
+              value: _default_
+  - match: { hits.total: 0 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test2
+        body:
+          query:
+            type:
+              value: _doc
+  - match: { hits.total: 1 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test2
+        body:
+          query:
+            type:
+              value: dog
+  - match: { hits.total: 0 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test2
+        body:
+          query:
+            type:
+              value: _default_
+  - match: { hits.total: 0 }

server/src/main/java/org/elasticsearch/index/IndexService.java

@@ -614,6 +614,7 @@ public QueryShardContext newQueryShardContext(
             indexCache.bitsetFilterCache(),
             indexFieldData::getForField,
             mapperService(),
+            mapperService().mappingLookup(),
             similarityService(),
             scriptService,
             xContentRegistry,

server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -116,7 +116,7 @@ private DocumentMapper(IndexSettings indexSettings,
         this.documentParser = documentParser;
         this.indexSettings = indexSettings;
         this.indexAnalyzers = indexAnalyzers;
-        this.fieldMappers = MappingLookup.fromMapping(this.mapping);
+        this.fieldMappers = MappingLookup.fromMapping(mapping, this::parse);
 
         try {
             mappingSource = new CompressedXContent(this, XContentType.JSON, ToXContent.EMPTY_PARAMS);

server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java

@@ -20,8 +20,10 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.common.regex.Regex;
+import org.elasticsearch.index.mapper.TypeFieldMapper.TypeFieldType;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -43,11 +45,16 @@ final class FieldTypeLookup {
      * For convenience, the set of copied fields includes the field itself.
      */
     private final Map<String, Set<String>> fieldToCopiedFields = new HashMap<>();
+    private final String type;
     private final DynamicKeyFieldTypeLookup dynamicKeyLookup;
 
-    FieldTypeLookup(Collection<FieldMapper> fieldMappers,
-                    Collection<FieldAliasMapper> fieldAliasMappers,
-                    Collection<RuntimeFieldType> runtimeFieldTypes) {
+    FieldTypeLookup(
+        String type,
+        Collection<FieldMapper> fieldMappers,
+        Collection<FieldAliasMapper> fieldAliasMappers,
+        Collection<RuntimeFieldType> runtimeFieldTypes
+    ) {
+        this.type = type;
         Map<String, DynamicKeyFieldMapper> dynamicKeyMappers = new HashMap<>();
 
         for (FieldMapper fieldMapper : fieldMappers) {
@@ -89,6 +96,10 @@ final class FieldTypeLookup {
      * Returns the mapped field type for the given field name.
      */
     MappedFieldType get(String field) {
+        if (field.equals(TypeFieldMapper.NAME)) {
+            return new TypeFieldType(type);
+        }
+
         MappedFieldType fieldType = fullNameToFieldType.get(field);
         if (fieldType != null) {
             return fieldType;
@@ -103,6 +114,10 @@ MappedFieldType get(String field) {
      * Returns a list of the full names of a simple match regex like pattern against full name and index name.
      */
     Set<String> simpleMatchToFullName(String pattern) {
+        if (Regex.isSimpleMatchPattern(pattern) == false) {
+            // no wildcards
+            return Collections.singleton(pattern);
+        }
         Set<String> fields = new HashSet<>();
         for (String field : fullNameToFieldType.keySet()) {
             if (Regex.simpleMatch(pattern, field)) {
@@ -125,6 +140,9 @@ Set<String> simpleMatchToFullName(String pattern) {
      * @return A set of paths in the _source that contain the field's values.
      */
     Set<String> sourcePaths(String field) {
+        if (fullNameToFieldType.isEmpty()) {
+            return org.elasticsearch.common.collect.Set.of();
+        }
         String resolvedField = field;
         int lastDotIndex = field.lastIndexOf('.');
         if (lastDotIndex > 0) {
@@ -149,4 +167,8 @@ Iterable<MappedFieldType> filter(Predicate<MappedFieldType> predicate) {
         return () -> Stream.concat(fullNameToFieldType.values().stream(), dynamicKeyLookup.fieldTypes())
             .distinct().filter(predicate).iterator();
     }
+
+    public String getType() {
+        return type;
+    }
 }

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.index.mapper;
 
 import com.carrotsearch.hppc.cursors.ObjectCursor;
+
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.Assertions;
 import org.elasticsearch.Version;
@@ -29,7 +30,6 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.logging.DeprecationLogger;
-import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
@@ -165,7 +165,7 @@ public MapperService(IndexSettings indexSettings, IndexAnalyzers indexAnalyzers,
     }
 
     public boolean hasNested() {
-        return this.mapper != null && this.mapper.hasNestedObjects();
+        return mappingLookup().hasNested();
     }
 
     public IndexAnalyzers getIndexAnalyzers() {
@@ -598,31 +598,23 @@ public DocumentMapperForType documentMapperWithAutoCreate(String type) {
      * Given the full name of a field, returns its {@link MappedFieldType}.
      */
     public MappedFieldType fieldType(String fullName) {
-        if (fullName.equals(TypeFieldMapper.NAME)) {
-            String type = mapper == null ? null : mapper.type();
-            return new TypeFieldMapper.TypeFieldType(type);
-        }
-        return this.mapper == null ? null : this.mapper.mappers().fieldTypes().get(fullName);
+        return mappingLookup().fieldTypes().get(fullName);
     }
 
     /**
      * Returns all the fields that match the given pattern. If the pattern is prefixed with a type
      * then the fields will be returned with a type prefix.
      */
     public Set<String> simpleMatchToFullName(String pattern) {
-        if (Regex.isSimpleMatchPattern(pattern) == false) {
-            // no wildcards
-            return Collections.singleton(pattern);
-        }
-        return this.mapper == null ? Collections.emptySet() : this.mapper.mappers().fieldTypes().simpleMatchToFullName(pattern);
+        return mappingLookup().simpleMatchToFullName(pattern);
     }
 
     /**
-     * Given a field name, returns its possible paths in the _source. For example,
-     * the 'source path' for a multi-field is the path to its parent field.
+     * {@code volatile} read a (mostly) immutable snapshot current mapping.
      */
-    public Set<String> sourcePath(String fullName) {
-        return this.mapper == null ? Collections.emptySet() : this.mapper.mappers().fieldTypes().sourcePaths(fullName);
+    public MappingLookup mappingLookup() {
+        DocumentMapper mapper = this.mapper;
+        return mapper == null ? MappingLookup.EMPTY : mapper.mappers();
     }
 
     /**
@@ -644,18 +636,7 @@ public ObjectMapper getObjectMapper(String name) {
      *                                  directly associated index-time analyzer
      */
     public NamedAnalyzer indexAnalyzer(String field, Function<String, NamedAnalyzer> unindexedFieldAnalyzer) {
-        if (this.mapper == null) {
-            return unindexedFieldAnalyzer.apply(field);
-        }
-        return this.mapper.mappers().indexAnalyzer(field, unindexedFieldAnalyzer);
-    }
-
-    public boolean containsBrokenAnalysis(String field) {
-        NamedAnalyzer a = indexAnalyzer(field, f -> null);
-        if (a == null) {
-            return false;
-        }
-        return a.containsBrokenAnalysis();
+        return mappingLookup().indexAnalyzer(field, unindexedFieldAnalyzer);
     }
 
     @Override
@@ -688,6 +669,7 @@ public synchronized List<String> reloadSearchAnalyzers(AnalysisRegistry registry
                 reloadedAnalyzers.add(analyzerName);
             }
         }
+        // TODO this should bust the cache somehow. Tracked in https://github.com/elastic/elasticsearch/issues/66722
         return reloadedAnalyzers;
     }
 }