Use a copy of the SearchExecutionContext for each Percolator execution by davidkyle · Pull Request #142765 · elastic/elasticsearch

davidkyle · 2026-02-20T15:11:11Z

The AutoPrefilteringScope member of SearchExecutionContext is not thread safe but Percolate accesses the member from multiple threads resulting in the stack trace below. SearchExecutionContext::NestedScope can also be used in non-thread safe manner in Percolator query.

This PR adds a copyForConcurrentUse() to SearchExecutionContext which creates a shallow copy of the context with new instances of the mutable members. These members are no longer shared between the threads removing the concurrent access issues.

Caused by: java.util.NoSuchElementException
	at java.util.LinkedList.removeFirst(LinkedList.java:282) ~[?:?]
	at java.util.LinkedList.pop(LinkedList.java:813) ~[?:?]
	at org.elasticsearch.index.query.support.AutoPrefilteringScope.pop(AutoPrefilteringScope.java:47) ~[elasticsearch-9.4.0-SNAPSHOT.jar:9.4.0-SNAPSHOT]
	at org.elasticsearch.index.query.support.AutoPrefilteringScope.close(AutoPrefilteringScope.java:59) ~[elasticsearch-9.4.0-SNAPSHOT.jar:9.4.0-SNAPSHOT]
	at org.elasticsearch.index.query.BoolQueryBuilder.addBooleanClauses(BoolQueryBuilder.java:333) ~[elasticsearch-9.4.0-SNAPSHOT.jar:9.4.0-SNAPSHOT]
	at org.elasticsearch.index.query.BoolQueryBuilder.doToQuery(BoolQueryBuilder.java:303) ~[elasticsearch-9.4.0-SNAPSHOT.jar:9.4.0-SNAPSHOT]
	at org.elasticsearch.index.query.AbstractQueryBuilder.toQuery(AbstractQueryBuilder.java:119) ~[elasticsearch-9.4.0-SNAPSHOT.jar:9.4.0-SNAPSHOT]
	at org.elasticsearch.percolator.PercolateQueryBuilder.lambda$createStore$9(PercolateQueryBuilder.java:577) ~[main/:?]
	at org.elasticsearch.percolator.PercolateQuery$1$1$1.matchDocId(PercolateQuery.java:138) ~[main/:?]
	at org.elasticsearch.percolator.PercolateQuery$BaseScorer$1.matches(PercolateQuery.java:301) ~[main/:?]
	at org.apache.lucene.search.Weight$DefaultBulkScorer.scoreTwoPhaseIterator(Weight.java:311) ~[lucene-core-10.3.2.jar:10.3.2 dadfd90b4401947f4d0387669dc94999fbb2c830 - 2025-11-13 10:41:29]
	at org.apache.lucene.search.Weight$DefaultBulkScorer.score(Weight.java:282) ~[lucene-core-10.3.2.jar:10.3.2 dadfd90b4401947f4d0387669dc94999fbb2c830 - 2025-11-13 10:41:29]
	at org.elasticsearch.search.internal.ContextIndexSearcher.searchLeaf(ContextIndexSearcher.java:483) ~[elasticsearch-9.4.0-SNAPSHOT.jar:9.4.0-SNAPSHOT]
	at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:809) ~[lucene-core-10.3.2.jar:10.3.2 dadfd90b4401947f4d0387669dc94999fbb2c830 - 2025-11-13 10:41:29]
	at org.elasticsearch.search.internal.ContextIndexSearcher.search(ContextIndexSearcher.java:407) ~[elasticsearch-9.4.0-SNAPSHOT.jar:9.4.0-SNAPSHOT]
	at org.elasticsearch.search.internal.ContextIndexSearcher.lambda$search$3(ContextIndexSearcher.java:385) ~[elasticsearch-9.4.0-SNAPSHOT.jar:9.4.0-SNAPSHOT]
	at java.util.concurrent.FutureTask.run(FutureTask.java:328) ~[?:?]
	at org.apache.lucene.search.TaskExecutor$Task.run(TaskExecutor.java:173) ~[lucene-core-10.3.2.jar:10.3.2 dadfd90b4401947f4d0387669dc94999fbb2c830 - 2025-11-13 10:41:29]
	at org.apache.lucene.search.TaskExecutor.lambda$invokeAll$1(TaskExecutor.java:98) ~[lucene-core-10.3.2.jar:10.3.2 dadfd90b4401947f4d0387669dc94999fbb2c830 - 2025-11-13 10:41:29]

