Fix circuit breaker leak in percolator query construction #144827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

drempapis merged 21 commits into elastic:main from drempapis:fix/cb_leak_percolator

Mar 26, 2026

docs/changelog/144827.yaml

-Original file line number
+Diff line change
@@ -0,0 +1,5 @@
+    area: Search
+    issues: []
+    pr: 144827
+    summary: Fix circuit breaker leak in percolator query construction
+    type: bug

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

-Original file line number
+Diff line change
@@ Expand Up / @@ -699,6 +699,21 @@ public boolean fieldExistsInIndex(String fieldname) { @@
                 public void addNamedQuery(String name, Query query) {
                     source.addNamedQuery(name, query);
                 }
+                @Override
+                public void addCircuitBreakerMemory(long bytes, String label) {
+                    source.addCircuitBreakerMemory(bytes, label);
+                }
+                @Override
+                public long getQueryConstructionMemoryUsed() {
+                    return source.getQueryConstructionMemoryUsed();
+                }
+                @Override
+                public void releaseQueryConstructionMemory() {
+                    source.releaseQueryConstructionMemory();
+                }
             };
             // This means that fields in the query need to exist in the mapping prior to registering this query
@@ Expand Down @@

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

-Original file line number
+Diff line change
@@ Expand Up / @@ -20,8 +20,10 @@ @@
     import org.apache.lucene.store.Directory;
     import org.elasticsearch.TransportVersion;
     import org.elasticsearch.cluster.metadata.IndexMetadata;
+    import org.elasticsearch.common.breaker.CircuitBreaker;
     import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
     import org.elasticsearch.common.settings.Settings;
+    import org.elasticsearch.common.unit.ByteSizeValue;
     import org.elasticsearch.core.CheckedFunction;
     import org.elasticsearch.index.IndexMode;
     import org.elasticsearch.index.IndexSettings;
@@ Expand All / @@ -39,8 +41,11 @@ @@
     import org.elasticsearch.index.mapper.Mapping;
     import org.elasticsearch.index.mapper.MappingLookup;
     import org.elasticsearch.index.mapper.TestDocumentParserContext;
+    import org.elasticsearch.index.query.QueryBuilder;
+    import org.elasticsearch.index.query.RegexpQueryBuilder;
     import org.elasticsearch.index.query.SearchExecutionContext;
     import org.elasticsearch.index.query.TermQueryBuilder;
+    import org.elasticsearch.index.query.WildcardQueryBuilder;
     import org.elasticsearch.script.field.BinaryDocValuesField;
     import org.elasticsearch.search.SearchModule;
     import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
@@ Expand All / @@ -56,6 +61,7 @@ @@
     import java.util.stream.Collectors;
     import static org.elasticsearch.index.query.SearchExecutionContextHelper.SHARD_SEARCH_STATS;
