Bust the request cache when the mapping changes (elastic#66295)

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 elastic#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>>:
 

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.99.99"
+      reason:  cache fixed in 8.0.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 }

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

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

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

@@ -115,7 +115,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

@@ -22,6 +22,7 @@
 import org.elasticsearch.common.regex.Regex;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -43,11 +44,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 +95,10 @@ final class FieldTypeLookup {
      * Returns the mapped field type for the given field name.
      */
     MappedFieldType get(String field) {
+        if (field.equals(TypeFieldType.NAME)) {
+            return new TypeFieldType(type);
+        }
+
         MappedFieldType fieldType = fullNameToFieldType.get(field);
         if (fieldType != null) {
             return fieldType;
@@ -103,6 +113,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 +139,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 Set.of();
+        }
         String resolvedField = field;
         int lastDotIndex = field.lastIndexOf('.');
         if (lastDotIndex > 0) {

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

@@ -26,7 +26,6 @@
 import org.elasticsearch.cluster.metadata.MappingMetadata;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.compress.CompressedXContent;
-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;
@@ -132,7 +131,7 @@ public MapperService(IndexSettings indexSettings, IndexAnalyzers indexAnalyzers,
     }
 
     public boolean hasNested() {
-        return this.mapper != null && this.mapper.hasNestedObjects();
+        return mappingLookup().hasNested();
     }
 
     public IndexAnalyzers getIndexAnalyzers() {
@@ -399,30 +398,23 @@ public DocumentMapperForType documentMapperWithAutoCreate() {
      * Given the full name of a field, returns its {@link MappedFieldType}.
      */
     public MappedFieldType fieldType(String fullName) {
-        if (fullName.equals(TypeFieldType.NAME)) {
-            return new TypeFieldType(this.mapper == null ? "_doc" : this.mapper.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();
     }
 
     /**
@@ -444,18 +436,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
@@ -504,6 +485,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;
     }
 }