From af0a5fb4f339e65a7e4f04c20d4df5b452d55b0c Mon Sep 17 00:00:00 2001 From: Dimitris Rempapis Date: Tue, 24 Mar 2026 10:19:12 +0200 Subject: [PATCH 1/4] Fix circuit breaker leak in percolator query construction --- .../percolator/PercolateQueryBuilder.java | 8 +- .../percolator/QueryBuilderStoreTests.java | 126 ++++++++++++++++++ 2 files changed, 132 insertions(+), 2 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index d2cc9602c5df7..f473cabcc3c3d 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -578,8 +578,12 @@ static PercolateQuery.QueryStore createStore( return percolateShardContext.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name()); }); - queryBuilder = Rewriteable.rewrite(queryBuilder, percolateShardContext); - return queryBuilder.toQuery(percolateShardContext); + try { + queryBuilder = Rewriteable.rewrite(queryBuilder, percolateShardContext); + return queryBuilder.toQuery(percolateShardContext); + } finally { + percolateShardContext.releaseQueryConstructionMemory(); + } } else { return null; } diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java index dc1e72ccb629e..2c9aa59032b6b 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java @@ -20,7 +20,9 @@ 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.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.index.IndexMode; @@ -39,8 +41,13 @@ 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.indices.breaker.CircuitBreakerMetrics; +import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService; import org.elasticsearch.script.field.BinaryDocValuesField; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; @@ -162,4 +169,123 @@ public void testStoringQueryBuilders() throws IOException { } } } + + public void testCircuitBreakerReleasedAfterPerDocumentQueryConstruction() throws IOException { + Settings breakerSettings = Settings.builder() + .put("indices.breaker.request.limit", "100mb") + .put("indices.breaker.request.overhead", "1.0") + .build(); + ClusterSettings clusterSettings = new ClusterSettings(breakerSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + CircuitBreaker circuitBreaker = new HierarchyCircuitBreakerService( + CircuitBreakerMetrics.NOOP, + breakerSettings, + Collections.emptyList(), + clusterSettings + ).getBreaker(CircuitBreaker.REQUEST); + + 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> indexFieldDataLookup = (mft, fdc) -> fieldData; + + // Build a base context (no CB), then wrap it with the real circuit breaker. + SearchExecutionContext baseContext = new SearchExecutionContext( + 0, + 0, + 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 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); + assertEquals( + "Circuit breaker bytes must be fully released after processing percolator document " + i, + baselineUsed, + circuitBreaker.getUsed() + ); + } + } + } + } } From 540da876c8c24c26d81bc81d5204732663a09c53 Mon Sep 17 00:00:00 2001 From: Dimitris Rempapis Date: Tue, 24 Mar 2026 10:33:00 +0200 Subject: [PATCH 2/4] Update docs/changelog/144827.yaml --- docs/changelog/144827.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/144827.yaml diff --git a/docs/changelog/144827.yaml b/docs/changelog/144827.yaml new file mode 100644 index 0000000000000..61791428d01eb --- /dev/null +++ b/docs/changelog/144827.yaml @@ -0,0 +1,5 @@ +area: Search +issues: [] +pr: 144827 +summary: Fix circuit breaker leak in percolator query construction +type: bug From 886e26a7ca21c4cbcfc876baa689f75876d6f2c6 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Tue, 24 Mar 2026 08:47:03 +0000 Subject: [PATCH 3/4] [CI] Auto commit changes from spotless --- .../org/elasticsearch/percolator/QueryBuilderStoreTests.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java index 2c9aa59032b6b..4370292716fcd 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java @@ -188,8 +188,7 @@ public void testCircuitBreakerReleasedAfterPerDocumentQueryConstruction() throws 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"), - }; + new RegexpQueryBuilder(fieldName, "prefix[0-9]+suffix"), }; try (Directory directory = newDirectory()) { IndexWriterConfig config = new IndexWriterConfig(new WhitespaceAnalyzer()); From 05868b4f2dbbc9f942a35485bc099e12a49c031d Mon Sep 17 00:00:00 2001 From: Dimitris Rempapis Date: Wed, 25 Mar 2026 12:23:37 +0200 Subject: [PATCH 4/4] update after review --- .../percolator/PercolateQueryBuilder.java | 23 +- .../percolator/QueryBuilderStoreTests.java | 33 +-- .../suggest/phrase/PhraseSuggester.java | 8 +- ...rHitContextBuilderCircuitBreakerTests.java | 137 ++++++++++ .../PhraseSuggesterCircuitBreakerTests.java | 251 ++++++++++++++++++ .../esql/enrich/ExpressionQueryList.java | 38 +-- ...xpressionQueryListCircuitBreakerTests.java | 167 ++++++++++++ 7 files changed, 612 insertions(+), 45 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/index/query/InnerHitContextBuilderCircuitBreakerTests.java create mode 100644 server/src/test/java/org/elasticsearch/search/suggest/phrase/PhraseSuggesterCircuitBreakerTests.java create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryListCircuitBreakerTests.java diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index f473cabcc3c3d..e4e745148a716 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -578,12 +578,8 @@ static PercolateQuery.QueryStore createStore( return percolateShardContext.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name()); }); - try { - queryBuilder = Rewriteable.rewrite(queryBuilder, percolateShardContext); - return queryBuilder.toQuery(percolateShardContext); - } finally { - percolateShardContext.releaseQueryConstructionMemory(); - } + queryBuilder = Rewriteable.rewrite(queryBuilder, percolateShardContext); + return queryBuilder.toQuery(percolateShardContext); } else { return null; } @@ -703,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 diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java index 4370292716fcd..c9b82a1337353 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java @@ -22,8 +22,8 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.common.settings.ClusterSettings; 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; @@ -46,8 +46,6 @@ import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.index.query.WildcardQueryBuilder; -import org.elasticsearch.indices.breaker.CircuitBreakerMetrics; -import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService; import org.elasticsearch.script.field.BinaryDocValuesField; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; @@ -63,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 { @@ -126,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( 0, 0, indexSettings, @@ -150,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(), @@ -171,17 +172,7 @@ public void testStoringQueryBuilders() throws IOException { } public void testCircuitBreakerReleasedAfterPerDocumentQueryConstruction() throws IOException { - Settings breakerSettings = Settings.builder() - .put("indices.breaker.request.limit", "100mb") - .put("indices.breaker.request.overhead", "1.0") - .build(); - ClusterSettings clusterSettings = new ClusterSettings(breakerSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - CircuitBreaker circuitBreaker = new HierarchyCircuitBreakerService( - CircuitBreakerMetrics.NOOP, - breakerSettings, - Collections.emptyList(), - clusterSettings - ).getBreaker(CircuitBreaker.REQUEST); + CircuitBreaker circuitBreaker = newLimitedBreaker(ByteSizeValue.ofMb(100)); String fieldName = "keyword_field"; QueryBuilder[] queryBuilders = new QueryBuilder[] { @@ -237,7 +228,6 @@ public void testCircuitBreakerReleasedAfterPerDocumentQueryConstruction() throws ); BiFunction> indexFieldDataLookup = (mft, fdc) -> fieldData; - // Build a base context (no CB), then wrap it with the real circuit breaker. SearchExecutionContext baseContext = new SearchExecutionContext( 0, 0, @@ -278,12 +268,15 @@ public void testCircuitBreakerReleasedAfterPerDocumentQueryConstruction() throws long baselineUsed = circuitBreaker.getUsed(); for (int i = 0; i < queryBuilders.length; i++) { queries.apply(i); - assertEquals( - "Circuit breaker bytes must be fully released after processing percolator document " + i, - baselineUsed, - circuitBreaker.getUsed() + 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()); } } } diff --git a/server/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java b/server/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java index ebaf969b40aef..8ce7816a9568c 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java @@ -135,8 +135,12 @@ public Suggestion> innerExecute( .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) { diff --git a/server/src/test/java/org/elasticsearch/index/query/InnerHitContextBuilderCircuitBreakerTests.java b/server/src/test/java/org/elasticsearch/index/query/InnerHitContextBuilderCircuitBreakerTests.java new file mode 100644 index 0000000000000..95548d856cb57 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/query/InnerHitContextBuilderCircuitBreakerTests.java @@ -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( + 0, + 0, + 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)); + } + } +} diff --git a/server/src/test/java/org/elasticsearch/search/suggest/phrase/PhraseSuggesterCircuitBreakerTests.java b/server/src/test/java/org/elasticsearch/search/suggest/phrase/PhraseSuggesterCircuitBreakerTests.java new file mode 100644 index 0000000000000..0977b6c423472 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/search/suggest/phrase/PhraseSuggesterCircuitBreakerTests.java @@ -0,0 +1,251 @@ +/* + * 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.search.suggest.phrase; + +import org.apache.lucene.analysis.Analyzer; +import org.apache.lucene.analysis.LowerCaseFilter; +import org.apache.lucene.analysis.Tokenizer; +import org.apache.lucene.analysis.core.WhitespaceAnalyzer; +import org.apache.lucene.analysis.miscellaneous.PerFieldAnalyzerWrapper; +import org.apache.lucene.analysis.shingle.ShingleFilter; +import org.apache.lucene.analysis.standard.StandardTokenizer; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.TextField; +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.search.spell.SuggestMode; +import org.apache.lucene.store.ByteBuffersDirectory; +import org.apache.lucene.store.Directory; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.CharsRefBuilder; +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.index.query.SearchExecutionContext; +import org.elasticsearch.script.TemplateScript; +import org.elasticsearch.search.SearchModule; +import org.elasticsearch.search.suggest.Suggest; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.NamedXContentRegistry; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.elasticsearch.index.query.SearchExecutionContextHelper.SHARD_SEARCH_STATS; +import static org.hamcrest.Matchers.equalTo; + +public class PhraseSuggesterCircuitBreakerTests 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 testCBReleasedAfterEachCollateIteration() throws IOException { + Directory dir = new ByteBuffersDirectory(); + + Map analyzerMap = new HashMap<>(); + analyzerMap.put("body_ngram", new Analyzer() { + @Override + protected TokenStreamComponents createComponents(String fieldName) { + Tokenizer t = new StandardTokenizer(); + ShingleFilter sf = new ShingleFilter(t, 2, 3); + sf.setOutputUnigrams(false); + return new TokenStreamComponents(t, new LowerCaseFilter(sf)); + } + }); + analyzerMap.put("body", new Analyzer() { + @Override + protected TokenStreamComponents createComponents(String fieldName) { + Tokenizer t = new StandardTokenizer(); + return new TokenStreamComponents(t, new LowerCaseFilter(t)); + } + }); + PerFieldAnalyzerWrapper wrapper = new PerFieldAnalyzerWrapper(new WhitespaceAnalyzer(), analyzerMap); + + IndexWriterConfig conf = new IndexWriterConfig(wrapper); + try (IndexWriter writer = new IndexWriter(dir, conf)) { + for (String line : new String[] { "captain america", "american ace", "captain marvel", "american hero", "captain planet" }) { + Document doc = new Document(); + doc.add(new Field("body", line, TextField.TYPE_NOT_STORED)); + doc.add(new Field("body_ngram", line, TextField.TYPE_NOT_STORED)); + writer.addDocument(doc); + } + } + + 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 bodyMapper = new KeywordFieldMapper.Builder("body", indexSettings).build( + MapperBuilderContext.root(false, false) + ); + MappingLookup mappingLookup = MappingLookup.fromMappers( + Mapping.EMPTY, + List.of(bodyMapper), + Collections.emptyList(), + IndexMode.STANDARD + ); + + SearchExecutionContext baseCtx = new SearchExecutionContext( + 0, + 0, + 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); + + TemplateScript.Factory scriptFactory = params -> new TemplateScript(params) { + @Override + public String execute() { + return "{\"wildcard\":{\"body\":{\"value\":\"captain*\"}}}"; + } + }; + + PhraseSuggestionContext suggestion = new PhraseSuggestionContext(ctx); + suggestion.setField("body_ngram"); + suggestion.setAnalyzer(wrapper); + suggestion.setSize(5); + suggestion.setShardSize(5); + suggestion.setGramSize(2); + suggestion.setConfidence(0.0f); + suggestion.setMaxErrors(2.0f); + suggestion.setRequireUnigram(false); + suggestion.setCollateQueryScript(scriptFactory); + suggestion.setText(new BytesRef("captan amrica")); + + PhraseSuggestionContext.DirectCandidateGenerator generator = new PhraseSuggestionContext.DirectCandidateGenerator(); + generator.setField("body"); + generator.suggestMode(SuggestMode.SUGGEST_MORE_POPULAR); + generator.size(10); + generator.accuracy(0.3f); + generator.minWordLength(2); + suggestion.addGenerator(generator); + + assertEquals("CB must be zero before innerExecute", 0L, cb.getUsed()); + + Suggest.Suggestion> result = + PhraseSuggester.INSTANCE.innerExecute("test", suggestion, searcher, new CharsRefBuilder()); + + assertNotNull("innerExecute must return a result", result); + + assertThat( + "CB tracked bytes must be fully released after innerExecute (fix: release per correction)", + ctx.getQueryConstructionMemoryUsed(), + equalTo(0L) + ); + assertThat("Raw CB usage must be zero after innerExecute", cb.getUsed(), equalTo(0L)); + } + } + + public void testNoCBUsageWithoutCollateScript() throws IOException { + Directory dir = new ByteBuffersDirectory(); + try (IndexWriter writer = new IndexWriter(dir, new IndexWriterConfig(new WhitespaceAnalyzer()))) { + Document doc = new Document(); + doc.add(new Field("body", "hello world", TextField.TYPE_NOT_STORED)); + writer.addDocument(doc); + } + + try (DirectoryReader reader = DirectoryReader.open(dir)) { + IndexSearcher searcher = new IndexSearcher(reader); + CircuitBreaker cb = newLimitedBreaker(ByteSizeValue.ofMb(100)); + + IndexVersion indexVersion = IndexVersion.current(); + Settings indexSettingsSettings = indexSettings(indexVersion, 1, 1).build(); + IndexSettings indexSettings = new IndexSettings( + IndexMetadata.builder("test").settings(indexSettingsSettings).build(), + Settings.EMPTY + ); + SearchExecutionContext baseCtx = new SearchExecutionContext( + 0, + 0, + indexSettings, + null, + null, + null, + MappingLookup.EMPTY, + null, + null, + parserConfig(), + writableRegistry(), + null, + searcher, + System::currentTimeMillis, + null, + null, + () -> true, + null, + Collections.emptyMap(), + null, + MapperMetrics.NOOP, + SHARD_SEARCH_STATS + ); + SearchExecutionContext ctx = new SearchExecutionContext(baseCtx, cb); + + PhraseSuggestionContext suggestion = new PhraseSuggestionContext(ctx); + suggestion.setField("body"); + suggestion.setAnalyzer(new WhitespaceAnalyzer()); + suggestion.setSize(5); + suggestion.setShardSize(5); + suggestion.setGramSize(1); + suggestion.setConfidence(0.0f); + suggestion.setText(new BytesRef("hello")); + + PhraseSuggester.INSTANCE.innerExecute("test", suggestion, searcher, new CharsRefBuilder()); + assertThat("No CB bytes should be used when there is no collate script", cb.getUsed(), equalTo(0L)); + } + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryList.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryList.java index 53b74cef30144..ad7c1f0e653a8 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryList.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryList.java @@ -281,26 +281,30 @@ private void buildPreJoinFilter( */ @Override public Query getQuery(int position, Page inputPage, SearchExecutionContext searchExecutionContext) { - BooleanQuery.Builder builder = new BooleanQuery.Builder(); - for (QueryList queryList : queryLists) { - Query q = queryList.getQuery(position, inputPage, searchExecutionContext); - if (q == null) { - // if any of the matchFields are null, it means there is no match for this position - // A AND NULL is always NULL, so we can skip this position - return null; + try { + BooleanQuery.Builder builder = new BooleanQuery.Builder(); + for (QueryList queryList : queryLists) { + Query q = queryList.getQuery(position, inputPage, searchExecutionContext); + if (q == null) { + // if any of the matchFields are null, it means there is no match for this position + // A AND NULL is always NULL, so we can skip this position + return null; + } + builder.add(q, BooleanClause.Occur.FILTER); } - builder.add(q, BooleanClause.Occur.FILTER); - } - // also attach the pre-join filter if it exists - // Build queries from QueryBuilders dynamically to avoid caching stale IndexReader references - for (QueryBuilder queryBuilder : lucenePushableFilterBuilders) { - try { - builder.add(queryBuilder.toQuery(searchExecutionContext), BooleanClause.Occur.FILTER); - } catch (IOException e) { - throw new UncheckedIOException("Error while building query for Lucene pushable filter", e); + // also attach the pre-join filter if it exists + // Build queries from QueryBuilders dynamically to avoid caching stale IndexReader references + for (QueryBuilder queryBuilder : lucenePushableFilterBuilders) { + try { + builder.add(queryBuilder.toQuery(searchExecutionContext), BooleanClause.Occur.FILTER); + } catch (IOException e) { + throw new UncheckedIOException("Error while building query for Lucene pushable filter", e); + } } + return builder.build(); + } finally { + searchExecutionContext.releaseQueryConstructionMemory(); } - return builder.build(); } /** diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryListCircuitBreakerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryListCircuitBreakerTests.java new file mode 100644 index 0000000000000..3a20ce85a7ec7 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryListCircuitBreakerTests.java @@ -0,0 +1,167 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +package org.elasticsearch.xpack.esql.enrich; + +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.ClusterSettings; +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.index.query.SearchExecutionContext; +import org.elasticsearch.index.query.WildcardQueryBuilder; +import org.elasticsearch.search.SearchModule; +import org.elasticsearch.search.internal.AliasFilter; +import org.elasticsearch.test.ClusterServiceUtils; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.TestThreadPool; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xpack.esql.plugin.EsqlFlags; +import org.junit.AfterClass; +import org.junit.BeforeClass; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; + +import static org.elasticsearch.index.query.SearchExecutionContextHelper.SHARD_SEARCH_STATS; +import static org.hamcrest.Matchers.equalTo; + +public class ExpressionQueryListCircuitBreakerTests extends ESTestCase { + + private static TestThreadPool threadPool; + + @BeforeClass + public static void init() { + threadPool = new TestThreadPool("ExpressionQueryListCircuitBreakerTests"); + } + + @AfterClass + public static void cleanup() throws Exception { + terminate(threadPool); + threadPool = null; + } + + @Override + protected NamedXContentRegistry xContentRegistry() { + SearchModule searchModule = new SearchModule(Settings.EMPTY, Collections.emptyList()); + return new NamedXContentRegistry(searchModule.getNamedXContents()); + } + + @Override + protected NamedWriteableRegistry writableRegistry() { + SearchModule searchModule = new SearchModule(Settings.EMPTY, Collections.emptyList()); + return new NamedWriteableRegistry(searchModule.getNamedWriteables()); + } + + public void testCBReleasedAfterEachGetQueryCall() throws IOException { + Directory dir = new ByteBuffersDirectory(); + // Empty index – we only need a valid IndexSearcher for the context. + try (IndexWriter writer = new IndexWriter(dir, new IndexWriterConfig(new WhitespaceAnalyzer()))) { + // intentionally empty + } + + var registeredSettings = new HashSet<>(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + registeredSettings.addAll(EsqlFlags.ALL_ESQL_FLAGS_SETTINGS); + + try ( + DirectoryReader reader = DirectoryReader.open(dir); + var clusterService = ClusterServiceUtils.createClusterService( + threadPool, + new ClusterSettings(Settings.EMPTY, registeredSettings) + ) + ) { + + 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( + 0, + 0, + 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); + WildcardQueryBuilder pushedQuery = new WildcardQueryBuilder("field", "*test*pattern*"); + ExpressionQueryList queryList = ExpressionQueryList.fieldBasedJoin( + Collections.emptyList(), + ctx, + null, + pushedQuery, + clusterService, + AliasFilter.EMPTY + ); + + assertEquals("CB must be zero before any getQuery() call", 0L, cb.getUsed()); + + int positions = 10; + for (int i = 0; i < positions; i++) { + long cbBefore = cb.getUsed(); + + queryList.getQuery(i, null, ctx); + + assertThat( + "CB tracked bytes must be zero after getQuery() (position " + i + ")", + ctx.getQueryConstructionMemoryUsed(), + equalTo(0L) + ); + assertThat("Raw CB usage must return to baseline after getQuery() (position " + i + ")", cb.getUsed(), equalTo(cbBefore)); + } + assertThat("No CB bytes should remain after all positions", cb.getUsed(), equalTo(0L)); + } + } +}