+    import static org.hamcrest.Matchers.greaterThan;
     public class QueryBuilderStoreTests extends ESTestCase {
@@ Expand Down Expand Up / @@ -119,7 +125,8 @@ public void testStoringQueryBuilders() throws IOException { @@
                     Collections.emptyList(),
                     IndexMode.STANDARD
                 );
-                SearchExecutionContext searchExecutionContext = new SearchExecutionContext(
+                CircuitBreaker breaker = newLimitedBreaker(ByteSizeValue.ofMb(100));
+                SearchExecutionContext baseContext = new SearchExecutionContext(
 ,
 ,
                     indexSettings,
@@ Expand All / @@ -143,6 +150,7 @@ public void testStoringQueryBuilders() throws IOException { @@
                     MapperMetrics.NOOP,
                     SHARD_SEARCH_STATS
                 );
+                SearchExecutionContext searchExecutionContext = new SearchExecutionContext(baseContext, breaker);
                 PercolateQuery.QueryStore queryStore = PercolateQueryBuilder.createStore(
                     fieldMapper.fieldType(),
@@ Expand All @@
                 }
             }
         }
+        public void testCircuitBreakerReleasedAfterPerDocumentQueryConstruction() throws IOException {
+            CircuitBreaker circuitBreaker = newLimitedBreaker(ByteSizeValue.ofMb(100));
+            String fieldName = "keyword_field";
+            QueryBuilder[] queryBuilders = new QueryBuilder[] {
+                new WildcardQueryBuilder(fieldName, "test*pattern*with*wildcards"),
+                new RegexpQueryBuilder(fieldName, ".*test.*regexp.*pattern.*"),
+                new WildcardQueryBuilder(fieldName, "another*wildcard*query"),
+                new RegexpQueryBuilder(fieldName, "prefix[0-9]+suffix"), };
+            try (Directory directory = newDirectory()) {
+                IndexWriterConfig config = new IndexWriterConfig(new WhitespaceAnalyzer());
+                config.setMergePolicy(NoMergePolicy.INSTANCE);
+                BinaryFieldMapper fieldMapper = PercolatorFieldMapper.Builder.createQueryBuilderFieldBuilder(
+                    MapperBuilderContext.root(false, false)
+                );
+                IndexVersion indexVersion = IndexVersion.current();
+                try (IndexWriter indexWriter = new IndexWriter(directory, config)) {
+                    for (QueryBuilder queryBuilder : queryBuilders) {
+                        DocumentParserContext documentParserContext = new TestDocumentParserContext();
+                        PercolatorFieldMapper.createQueryBuilderField(
+                            indexVersion,
+                            TransportVersion.current(),
+                            fieldMapper,
+                            queryBuilder,
+                            documentParserContext
+                        );
+                        indexWriter.addDocument(documentParserContext.doc());
+                    }
+                }
+                NamedWriteableRegistry writeableRegistry = writableRegistry();
+                XContentParserConfiguration parserConfig = parserConfig();
+                Settings indexSettingsSettings = indexSettings(indexVersion, 1, 1).build();
+                IndexSettings indexSettings = new IndexSettings(
+                    IndexMetadata.builder("test").settings(indexSettingsSettings).build(),
+                    Settings.EMPTY
+                );
+                KeywordFieldMapper keywordMapper = new KeywordFieldMapper.Builder(fieldName, indexSettings).build(
+                    MapperBuilderContext.root(false, false)
+                );
+                MappingLookup mappingLookup = MappingLookup.fromMappers(
+                    Mapping.EMPTY,
+                    List.of(keywordMapper),
+                    Collections.emptyList(),
+                    IndexMode.STANDARD
+                );
+                BytesBinaryIndexFieldData fieldData = new BytesBinaryIndexFieldData(
+                    fieldMapper.fullPath(),
+                    CoreValuesSourceType.KEYWORD,
+                    BinaryDocValuesField::new
+                );
+                BiFunction<MappedFieldType, FieldDataContext, IndexFieldData<?>> indexFieldDataLookup = (mft, fdc) -> fieldData;
+                SearchExecutionContext baseContext = new SearchExecutionContext(
+,
+,
+                    indexSettings,
+                    null,
+                    indexFieldDataLookup,
+                    null,
+                    mappingLookup,
+                    null,
+                    null,
+                    parserConfig,
+                    writeableRegistry,
+                    null,
+                    null,
+                    System::currentTimeMillis,
+                    null,
+                    null,
+                    () -> true,
+                    null,
+                    Collections.emptyMap(),
+                    null,
+                    MapperMetrics.NOOP,
+                    SHARD_SEARCH_STATS
+                );
+                SearchExecutionContext searchExecutionContext = new SearchExecutionContext(baseContext, circuitBreaker);
+                PercolateQuery.QueryStore queryStore = PercolateQueryBuilder.createStore(
+                    fieldMapper.fieldType(),
+                    false,
+                    searchExecutionContext
+                );
+                try (IndexReader indexReader = DirectoryReader.open(directory)) {
+                    LeafReaderContext leafContext = indexReader.leaves().get(0);
+                    CheckedFunction<Integer, Query, IOException> queries = queryStore.getQueries(leafContext);
+                    assertEquals(queryBuilders.length, leafContext.reader().numDocs());
+                    long baselineUsed = circuitBreaker.getUsed();
+                    for (int i = 0; i < queryBuilders.length; i++) {
+                        queries.apply(i);
+                        assertThat(
+                            "CB bytes should still be tracked (not leaked) after document " + i,
+                            circuitBreaker.getUsed(),
+                            greaterThan(baselineUsed)
+                        );
+                    }
+                    searchExecutionContext.releaseQueryConstructionMemory();
+                    assertEquals("All CB bytes must be released after the request-end release", baselineUsed, circuitBreaker.getUsed());
+                }
+            }
+        }
     }