Closes #141489

benwtrent

@dimitris-athanasiou @Mikep86

y'all might find this interesting.

benwtrent · 2026-02-23T12:34:29Z

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

+                    // PercolateQuery.QueryStore function from multiple threads. Here the
+                    // solution is to create an AutoPrefilteringScope for each invocation
+                    // of PercolateQuery.QueryStore
+                    var safeContext = new FilteredSearchExecutionContext(context) {


I am not sure that Percolator should be reading/writing from the auto-prefiltering scope at all.

I really dislike that this was just added for all boolean queries, when its only needed in very specific cases for knn searches.

I think this autoPrefilteringScope should be null and not available at all unless the very specific query asks for it. Otherwise we end up with weird latent bugs for code paths that aren't actually necessary or expected.

@benwtrent autoPrefilteringScope uses the same design pattern as nestedScope, which is one of the reasons we went with it. This leads to an immediate follow-up question: Are there similar potential issues with nested percolated queries?

Indeed, this issue seems to happen with nestedScope as well: #141489

I've been trying to reproduce this with percolated nested queries locally, but haven't had much luck yet. Such is the nature of concurrency bugs :/

I added a copyForConcurrentUse() method to SearchExecutionContext that addresses both the AutoPrefilteringScope and NestedScope issues.

I really dislike that this was just added for all boolean queries, when its only needed in very specific cases for knn searches.

I explored walking the query tree to find a knn query and only enabling auto pre-filtering if present and ended up with logic not dissimilar to this https://github.com/elastic/elasticsearch/blob/main/server/src/main/java/org/elasticsearch/index/query/support/AutoPrefilteringUtils.java#L45

There doesn't seem to be a blessed way to walk the query tree as getting the child queries for each query type has to be handled differently. It feels out of scope for this bug fix, we can explore the best way to handle this in another PR possibly by adding a List<QueryBuilder> subQueries() function to QueryBuilder

++ for exploring other ways to walk the query tree in a separate PR. I think this one should focus only on making SearchExecutionContext usable by percolator.

modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java

davidkyle · 2026-02-27T15:43:05Z

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

+                    // The context's NestedScope is also vulnerable to concurrent modification.
+                    // Use a cloned SearchExecutionContext for each thread with new instances of
+                    // the mutable fields.
+                    var safeContext = context.copyForConcurrentUse();


The context here is actually a FilteredSearchExecutionContext create at https://github.com/elastic/elasticsearch/blob/main/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java#L581

Mikep86

Partial review, I can finish up tomorrow

server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java

Mikep86

I took a closer look at this and I think the current solution is both too complicated and fundamentally flawed.

We create the FilteredSearchExecutionContext for the percolator query in wrapAllEmptyTextFields. However, when copying it in createStore, we generate a new FilteredSearchExecutionContext instance that does not persist the anonymously overridden methods. The whole reason for using FilteredSearchExecutionContext is lost by generating a copy.

There is a simpler solution that both generates a copy of SearchExecutionContext and persists the anonymously overridden methods: Configure the context late in createStore. The steps would be:

Refactor configureContext and downstream methods as necessary so they are callable from createStore.
Simplify the implementation of wrapAllEmptyTextFields to:

    static SearchExecutionContext wrapAllEmptyTextFields(SearchExecutionContext searchExecutionContext) {
        return new SearchExecutionContext(searchExecutionContext) {
            @Override
            public boolean fieldExistsInIndex(String fieldname) {
                return true;
            }
        };
    }

This both generates a copy of the context (fixing this bug) and anonymously overrides the desired methods in each copy.

Call configureContext in createStore
We can completely remove FilteredSearchExecutionContext at this point. It is only used by the percolator query, which no longer needs it.

@benwtrent WDYT?

This reverts commit e39e731.

davidkyle · 2026-03-09T14:48:31Z

Thanks for the simplification @Mikep86 I ended up doing some Yak shaving on this one.

The combination of FilteredSearchExecutionContext and the anonymous classes is broken and very hard to reason about. The FilteredX pattern should implement an interface so that if a new method is added then the filtered version will have to implement it. FilteredSearchExecutionContext does not do this so a developer could add new methods to SearchExecutionContext but FilteredSearchExecutionContext will not delegate those methods. I started to fix but it was much simpler to removed FilteredSearchExecutionContext as you noted. Also it turns out that Percolator is dependant on the implicit copy when deriving anonymous SearchExecutionContext classes.

Then I've pretty much done as suggested to move the creation of the new context to createStore() so each calling thread will have a copy.

benwtrent

This ended up in a nice place. Thank you @Mikep86 @davidkyle !!!

Mikep86

LGTM, nice clean fix!

Mikep86 · 2026-03-09T15:11:24Z

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

        }

