[9.2] Use a copy of the SearchExecutionContext for each Percolator execution (#142765)

modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java

@@ -889,7 +889,11 @@ public void testWithMultiplePercolatorFields() throws Exception {
         assertThat(e.getCause().getMessage(), equalTo("a document can only contain one percolator query"));
     }
 
-    public void testPercolateQueryWithNestedDocuments() throws Exception {
+    /**
+     * Mapping for percolator tests that use nested "employee" documents.
+     * Includes query (percolator), id (keyword), companyname (text), and employee (nested with name).
+     */
+    private XContentBuilder nestedPercolatorMapping() throws IOException {
         XContentBuilder mapping = XContentFactory.jsonBuilder();
         mapping.startObject()
             .startObject("properties")
@@ -912,7 +916,11 @@ public void testPercolateQueryWithNestedDocuments() throws Exception {
             .endObject()
             .endObject()
             .endObject();
-        assertAcked(indicesAdmin().prepareCreate("test").setMapping(mapping));
+        return mapping;
+    }
+
+    public void testPercolateQueryWithNestedDocuments() throws Exception {
+        assertAcked(indicesAdmin().prepareCreate("test").setMapping(nestedPercolatorMapping()));
         prepareIndex("test").setId("q1")
             .setSource(
                 jsonBuilder().startObject()
@@ -1368,4 +1376,119 @@ public void testKnnQueryNotSupportedInPercolator() throws IOException {
         assertThat(exception.getMessage(), containsString("the [knn] query is unsupported inside a percolator"));
     }
 
+    public void testPercolatorBooleanQueriesWithConcurrency() throws Exception {
+        assertAcked(
+            indicesAdmin().prepareCreate("test")
+                .setSettings(Settings.builder().put(indexSettings()).put("index.number_of_shards", 1))
+                .setMapping("field1", "type=long", "query", "type=percolator")
+        );
+
+        prepareIndex("test").setId("1")
+            .setSource(
+                jsonBuilder().startObject()
+                    .field("query", boolQuery().must(rangeQuery("field1").from(10).to(12)).must(rangeQuery("field1").from(12).to(14)))
+                    .endObject()
+            )
+            .get();
+        prepareIndex("test").setId("2")
+            .setSource(
+                jsonBuilder().startObject()
+                    .field("query", boolQuery().must(rangeQuery("field1").from(3).to(4)).must(rangeQuery("field1").from(4).to(6)))
+                    .endObject()
+            )
+            .get();
+        prepareIndex("test").setId("3")
+            .setSource(
+                jsonBuilder().startObject()
+                    .field("query", boolQuery().must(rangeQuery("field1").from(10).to(12)).must(rangeQuery("field1").from(12).to(14)))
+                    .endObject()
+            )
+            .get();
+        prepareIndex("test").setId("4")
+            .setSource(
+                jsonBuilder().startObject()
+                    .field("query", boolQuery().must(rangeQuery("field1").from(3).to(4)).must(rangeQuery("field1").from(4).to(6)))
+                    .endObject()
+            )
+            .get();
+        prepareIndex("test").setId("5")
+            .setSource(
+                jsonBuilder().startObject()
+                    .field("query", boolQuery().must(rangeQuery("field1").from(10).to(12)).must(rangeQuery("field1").from(12).to(14)))
+                    .endObject()
+            )
+            .get();
+        prepareIndex("test").setId("6")
+            .setSource(
+                jsonBuilder().startObject()
+                    .field("query", boolQuery().must(rangeQuery("field1").from(3).to(4)).must(rangeQuery("field1").from(4).to(6)))
+                    .endObject()
+            )
+            .get();
+
+        indicesAdmin().prepareRefresh().get();
+
+        BytesReference source = BytesReference.bytes(jsonBuilder().startObject().field("field1", 12).endObject());
+        assertResponse(prepareSearch().setQuery(new PercolateQueryBuilder("query", source, XContentType.JSON)), response -> {
+            assertHitCount(response, 3);
+        });
+    }
+
+    public void testPercolatorNestedQueriesWithConcurrency() throws Exception {
+        assertAcked(
+            indicesAdmin().prepareCreate("test")
+                .setSettings(Settings.builder().put(indexSettings()).put("index.number_of_shards", 1))
+                .setMapping(nestedPercolatorMapping())
+        );
+
+        QueryBuilder nestedVirginia = QueryBuilders.nestedQuery(
+            "employee",
+            QueryBuilders.matchQuery("employee.name", "virginia potts").operator(Operator.AND),
+            ScoreMode.Avg
+        );
+        QueryBuilder nestedTony = QueryBuilders.nestedQuery(
+            "employee",
+            QueryBuilders.matchQuery("employee.name", "tony stark").operator(Operator.AND),
+            ScoreMode.Avg
+        );
+
+        prepareIndex("test").setId("1").setSource(jsonBuilder().startObject().field("query", nestedVirginia).endObject()).get();
+        prepareIndex("test").setId("2").setSource(jsonBuilder().startObject().field("query", nestedTony).endObject()).get();
+        prepareIndex("test").setId("3").setSource(jsonBuilder().startObject().field("query", nestedVirginia).endObject()).get();
+        prepareIndex("test").setId("4").setSource(jsonBuilder().startObject().field("query", nestedTony).endObject()).get();
+        prepareIndex("test").setId("5").setSource(jsonBuilder().startObject().field("query", nestedVirginia).endObject()).get();
+        prepareIndex("test").setId("6").setSource(jsonBuilder().startObject().field("query", nestedTony).endObject()).get();
+
+        indicesAdmin().prepareRefresh().get();
+
+        BytesReference source = BytesReference.bytes(
+            jsonBuilder().startObject()
+                .startArray("employee")
+                .startObject()
+                .field("name", "virginia potts")
+                .endObject()
+                .startObject()
+                .field("name", "tony stark")
+                .endObject()
+                .endArray()
+                .endObject()
+        );
+        assertResponse(prepareSearch().setQuery(new PercolateQueryBuilder("query", source, XContentType.JSON)), response -> {
+            assertHitCount(response, 6);
+        });
+
+        BytesReference sourceVirginiaOnly = BytesReference.bytes(
+            jsonBuilder().startObject()
+                .startArray("employee")
+                .startObject()
+                .field("name", "virginia potts")
+                .endObject()
+                .endArray()
+                .endObject()
+        );
+        assertResponse(prepareSearch().setQuery(new PercolateQueryBuilder("query", sourceVirginiaOnly, XContentType.JSON)), response -> {
+            assertHitCount(response, 3);
+        });
+    }
+
 }

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

@@ -495,9 +495,7 @@ protected Analyzer getWrappedAnalyzer(String fieldName) {
 
         PercolatorFieldMapper.PercolatorFieldType pft = (PercolatorFieldMapper.PercolatorFieldType) fieldType;
         String queryName = this.name != null ? this.name : pft.name();
-        SearchExecutionContext percolateShardContext = wrap(context);
-        percolateShardContext = PercolatorFieldMapper.configureContext(percolateShardContext, pft.mapUnmappedFieldsAsText);
-        PercolateQuery.QueryStore queryStore = createStore(pft.queryBuilderField, percolateShardContext);
+        PercolateQuery.QueryStore queryStore = createStore(pft.queryBuilderField, pft.mapUnmappedFieldsAsText, context);
 
         return pft.percolateQuery(queryName, queryStore, documents, docSearcher, excludeNestedDocuments, context.indexVersionCreated());
     }
@@ -536,7 +534,11 @@ static IndexSearcher createMultiDocumentSearcher(Analyzer analyzer, Collection<P
         }
     }
 
-    static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldType, SearchExecutionContext context) {
+    static PercolateQuery.QueryStore createStore(
+        MappedFieldType queryBuilderFieldType,
+        boolean mapUnmappedFieldsAsText,
+        SearchExecutionContext context
+    ) {
         IndexVersion indexVersion = context.indexVersionCreated();
         NamedWriteableRegistry registry = context.getWriteableRegistry();
         return ctx -> {
@@ -547,33 +549,36 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy
             }
             return docId -> {
                 if (binaryDocValues.advanceExact(docId)) {
+                    // create a shallow copy and set overrides
+                    var percolateShardContext = newPercolateSearchContext(context, mapUnmappedFieldsAsText);
+
                     BytesRef qbSource = binaryDocValues.binaryValue();
                     QueryBuilder queryBuilder = readQueryBuilder(qbSource, registry, indexVersion, () -> {
                         // query builder is written in an incompatible format, fall-back to reading it from source
-                        if (context.isSourceEnabled() == false) {
+                        if (percolateShardContext.isSourceEnabled() == false) {
                             throw new ElasticsearchException(
                                 "Unable to read percolator query. Original transport version is incompatible and source is "
                                     + "unavailable on index [{}].",
-                                context.index().getName()
+                                percolateShardContext.index().getName()
                             );
                         }
                         LOGGER.warn(
                             "Reading percolator query from source. For best performance, reindexing of index [{}] is required.",
-                            context.index().getName()
+                            percolateShardContext.index().getName()
                         );
-                        SourceProvider sourceProvider = context.createSourceProvider(new SourceFilter(null, null));
+                        SourceProvider sourceProvider = percolateShardContext.createSourceProvider(new SourceFilter(null, null));
                         Source source = sourceProvider.getSource(ctx, docId);
                         SourceToParse sourceToParse = new SourceToParse(
                             String.valueOf(docId),
                             source.internalSourceRef(),
                             source.sourceContentType()
                         );
 
-                        return context.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name());
+                        return percolateShardContext.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name());
                     });
 
-                    queryBuilder = Rewriteable.rewrite(queryBuilder, context);
-                    return queryBuilder.toQuery(context);
+                    queryBuilder = Rewriteable.rewrite(queryBuilder, percolateShardContext);
+                    return queryBuilder.toQuery(percolateShardContext);
                 } else {
                     return null;
                 }
@@ -623,8 +628,18 @@ private static QueryBuilder readQueryBuilder(
         }
     }
 
-    static SearchExecutionContext wrap(SearchExecutionContext delegate) {
-        return new SearchExecutionContext(delegate) {
+    /**
+     * Create a shallow copy of the {@code source} context with specific
+     * overrides for Percolator usage. The shallow copy makes the shared
+     * elements thread safe
+     * @param source The context to copy
+     * @param mapUnmappedFieldsAsText Controls unmapped fields behavior
+     * @return A copy of the source context with overrides
+     */
+    static SearchExecutionContext newPercolateSearchContext(SearchExecutionContext source, boolean mapUnmappedFieldsAsText) {
+        assert source.getClass().isAnonymousClass() == false
+            : "source must not be an anonymous class as overridden methods will be lost when a new SearchExecutionContext is created";
+        var wrapped = new SearchExecutionContext(source) {
 
             @Override
             public IndexReader getIndexReader() {
@@ -658,9 +673,9 @@ public <IFD extends IndexFieldData<?>> IFD getForField(
             ) {
                 IndexFieldData.Builder builder = fieldType.fielddataBuilder(
                     new FieldDataContext(
-                        delegate.getFullyQualifiedIndex().getName(),
-                        delegate.getIndexSettings(),
-                        delegate::lookup,
+                        source.getFullyQualifiedIndex().getName(),
+                        source.getIndexSettings(),
+                        source::lookup,
                         this::sourcePath,
                         fielddataOperation
                     )
@@ -670,26 +685,54 @@ public <IFD extends IndexFieldData<?>> IFD getForField(
                 return (IFD) builder.build(cache, circuitBreaker);
             }
 
+            // When expanding wildcard fields for term queries, we don't expand to fields that are empty.
+            // This is sane behavior for typical usage. But for percolator, the fields for the may not have any terms
+            // Consequently, we may erroneously skip expanding those term fields.
+            // This override allows mapped field values to expand via wildcard input, even if the field is empty in the shard.
+            @Override
+            public boolean fieldExistsInIndex(String fieldname) {
+                return true;
+            }
+
             @Override
             public void addNamedQuery(String name, Query query) {
-                delegate.addNamedQuery(name, query);
+                source.addNamedQuery(name, query);
             }
 
             @Override
             public void addCircuitBreakerMemory(long bytes, String label) {
-                delegate.addCircuitBreakerMemory(bytes, label);
+                source.addCircuitBreakerMemory(bytes, label);
             }
 
             @Override
             public long getQueryConstructionMemoryUsed() {
-                return delegate.getQueryConstructionMemoryUsed();
+                return source.getQueryConstructionMemoryUsed();
             }
 
             @Override
             public void releaseQueryConstructionMemory() {
-                delegate.releaseQueryConstructionMemory();
+                source.releaseQueryConstructionMemory();
             }
         };
+
+        // This means that fields in the query need to exist in the mapping prior to registering this query
+        // The reason that this is required, is that if a field doesn't exist then the query assumes defaults, which may be undesired.
+        //
+        // Even worse when fields mentioned in percolator queries do go added to map after the queries have been registered
+        // then the percolator queries don't work as expected any more.
+        //
+        // Query parsing can't introduce new fields in mappings (which happens when registering a percolator query),
+        // because field type can't be inferred from queries (like document do) so the best option here is to disallow
+        // the usage of unmapped fields in percolator queries to avoid unexpected behaviour
+        //
+        // if index.percolator.map_unmapped_fields_as_string is set to true, query can contain unmapped fields which will be mapped
+        // as an analyzed string.
+        wrapped.setAllowUnmappedFields(false);
+        wrapped.setMapUnmappedFieldAsString(mapUnmappedFieldsAsText);
+        // We need to rewrite queries with name to Lucene NamedQuery to find matched sub-queries of percolator query
+        wrapped.setRewriteToNamedQueries();
+
+        return wrapped;
     }
 
     @Override

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

@@ -56,7 +56,6 @@
 import org.elasticsearch.index.mapper.RangeType;
 import org.elasticsearch.index.mapper.SourceValueFetcher;
 import org.elasticsearch.index.mapper.ValueFetcher;
-import org.elasticsearch.index.query.FilteredSearchExecutionContext;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryShardException;
 import org.elasticsearch.index.query.Rewriteable;
@@ -410,7 +409,7 @@ public void parse(DocumentParserContext context) throws IOException {
             throw new IllegalArgumentException("a document can only contain one percolator query");
         }
 
-        executionContext = configureContext(executionContext, isMapUnmappedFieldAsText());
+        executionContext = PercolateQueryBuilder.newPercolateSearchContext(executionContext, isMapUnmappedFieldAsText());
 
         QueryBuilder queryBuilder = parseQueryBuilder(context);
         // Fetching of terms, shapes and indexed scripts happen during this rewrite:
@@ -512,27 +511,6 @@ void processQuery(Query query, DocumentParserContext context) {
         doc.add(new NumericDocValuesField(minimumShouldMatchFieldMapper.fullPath(), result.minimumShouldMatch));
     }
 
-    static SearchExecutionContext configureContext(SearchExecutionContext context, boolean mapUnmappedFieldsAsString) {
-        SearchExecutionContext wrapped = wrapAllEmptyTextFields(context);
-        // This means that fields in the query need to exist in the mapping prior to registering this query
-        // The reason that this is required, is that if a field doesn't exist then the query assumes defaults, which may be undesired.
-        //
-        // Even worse when fields mentioned in percolator queries do go added to map after the queries have been registered
-        // then the percolator queries don't work as expected any more.
-        //
-        // Query parsing can't introduce new fields in mappings (which happens when registering a percolator query),
-        // because field type can't be inferred from queries (like document do) so the best option here is to disallow
-        // the usage of unmapped fields in percolator queries to avoid unexpected behaviour
-        //
-        // if index.percolator.map_unmapped_fields_as_string is set to true, query can contain unmapped fields which will be mapped
-        // as an analyzed string.
-        wrapped.setAllowUnmappedFields(false);
-        wrapped.setMapUnmappedFieldAsString(mapUnmappedFieldsAsString);
-        // We need to rewrite queries with name to Lucene NamedQuery to find matched sub-queries of percolator query
-        wrapped.setRewriteToNamedQueries();
-        return wrapped;
-    }
-
     @Override
     public Iterator<Mapper> iterator() {
         return Arrays.<Mapper>asList(
@@ -577,17 +555,4 @@ static byte[] encodeRange(String rangeFieldName, byte[] minEncoded, byte[] maxEn
         System.arraycopy(maxEncoded, 0, bytes, BinaryRange.BYTES + offset, maxEncoded.length);
         return bytes;
     }
-
-    // When expanding wildcard fields for term queries, we don't expand to fields that are empty.
-    // This is sane behavior for typical usage. But for percolator, the fields for the may not have any terms
-    // Consequently, we may erroneously skip expanding those term fields.
-    // This override allows mapped field values to expand via wildcard input, even if the field is empty in the shard.
-    static SearchExecutionContext wrapAllEmptyTextFields(SearchExecutionContext searchExecutionContext) {
-        return new FilteredSearchExecutionContext(searchExecutionContext) {
-            @Override
-            public boolean fieldExistsInIndex(String fieldname) {
-                return true;
-            }
-        };
-    }
 }