server/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java

-Original file line number
+Diff line change
@@ Expand Up @@
                                 .createParser(searchExecutionContext.getParserConfig(), querySource)
                         ) {
                             QueryBuilder innerQueryBuilder = AbstractQueryBuilder.parseTopLevelQuery(parser);
-                            final ParsedQuery parsedQuery = searchExecutionContext.toQuery(innerQueryBuilder);
-                            collateMatch = Lucene.exists(searcher, parsedQuery.query());
+                            try {
+                                final ParsedQuery parsedQuery = searchExecutionContext.toQuery(innerQueryBuilder);
+                                collateMatch = Lucene.exists(searcher, parsedQuery.query());
+                            } finally {
+                                searchExecutionContext.releaseQueryConstructionMemory();
+                            }
                         }
                     }
                     if (collateMatch == false && collatePrune == false) {
@@ Expand Down @@

...rc/test/java/org/elasticsearch/index/query/InnerHitContextBuilderCircuitBreakerTests.java

-Original file line number
+Diff line change
@@ -0,0 +1,137 @@
+    /*
+     * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+     * or more contributor license agreements. Licensed under the "Elastic License
+     * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+     * Public License v 1"; you may not use this file except in compliance with, at
+     * your election, the "Elastic License 2.0", the "GNU Affero General Public
+     * License v3.0 only", or the "Server Side Public License, v 1".
+     */
+    package org.elasticsearch.index.query;
+    import org.apache.lucene.analysis.core.WhitespaceAnalyzer;
+    import org.apache.lucene.index.DirectoryReader;
+    import org.apache.lucene.index.IndexWriter;
+    import org.apache.lucene.index.IndexWriterConfig;
+    import org.apache.lucene.search.IndexSearcher;
+    import org.apache.lucene.store.ByteBuffersDirectory;
+    import org.apache.lucene.store.Directory;
+    import org.elasticsearch.cluster.metadata.IndexMetadata;
+    import org.elasticsearch.common.breaker.CircuitBreaker;
+    import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+    import org.elasticsearch.common.settings.Settings;
+    import org.elasticsearch.common.unit.ByteSizeValue;
+    import org.elasticsearch.index.IndexMode;
+    import org.elasticsearch.index.IndexSettings;
+    import org.elasticsearch.index.IndexVersion;
+    import org.elasticsearch.index.mapper.KeywordFieldMapper;
+    import org.elasticsearch.index.mapper.MapperBuilderContext;
+    import org.elasticsearch.index.mapper.MapperMetrics;
+    import org.elasticsearch.index.mapper.Mapping;
+    import org.elasticsearch.index.mapper.MappingLookup;
+    import org.elasticsearch.search.SearchModule;
+    import org.elasticsearch.search.fetch.subphase.InnerHitsContext;
+    import org.elasticsearch.search.internal.SearchContext;
+    import org.elasticsearch.test.ESTestCase;
+    import org.elasticsearch.xcontent.NamedXContentRegistry;
+    import java.io.IOException;
+    import java.util.Collections;
+    import java.util.List;
+    import static org.elasticsearch.index.query.SearchExecutionContextHelper.SHARD_SEARCH_STATS;
+    import static org.hamcrest.Matchers.equalTo;
+    import static org.hamcrest.Matchers.greaterThan;
+    public class InnerHitContextBuilderCircuitBreakerTests extends ESTestCase {
+        @Override
+        protected NamedWriteableRegistry writableRegistry() {
+            SearchModule searchModule = new SearchModule(Settings.EMPTY, Collections.emptyList());
+            return new NamedWriteableRegistry(searchModule.getNamedWriteables());
+        }
+        @Override
+        protected NamedXContentRegistry xContentRegistry() {
+            SearchModule searchModule = new SearchModule(Settings.EMPTY, Collections.emptyList());
+            return new NamedXContentRegistry(searchModule.getNamedXContents());
+        }
+        public void testCBTrackedDuringInnerHitsAndReleasedAtRequestEnd() throws IOException {
+            Directory dir = new ByteBuffersDirectory();
+            // Empty index – we just need a valid IndexSearcher for the context.
+            try (IndexWriter writer = new IndexWriter(dir, new IndexWriterConfig(new WhitespaceAnalyzer()))) {
+                // intentionally empty
+            }
+            try (DirectoryReader reader = DirectoryReader.open(dir)) {
+                IndexSearcher searcher = new IndexSearcher(reader);
+                IndexVersion indexVersion = IndexVersion.current();
+                Settings indexSettingsSettings = indexSettings(indexVersion, 1, 1).build();
+                IndexSettings indexSettings = new IndexSettings(
+                    IndexMetadata.builder("test").settings(indexSettingsSettings).build(),
+                    Settings.EMPTY
+                );
+                KeywordFieldMapper fieldMapper = new KeywordFieldMapper.Builder("field", indexSettings).build(
+                    MapperBuilderContext.root(false, false)
+                );
+                MappingLookup mappingLookup = MappingLookup.fromMappers(
+                    Mapping.EMPTY,
+                    List.of(fieldMapper),
+                    Collections.emptyList(),
+                    IndexMode.STANDARD
+                );
+                SearchExecutionContext baseCtx = new SearchExecutionContext(
+,
+,
+                    indexSettings,
+                    null,
+                    null,
+                    null,
+                    mappingLookup,
+                    null,
+                    null,
+                    parserConfig(),
+                    writableRegistry(),
+                    null,
+                    searcher,
+                    System::currentTimeMillis,
+                    null,
+                    null,
+                    () -> true,
+                    null,
+                    Collections.emptyMap(),
+                    null,
+                    MapperMetrics.NOOP,
+                    SHARD_SEARCH_STATS
+                );
+                CircuitBreaker cb = newLimitedBreaker(ByteSizeValue.ofMb(100));
+                SearchExecutionContext ctx = new SearchExecutionContext(baseCtx, cb);
+                QueryBuilder innerQuery = new WildcardQueryBuilder("field", "*test*pattern*");
+                InnerHitBuilder innerHitBuilder = new InnerHitBuilder("test_inner");
+                InnerHitContextBuilder builder = new InnerHitContextBuilder(innerQuery, innerHitBuilder, Collections.emptyMap()) {
+                    @Override
+                    protected void doBuild(SearchContext parentSearchContext, InnerHitsContext innerHitsContext) {}
+                };
+                InnerHitsContext.InnerHitSubContext subContext = org.mockito.Mockito.mock(InnerHitsContext.InnerHitSubContext.class);
+                long baselineUsed = cb.getUsed();
+                int iterations = 5;
+                for (int i = 0; i < iterations; i++) {
+                    builder.setupInnerHitsContext(ctx, subContext);
+                    assertThat(
+                        "CB bytes must still be tracked (not released early) after iteration " + i,
+                        cb.getUsed(),
+                        greaterThan(baselineUsed)
+                    );
+                }
+                ctx.releaseQueryConstructionMemory();
+                assertThat("All CB bytes must be released after the request-end release", cb.getUsed(), equalTo(baselineUsed));
+            }
+        }
+    }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix circuit breaker leak in percolator query construction #144827

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!