-        executionContext = configureContext(executionContext, isMapUnmappedFieldAsText());
+        executionContext = PercolateQueryBuilder.newPercolateSearchContext(executionContext, isMapUnmappedFieldAsText());


It's great that we're using a SearchExecutionContext configured similarly at both document parsing time and at query time. There were probably other bugs we hadn't found yet caused by using differently configured contexts here vs. at query time...

davidkyle · 2026-03-10T12:46:23Z

💚 All backports created successfully

Status	Branch	Result
✅	9.3

Questions ?

Please refer to the Backport tool documentation

elastic#142765) (cherry picked from commit 22b4577)

…locations * upstream/main: (126 commits) Update KnnIndexTester to use more settings from datasets (elastic#143869) fix: dynamic template vector array is overridden by automatic dense_vector mapping (elastic#143733) ES|QL: Don't reuse the same alias for _fork column (elastic#143909) Close and initialize clients after each node upgrade in logsdb rolling upgrade tests. (elastic#143823) ESQL: Added GroupedTopNOperator for LIMIT BY, compute only (elastic#143476) Handle views in ResolveIndexAction (elastic#143561) Improve reindex rethrottle API in stateless (elastic#143771) Use a copy of the SearchExecutionContext for each Percolator execution (elastic#142765) Log the stacktrace when we encounter a deprecation warning for `default_metric` (elastic#143929) ESQL: evaluate ReferenceAttributes to potentially FieldAttributes for full-text functions restriction (elastic#143893) Add ClusterStateSerializationStats Serializatation Tests (elastic#142703) Adds Coordination Diagnostics Tests (elastic#142709) Upgrade Elasticsearch to Apache Lucene 10.4 (elastic#141882) ESQL: Add configurable bracket-based multi-value support for CSV reader (elastic#143890) time series es819 binary dv use up to a 1mb block size (elastic#143049) Dynamically enable / disable plugins in correspondence to stateless mode. (elastic#142147) ES|QL: Implement first/last_over_time for tdigest (elastic#143832) Document CHANGE_POINT limitation (elastic#143877) Fix OperationsOnSeqNoDisabledIndicesIT (elastic#143892) [Test] Test that sequence numbers are not pruned with retention lease (elastic#143825) ...

…ecution (#142765) (#143942) * Use a copy of the SearchExecutionContext for each Percolator execution (#142765) (cherry picked from commit 22b4577) * compilation fix

benwtrent · 2026-03-10T14:53:53Z

@davidkyle do we know if this bug also exists in 8.19? I wonder if we want to backport there as well.

davidkyle · 2026-03-10T15:11:27Z

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Questions ?

Please refer to the Backport tool documentation

elastic#142765) (cherry picked from commit 22b4577) # Conflicts: # modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java # modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java

#142765) (#143951) (cherry picked from commit 22b4577) # Conflicts: # modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java # modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java

Mikep86 · 2026-03-11T11:58:01Z

@davidkyle Sorry to pile on, but if we're backporting to 8.19, we should backport to 9.2 as well.

davidkyle · 2026-03-11T14:10:28Z

💚 All backports created successfully

Status	Branch	Result
✅	9.2

Questions ?

Please refer to the Backport tool documentation

elastic#142765) (cherry picked from commit 22b4577) # Conflicts: # modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java

davidkyle · 2026-03-11T14:34:38Z

Thanks for the shout @Mikep86. When I saw there are no more branches I wept for there were no more backports to make

…ecution (#142765) (#144027) * Use a copy of the SearchExecutionContext for each Percolator execution (#142765) (cherry picked from commit 22b4577) # Conflicts: # modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java * [CI] Auto commit changes from spotless * fix comp --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

elasticsearchmachine added the v9.4.0 label Feb 20, 2026

Create a new auto scope for each query store callback

27b4398

davidkyle force-pushed the auto-scope branch from ae52cce to 27b4398 Compare February 23, 2026 11:55

davidkyle added backport branch:9.3 v9.3.2 labels Feb 23, 2026

elasticsearchmachine removed the branch:9.3 label Feb 23, 2026

davidkyle added >bug :Search Relevance/Search Catch all for Search Relevance labels Feb 23, 2026

davidkyle marked this pull request as ready for review February 23, 2026 12:09

benwtrent reviewed Feb 23, 2026

View reviewed changes

davidkyle added 3 commits February 27, 2026 12:21

Merge branch 'main' into auto-scope

309e8fd

Use shallow copy of SEC

ba0dfce

fix da test

e515b29

davidkyle changed the title ~~Use the delegate's scope~~ Use a copy of the SEC for each percolator execution Feb 27, 2026

comment

c190831

davidkyle changed the title ~~Use a copy of the SEC for each percolator execution~~ Use a copy of the SearchExecutionContext for each Percolator execution Feb 27, 2026

davidkyle commented Feb 27, 2026

View reviewed changes

modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java Show resolved Hide resolved

davidkyle commented Feb 27, 2026

View reviewed changes

davidkyle added 2 commits February 27, 2026 16:52

Merge branch 'main' into auto-scope

ef26966

Merge branch 'main' into auto-scope

3fcb62a

Mikep86 reviewed Mar 5, 2026

View reviewed changes

server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java Outdated Show resolved Hide resolved

davidkyle added 2 commits March 6, 2026 16:40

another comment

566bdfd

clone FilteredSearchExecutionContext

e39e731

Mikep86 reviewed Mar 6, 2026

View reviewed changes

davidkyle added 5 commits March 9, 2026 11:57

Revert "clone FilteredSearchExecutionContext"

a93dbcf

This reverts commit e39e731.

Delete FilteredSEC

a060af5

reverting

2d17c8d

merge the 2 wrap methods

70ea69a

Merge branch 'main' into auto-scope

ec4c130

benwtrent approved these changes Mar 9, 2026

View reviewed changes

davidkyle added 2 commits March 9, 2026 14:55

tidy

8482bd6

better format

4abcbbe

Mikep86 reviewed Mar 9, 2026

View reviewed changes

Mikep86 approved these changes Mar 9, 2026

View reviewed changes

davidkyle enabled auto-merge (squash) March 9, 2026 15:47

Merge branch 'main' into auto-scope

089606f

davidkyle merged commit 22b4577 into elastic:main Mar 10, 2026
35 checks passed

davidkyle mentioned this pull request Mar 10, 2026

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

Merged

davidkyle added a commit to davidkyle/elasticsearch that referenced this pull request Mar 10, 2026

Use a copy of the SearchExecutionContext for each Percolator execution (

a5fe21d

elastic#142765) (cherry picked from commit 22b4577)

davidkyle mentioned this pull request Mar 10, 2026

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

Merged

davidkyle added the v8.19.13 label Mar 10, 2026

davidkyle mentioned this pull request Mar 11, 2026

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

Merged

davidkyle deleted the auto-scope branch March 12, 2026 10:28

Conversation

davidkyle commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benwtrent left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikep86 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikep86 left a comment

Choose a reason for hiding this comment

Uh oh!

davidkyle commented Mar 9, 2026

Uh oh!

benwtrent left a comment

Choose a reason for hiding this comment

Uh oh!

Mikep86 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidkyle commented Mar 10, 2026

💚 All backports created successfully

Questions ?

Uh oh!

benwtrent commented Mar 10, 2026

Uh oh!

davidkyle commented Mar 10, 2026

💚 All backports created successfully

Questions ?

Uh oh!

Mikep86 commented Mar 11, 2026

Uh oh!

davidkyle commented Mar 11, 2026

💚 All backports created successfully

Questions ?

Uh oh!

davidkyle commented Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

davidkyle commented Feb 20, 2026 •

edited

Loading