This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new ae34ff9c5a7 Cache parsed Lucene queries across segment readers (#19865)
ae34ff9c5a7 is described below
commit ae34ff9c5a7f994e876f6dd269a78f4110939ef0
Author: Arunkumar Saravanan <[email protected]>
AuthorDate: Sat Oct 17 03:08:21 2026 +0530
Cache parsed Lucene queries across segment readers (#19865)
---
.../operator/filter/TextMatchFilterOperator.java | 42 ++--
.../org/apache/pinot/core/plan/FilterPlanNode.java | 6 +-
.../core/query/request/context/QueryContext.java | 12 +-
.../index/readers/text/LuceneTextIndexReader.java | 104 +++++++-
.../readers/text/LuceneTextIndexCountTest.java | 2 +-
.../text/LuceneTextIndexQueryCacheTest.java | 264 +++++++++++++++++++++
.../segment/spi/index/reader/QueryScopedCache.java | 47 ++++
.../segment/spi/index/reader/TextIndexReader.java | 19 ++
8 files changed, 463 insertions(+), 33 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java
index 59e6a88ce21..e971b7bf9b6 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java
@@ -20,12 +20,14 @@ package org.apache.pinot.core.operator.filter;
import com.google.common.base.CaseFormat;
import java.util.List;
+import javax.annotation.Nullable;
import org.apache.pinot.common.request.context.predicate.TextMatchPredicate;
import org.apache.pinot.core.common.BlockDocIdSet;
import org.apache.pinot.core.common.Operator;
import org.apache.pinot.core.operator.ExplainAttributeBuilder;
import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
import org.apache.pinot.segment.spi.index.reader.MultiColumnTextIndexReader;
+import org.apache.pinot.segment.spi.index.reader.QueryScopedCache;
import org.apache.pinot.segment.spi.index.reader.TextIndexReader;
import org.apache.pinot.spi.trace.FilterType;
import org.apache.pinot.spi.trace.InvocationRecording;
@@ -42,13 +44,22 @@ public class TextMatchFilterOperator extends
BaseFilterOperator {
private final String _column;
private final TextIndexReader _textIndexReader;
private final TextMatchPredicate _predicate;
+ /// Lets the segments of one request share a parsed query instead of each
parsing it again. Null
+ /// when no query scope is available, which simply means no sharing.
+ private final QueryScopedCache _queryScopedCache;
public TextMatchFilterOperator(String column, TextIndexReader
textIndexReader, TextMatchPredicate predicate,
int numDocs) {
+ this(column, textIndexReader, predicate, numDocs, null);
+ }
+
+ public TextMatchFilterOperator(String column, TextIndexReader
textIndexReader, TextMatchPredicate predicate,
+ int numDocs, @Nullable QueryScopedCache queryScopedCache) {
super(getSearchableDocCount(textIndexReader, numDocs), false);
_column = column;
_textIndexReader = textIndexReader;
_predicate = predicate;
+ _queryScopedCache = queryScopedCache;
}
private static int getSearchableDocCount(TextIndexReader reader, int
numDocs) {
@@ -57,19 +68,17 @@ public class TextMatchFilterOperator extends
BaseFilterOperator {
}
public TextMatchFilterOperator(TextIndexReader textIndexReader,
TextMatchPredicate predicate, int numDocs) {
- this(null, textIndexReader, predicate, numDocs);
+ this(null, textIndexReader, predicate, numDocs, null);
+ }
+
+ public TextMatchFilterOperator(TextIndexReader textIndexReader,
TextMatchPredicate predicate, int numDocs,
+ @Nullable QueryScopedCache queryScopedCache) {
+ this(null, textIndexReader, predicate, numDocs, queryScopedCache);
}
@Override
protected BlockDocIdSet getTrues() {
- if (_textIndexReader.isMultiColumn()) {
- return BitmapDocIdSet.create(
- ((MultiColumnTextIndexReader) _textIndexReader).getDocIds(_column,
_predicate.getValue(),
- _predicate.getOptions()), _numDocs);
- } else {
- return
BitmapDocIdSet.create(_textIndexReader.getDocIds(_predicate.getValue(),
_predicate.getOptions()),
- _numDocs);
- }
+ return BitmapDocIdSet.create(getMatchingDocIds(), _numDocs);
}
@Override
@@ -85,7 +94,8 @@ public class TextMatchFilterOperator extends
BaseFilterOperator {
} else {
// Ask the index for the size of the match set rather than for the match
set itself: building a bitmap of
// every matching doc just to read its cardinality is the dominant cost
of a count over a frequent term.
- return _textIndexReader.getNumMatchingDocs(_predicate.getValue(),
_predicate.getOptions());
+ return _textIndexReader.getNumMatchingDocs(_predicate.getValue(),
_predicate.getOptions(),
+ _queryScopedCache);
}
}
@@ -96,14 +106,18 @@ public class TextMatchFilterOperator extends
BaseFilterOperator {
@Override
public BitmapCollection getBitmaps() {
- ImmutableRoaringBitmap bitmap = _textIndexReader.isMultiColumn()
- ? ((MultiColumnTextIndexReader) _textIndexReader).getDocIds(_column,
_predicate.getValue(),
- _predicate.getOptions()) :
_textIndexReader.getDocIds(_predicate.getValue(), _predicate.getOptions());
-
+ ImmutableRoaringBitmap bitmap = getMatchingDocIds();
record(bitmap);
return new BitmapCollection(_numDocs, false, bitmap);
}
+ private ImmutableRoaringBitmap getMatchingDocIds() {
+ return _textIndexReader.isMultiColumn()
+ ? ((MultiColumnTextIndexReader) _textIndexReader).getDocIds(_column,
_predicate.getValue(),
+ _predicate.getOptions())
+ : _textIndexReader.getDocIds(_predicate.getValue(),
_predicate.getOptions(), _queryScopedCache);
+ }
+
@Override
public List<Operator> getChildOperators() {
return List.of();
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java
b/pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java
index 74bdb9f2a38..efbd73e8dfb 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java
@@ -307,9 +307,11 @@ public class FilterPlanNode implements PlanNode {
"Cannot apply TEXT_MATCH on column: %s without text index",
column);
if (textIndexReader.isMultiColumn()) {
- return new TextMatchFilterOperator(column, textIndexReader,
(TextMatchPredicate) predicate, numDocs);
+ return new TextMatchFilterOperator(column, textIndexReader,
(TextMatchPredicate) predicate, numDocs,
+ _queryContext);
} else {
- return new TextMatchFilterOperator(textIndexReader,
(TextMatchPredicate) predicate, numDocs);
+ return new TextMatchFilterOperator(textIndexReader,
(TextMatchPredicate) predicate, numDocs,
+ _queryContext);
}
case REGEXP_LIKE:
// PredicateEvaluatorProvider handles FST/IFST upgrade
internally when the dictionary is usable for
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java
b/pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java
index dbbba9a8bb7..24548c551dd 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java
@@ -41,6 +41,7 @@ import
org.apache.pinot.core.query.aggregation.function.AggregationFunction;
import
org.apache.pinot.core.query.aggregation.function.AggregationFunctionFactory;
import org.apache.pinot.core.util.GroupByUtils;
import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.QueryScopedCache;
import org.apache.pinot.spi.config.table.FieldConfig;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.utils.CommonConstants;
@@ -64,7 +65,7 @@ import org.apache.pinot.spi.utils.CommonConstants.Server;
/// - We can also add some helper variables or methods in the context classes
which can be shared for all segments to
/// reduce the repetitive work for each segment.
@SuppressWarnings({"rawtypes", "unchecked"})
-public class QueryContext {
+public class QueryContext implements QueryScopedCache {
private final String _tableName;
private final QueryContext _subquery;
private final List<ExpressionContext> _selectExpressions;
@@ -612,14 +613,7 @@ public class QueryContext {
return _sortAggregateSequentialCombineNumSegmentsThreshold;
}
- /// Gets or computes a value of type `V` associated with a key of type `K`
so that it can be shared
- /// within the scope of a query.
- /// @param type the type of the value produced, guarantees type pollution is
impossible.
- /// @param key the key used to determine if the value has already been
computed.
- /// @param mapper A function to apply the first time a key is encountered to
construct the value.
- /// @param <K> the key type
- /// @param <V> the value type
- /// @return the shared value
+ @Override
public <K, V> V getOrComputeSharedValue(Class<V> type, K key, Function<K, V>
mapper) {
return ((ConcurrentHashMap<K, V>) _sharedValues.computeIfAbsent(type, k ->
new ConcurrentHashMap<>()))
.computeIfAbsent(key, mapper);
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java
index 8fa450766b1..6e73e11f10d 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java
@@ -50,6 +50,7 @@ import
org.apache.pinot.segment.local.utils.LuceneTextIndexUtils;
import org.apache.pinot.segment.spi.V1Constants;
import org.apache.pinot.segment.spi.index.TextIndexConfig;
import org.apache.pinot.segment.spi.index.TextIndexConfig.DocIdTranslatorMode;
+import org.apache.pinot.segment.spi.index.reader.QueryScopedCache;
import org.apache.pinot.segment.spi.index.reader.TextIndexReader;
import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
import org.apache.pinot.segment.spi.store.SegmentDirectoryPaths;
@@ -79,6 +80,9 @@ public class LuceneTextIndexReader implements TextIndexReader
{
private final String _queryParserClass;
private Constructor<QueryParserBase> _queryParserClassConstructor;
private boolean _enablePrefixSuffixMatchingInPhraseQueries = false;
+ /// Everything about this column that changes how a search string parses.
Two readers may share a
+ /// parsed query only when these are equal. Computed once; immutable, with
structural equality.
+ private final ParseConfig _parseConfig;
public LuceneTextIndexReader(String column, File indexDir, int numDocs,
TextIndexConfig config) {
_column = column;
@@ -112,6 +116,8 @@ public class LuceneTextIndexReader implements
TextIndexReader {
_queryParserClass = config.getLuceneQueryParserClass();
_queryParserClassConstructor =
TextIndexUtils.getQueryParserWithStringAndAnalyzerTypeConstructor(_queryParserClass);
+ // after the properties-file override above, so it reflects the
configuration actually in force
+ _parseConfig = buildParseConfig(_column, config);
LOGGER.info("Successfully read lucene index for {} from {}", _column,
indexDir);
} catch (Exception e) {
LOGGER.error("Failed to instantiate Lucene text index reader for column
{}, exception {}", column,
@@ -183,6 +189,8 @@ public class LuceneTextIndexReader implements
TextIndexReader {
_queryParserClass = config.getLuceneQueryParserClass();
_queryParserClassConstructor =
TextIndexUtils.getQueryParserWithStringAndAnalyzerTypeConstructor(_queryParserClass);
+ // after the properties-file override above, so it reflects the
configuration actually in force
+ _parseConfig = buildParseConfig(_column, config);
LOGGER.info("Successfully read lucene index for {} from buffer",
_column);
} catch (Exception e) {
@@ -225,10 +233,16 @@ public class LuceneTextIndexReader implements
TextIndexReader {
@Override
public MutableRoaringBitmap getDocIds(String searchQuery, @Nullable String
optionsString) {
+ return getDocIds(searchQuery, optionsString, null);
+ }
+
+ @Override
+ public MutableRoaringBitmap getDocIds(String searchQuery, @Nullable String
optionsString,
+ @Nullable QueryScopedCache queryScopedCache) {
MutableRoaringBitmap docIds = new MutableRoaringBitmap();
Collector docIDCollector = new LuceneDocIdCollector(docIds,
_docIdTranslator);
try {
- _indexSearcher.search(buildQuery(searchQuery, optionsString),
docIDCollector);
+ _indexSearcher.search(buildQuery(searchQuery, optionsString,
queryScopedCache), docIDCollector);
return docIds;
} catch (QueryException e) {
throw e;
@@ -243,11 +257,49 @@ public class LuceneTextIndexReader implements
TextIndexReader {
}
}
+ /// Identifies a parsing configuration. A record so that equality is
structural: comparing the
+ /// rendered text of these lists instead would conflate `["failed",
"order"]` with
+ /// `["failed, order"]`, which are different stop-word sets that parse the
same string differently.
+ private record ParseConfig(String column, String analyzerClass, List<String>
analyzerArgs,
+ List<String> analyzerArgTypes, List<String>
stopWordsInclude,
+ List<String> stopWordsExclude, boolean
caseSensitive, String queryParserClass,
+ boolean useANDForMultiTermQueries, boolean
prefixSuffixMatchingInPhraseQueries) {
+ private ParseConfig {
+ analyzerArgs = copy(analyzerArgs);
+ analyzerArgTypes = copy(analyzerArgTypes);
+ stopWordsInclude = copy(stopWordsInclude);
+ stopWordsExclude = copy(stopWordsExclude);
+ }
+
+ private static List<String> copy(@Nullable List<String> values) {
+ return values == null ? List.of() : List.copyOf(values);
+ }
+ }
+
+ /// Cache key for one parsed query: the configuration plus the
request-supplied text.
+ private record ParsedQueryKey(ParseConfig parseConfig, String searchQuery,
@Nullable String optionsString) {
+ }
+
+ /// Carries a checked parse failure out of the cache's mapper so the
original exception, and in
+ /// particular a [QueryException]'s error code, survives instead of being
wrapped.
+ private static final class ParseFailure extends RuntimeException {
+ ParseFailure(Exception cause) {
+ super(cause);
+ }
+ }
+
+ private static ParseConfig buildParseConfig(String column, TextIndexConfig
config) {
+ return new ParseConfig(column, config.getLuceneAnalyzerClass(),
config.getLuceneAnalyzerClassArgs(),
+ config.getLuceneAnalyzerClassArgTypes(), config.getStopWordsInclude(),
config.getStopWordsExclude(),
+ config.isCaseSensitive(), config.getLuceneQueryParserClass(),
config.isUseANDForMultiTermQueries(),
+ config.isEnablePrefixSuffixMatchingInPhraseQueries());
+ }
+
/// Parses a search string into a Lucene query, applying the configured
parser and options.
///
/// Shared by the doc-id and the count paths so that a `count(*)` and the
filter it counts can never
/// interpret the same query differently.
- private Query buildQuery(String searchQuery)
+ private Query parseQueryWithoutOptions(String searchQuery)
throws Exception {
// Lucene query parsers are generally stateful and a new instance must be
created per query.
QueryParserBase parser = _queryParserClassConstructor.newInstance(_column,
_analyzer);
@@ -269,6 +321,12 @@ public class LuceneTextIndexReader implements
TextIndexReader {
@Override
public int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString) {
+ return getNumMatchingDocs(searchQuery, optionsString, null);
+ }
+
+ @Override
+ public int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString,
+ @Nullable QueryScopedCache queryScopedCache) {
try {
// A Lucene index larger than the segment means the doc-id mapping is
broken -- a state getDocIds
// surfaces as an IndexOutOfBoundsException from the translator.
Counting does not consult the
@@ -277,10 +335,10 @@ public class LuceneTextIndexReader implements
TextIndexReader {
// `context.docBase + doc`, which is maxDoc() space, and with deletions
numDocs() < maxDoc() would let a
// broken index slip through. Fall back so that it keeps failing loudly.
if (_indexReader.maxDoc() > _numDocs) {
- return getDocIds(searchQuery, optionsString).getCardinality();
+ return getDocIds(searchQuery, optionsString,
queryScopedCache).getCardinality();
}
return LuceneTextIndexUtils.countWithoutMaterializing(_indexSearcher,
- buildQuery(searchQuery, optionsString));
+ buildQuery(searchQuery, optionsString, queryScopedCache));
} catch (QueryException e) {
// A timeout or accountant kill arrives as a QueryException; wrapping it
would erase its error code,
// since only QueryException keeps its code through
BaseSingleBlockCombineOperator.
@@ -292,19 +350,51 @@ public class LuceneTextIndexReader implements
TextIndexReader {
}
}
- /// Parses a search string into a Lucene query, honouring the options string
when one is given.
+ /// Parses a search string into a Lucene query, honouring the options string
when one is given, and
+ /// reusing the query already parsed for this request when a cache is given.
///
/// Both [#getDocIds(String, String)] and [#getNumMatchingDocs] go through
this method, so a `count(*)` and
/// the filter it counts cannot interpret the same query differently. Keep
it that way: any new entry point
/// that parses a search string itself reintroduces that divergence.
+ ///
+ /// Every segment of a request parses the same string against the same
configuration, so without a
+ /// cache the work is repeated once per segment. With a cache the string is
parsed once per effective
+ /// parsing configuration rather than once per segment: one request can span
segments whose text
+ /// indexes are configured differently, and those keep their own entries. A
parse that throws caches
+ /// nothing, so the next segment retries and fails the same way.
+ ///
+ /// The returned query may be shared: Lucene queries are immutable and
[Query#rewrite] does not
+ /// mutate them, so a multi-term query is still expanded per segment against
that segment's own
+ /// term dictionary.
+ @VisibleForTesting
+ Query buildQuery(String searchQuery, @Nullable String optionsString,
@Nullable QueryScopedCache queryScopedCache)
+ throws Exception {
+ if (queryScopedCache == null) {
+ return parseQuery(searchQuery, optionsString);
+ }
+ ParsedQueryKey key = new ParsedQueryKey(_parseConfig, searchQuery,
optionsString);
+ try {
+ return queryScopedCache.getOrComputeSharedValue(Query.class, key, k -> {
+ try {
+ return parseQuery(k.searchQuery(), k.optionsString());
+ } catch (Exception e) {
+ throw new ParseFailure(e);
+ }
+ });
+ } catch (ParseFailure e) {
+ throw (Exception) e.getCause();
+ }
+ }
+
+ /// Parses unconditionally, bypassing any cache.
@VisibleForTesting
- Query buildQuery(String searchQuery, @Nullable String optionsString)
+ Query parseQuery(String searchQuery, @Nullable String optionsString)
throws Exception {
if (hasOptions(optionsString)) {
LuceneTextIndexUtils.LuceneTextIndexOptions options =
LuceneTextIndexUtils.createOptions(optionsString);
return LuceneTextIndexUtils.createQueryParserWithOptions(searchQuery,
options, _column, _analyzer);
}
- return buildQuery(searchQuery);
+ return parseQueryWithoutOptions(searchQuery);
}
/// Whether `optionsString` selects the options-driven parser, i.e. it is
non-blank and parses to at least
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java
index a6f4e144beb..92ba615cb22 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java
@@ -280,7 +280,7 @@ public class LuceneTextIndexCountTest {
assertTrue(indexReader.leaves().size() > 1, "fixture is not multi-leaf");
IndexSearcher searcher = new IndexSearcher(indexReader);
try (LuceneTextIndexReader reader = new LuceneTextIndexReader(COLUMN,
INDEX_DIR, DOCS.length, Map.of())) {
- Query parsed = reader.buildQuery(query, options);
+ Query parsed = reader.parseQuery(query, options);
assertEquals(LuceneTextIndexUtils.countWithoutMaterializing(searcher,
parsed), searcher.count(parsed),
"multi-leaf count disagreed with IndexSearcher#count for query: "
+ query);
}
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexQueryCacheTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexQueryCacheTest.java
new file mode 100644
index 00000000000..e41e977237e
--- /dev/null
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexQueryCacheTest.java
@@ -0,0 +1,264 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.segment.index.readers.text;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
+import org.apache.commons.io.FileUtils;
+import org.apache.lucene.search.Query;
+import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
+import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
+import org.apache.pinot.segment.spi.index.TextIndexConfig;
+import org.apache.pinot.segment.spi.index.reader.MultiColumnTextIndexReader;
+import org.apache.pinot.segment.spi.index.reader.QueryScopedCache;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotEquals;
+import static org.testng.Assert.assertNotSame;
+import static org.testng.Assert.assertSame;
+import static org.testng.Assert.assertTrue;
+
+
+/// Covers the query-scoped reuse of parsed queries in [LuceneTextIndexReader].
+public class LuceneTextIndexQueryCacheTest {
+ private static final File TEMP_DIR = new File(FileUtils.getTempDirectory(),
+ LuceneTextIndexQueryCacheTest.class.getSimpleName() + "-" +
UUID.randomUUID());
+ private static final File INDEX_DIR = new File(TEMP_DIR, "index");
+ private static final String COLUMN = "querycachebody";
+
+ private static final String[] DOCS = {
+ "failed to place order for user alice",
+ "failed to charge card for user bob",
+ "connection refused by payment service",
+ "order confirmation email sent to carol"
+ };
+
+ /// Stands in for `QueryContext`, which lives in pinot-core and is not
visible here. It uses the same
+ /// `computeIfAbsent` contract, and counts how many times a key is actually
computed so a test can
+ /// assert that a parse happened once rather than once per caller.
+ private static final class CountingCache implements QueryScopedCache {
+ private final ConcurrentHashMap<Object, Object> _values = new
ConcurrentHashMap<>();
+ private final AtomicInteger _computeCount = new AtomicInteger();
+
+ @SuppressWarnings("unchecked")
+ @Override
+ public <K, V> V getOrComputeSharedValue(Class<V> type, K key, Function<K,
V> mapper) {
+ return (V) _values.computeIfAbsent(key, k -> {
+ _computeCount.incrementAndGet();
+ return mapper.apply((K) k);
+ });
+ }
+
+ int computeCount() {
+ return _computeCount.get();
+ }
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws IOException {
+ FileUtils.forceMkdir(INDEX_DIR);
+ TextIndexConfig config = new TextIndexConfigBuilder().build();
+ try (LuceneTextIndexCreator creator = new LuceneTextIndexCreator(COLUMN,
INDEX_DIR, true, false, null, null,
+ config)) {
+ for (String doc : DOCS) {
+ creator.add(doc);
+ }
+ creator.seal();
+ }
+ }
+
+ @AfterClass
+ public void tearDown() {
+ FileUtils.deleteQuietly(TEMP_DIR);
+ }
+
+ private static TextIndexConfig configWithProperties(Map<String, String>
properties) {
+ return (TextIndexConfig) new
TextIndexConfigBuilder().withProperties(properties).build();
+ }
+
+ private static TextIndexConfig configWithStopWords(List<String>
stopWordsInclude) {
+ TextIndexConfig.AbstractBuilder builder = new TextIndexConfigBuilder();
+ builder.withStopWordsInclude(stopWordsInclude);
+ return (TextIndexConfig) builder.build();
+ }
+
+ private LuceneTextIndexReader reader(TextIndexConfig config) {
+ return new LuceneTextIndexReader(COLUMN, INDEX_DIR, DOCS.length, config);
+ }
+
+ @Test
+ public void testParseIsSharedWithinOneQuery()
+ throws Exception {
+ CountingCache cache = new CountingCache();
+ try (LuceneTextIndexReader first = reader(configWithProperties(Map.of()));
+ LuceneTextIndexReader second = reader(configWithProperties(Map.of())))
{
+ Query a = first.buildQuery("order", null, cache);
+ Query b = second.buildQuery("order", null, cache);
+ assertSame(b, a, "equivalent readers in one query should share the
parsed query");
+ assertEquals(cache.computeCount(), 1, "the query should have been parsed
exactly once");
+ assertNotSame(first.parseQuery("order", null), a, "parseQuery must
bypass the cache");
+ }
+ }
+
+ @Test
+ public void testNoCacheMeansNoSharing()
+ throws Exception {
+ try (LuceneTextIndexReader reader =
reader(configWithProperties(Map.of()))) {
+ Query a = reader.buildQuery("order", null, null);
+ Query b = reader.buildQuery("order", null, null);
+ assertNotSame(b, a);
+ assertEquals(b, a);
+ assertEquals(reader.getNumMatchingDocs("order", null, null), 2);
+ }
+ }
+
+ /// Regression test for a flattened cache key. `["failed", "order"]` and
`["failed, order"]` are
+ /// different stop-word sets -- the first makes `failed` a stop word, the
second does not -- yet both
+ /// render as `[failed, order]`. A key built from rendered text would let
the second configuration
+ /// reuse the first's empty query and silently lose matches.
+ @Test
+ public void testStopWordListsThatRenderIdenticallyDoNotCollide()
+ throws Exception {
+ CountingCache cache = new CountingCache();
+ try (LuceneTextIndexReader twoStopWords =
reader(configWithStopWords(List.of("failed", "order")));
+ LuceneTextIndexReader oneStopWord =
reader(configWithStopWords(List.of("failed, order")))) {
+ Query withFailedAsStopWord = twoStopWords.buildQuery("failed", null,
cache);
+ Query withFailedAsTerm = oneStopWord.buildQuery("failed", null, cache);
+ assertNotEquals(withFailedAsTerm, withFailedAsStopWord,
+ "configurations whose stop-word lists only render the same must not
share a cache entry");
+ assertEquals(cache.computeCount(), 2, "each configuration must be parsed
on its own");
+ assertEquals(withFailedAsStopWord, twoStopWords.parseQuery("failed",
null));
+ assertEquals(withFailedAsTerm, oneStopWord.parseQuery("failed", null));
+ }
+ }
+
+ @Test
+ public void testMultiTermConfigDoesNotCollide()
+ throws Exception {
+ CountingCache cache = new CountingCache();
+ try (LuceneTextIndexReader orReader = reader(
+
configWithProperties(Map.of(FieldConfig.TEXT_INDEX_USE_AND_FOR_MULTI_TERM_QUERIES,
"false")));
+ LuceneTextIndexReader andReader = reader(
+
configWithProperties(Map.of(FieldConfig.TEXT_INDEX_USE_AND_FOR_MULTI_TERM_QUERIES,
"true")))) {
+ assertNotEquals(andReader.buildQuery("failed order", null, cache),
+ orReader.buildQuery("failed order", null, cache));
+ assertEquals(orReader.getNumMatchingDocs("failed order", null, cache),
3);
+ assertEquals(andReader.getNumMatchingDocs("failed order", null, cache),
1);
+ }
+ }
+
+ @Test
+ public void testOptionsStringIsPartOfTheKey()
+ throws Exception {
+ CountingCache cache = new CountingCache();
+ try (LuceneTextIndexReader reader =
reader(configWithProperties(Map.of()))) {
+ assertNotEquals(reader.buildQuery("failed order",
"parser=MATCH,operator=AND", cache),
+ reader.buildQuery("failed order", null, cache));
+ assertEquals(cache.computeCount(), 2);
+ }
+ }
+
+ /// A cold query hit by every segment thread at once must still be parsed
once, not once per thread.
+ @Test
+ public void testConcurrentMissesParseOnce()
+ throws Exception {
+ int threads = 16;
+ CountingCache cache = new CountingCache();
+ ExecutorService executor = Executors.newFixedThreadPool(threads);
+ try {
+ // Opening a reader is far more expensive than the parse under test.
Releasing the start gate
+ // straight after submission would let an early worker reach the cache
while others were still
+ // in their constructor, so the test could pass without ever racing.
Gate on readiness instead,
+ // so every worker is one call away from the cache when the gate opens.
+ CountDownLatch ready = new CountDownLatch(threads);
+ CountDownLatch start = new CountDownLatch(1);
+ Future<Query>[] futures = new Future[threads];
+ for (int i = 0; i < threads; i++) {
+ futures[i] = executor.submit(() -> {
+ try (LuceneTextIndexReader reader =
reader(configWithProperties(Map.of()))) {
+ ready.countDown();
+ start.await();
+ return reader.buildQuery("connection", null, cache);
+ }
+ });
+ }
+ assertTrue(ready.await(60, TimeUnit.SECONDS), "all workers should open
their readers");
+ start.countDown();
+ Query first = futures[0].get(60, TimeUnit.SECONDS);
+ for (Future<Query> future : futures) {
+ assertSame(future.get(60, TimeUnit.SECONDS), first, "all threads
should observe one parsed query");
+ }
+ assertEquals(cache.computeCount(), 1, "a cold key must be parsed once,
not once per thread");
+ } finally {
+ executor.shutdownNow();
+ }
+ }
+
+ /// Guards overload resolution for [MultiColumnTextIndexReader].
+ ///
+ /// Cast null options to String to select the column-aware overloads instead
of the overloads
+ /// ending in a query-scoped cache.
+ @Test
+ public void testMultiColumnNullOptionsWithExplicitCast() {
+ MultiColumnTextIndexReader reader = multiColumnReaderReturning(new
MutableRoaringBitmap());
+ assertEquals(reader.getDocIds("body", "error", (String)
null).getCardinality(), 0);
+ assertEquals(reader.getNumMatchingDocs("body", "error", (String) null), 0);
+ }
+
+ private static MultiColumnTextIndexReader
multiColumnReaderReturning(MutableRoaringBitmap docIds) {
+ return new MultiColumnTextIndexReader() {
+ @Override
+ public MutableRoaringBitmap getDocIds(String column, String searchQuery,
String optionsString) {
+ return docIds;
+ }
+
+ @Override
+ public MutableRoaringBitmap getDocIds(String searchQuery) {
+ return docIds;
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getDictIds(String searchQuery) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void close() {
+ }
+ };
+ }
+}
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/QueryScopedCache.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/QueryScopedCache.java
new file mode 100644
index 00000000000..7419d70bcd6
--- /dev/null
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/QueryScopedCache.java
@@ -0,0 +1,47 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.reader;
+
+import java.util.function.Function;
+
+
+/// A cache whose lifetime is a single query, letting the segments of one
request share a value that
+/// is expensive to derive and identical for all of them.
+///
+/// Index readers live longer than a query and are shared by every query, so
they cannot hold
+/// per-query state themselves; the caller owns the cache and passes it in.
+///
+/// This interface exists so that segment-level code can use a query-scoped
cache without depending
+/// on the query engine. Implementations must be safe for concurrent use,
because the segments of
+/// one request are processed in parallel.
+public interface QueryScopedCache {
+
+ /// Returns the value for `key`, computing it with `mapper` if it is not
already present.
+ ///
+ /// Computation is atomic per `(type, key)`: concurrent callers that miss
the same key block until
+ /// one `mapper` completes, and all of them observe that single result. A
`mapper` that throws or
+ /// returns `null` leaves nothing cached, so a later call for the same key
runs it again. These are
+ /// the semantics of
[java.util.concurrent.ConcurrentHashMap#computeIfAbsent]; implementations
+ /// backed by it satisfy this contract.
+ ///
+ /// @param type the type of the value, which keeps unrelated callers from
colliding on equal keys
+ /// @param key the key identifying the value; must be immutable with
structural equality
+ /// @param mapper computes the value the first time the key is seen
+ <K, V> V getOrComputeSharedValue(Class<V> type, K key, Function<K, V>
mapper);
+}
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/TextIndexReader.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/TextIndexReader.java
index 79f33ac6797..3230ff4b008 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/TextIndexReader.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/TextIndexReader.java
@@ -43,6 +43,19 @@ public interface TextIndexReader extends IndexReader {
return getDocIds(searchQuery);
}
+ /// Returns the matching document ids, reusing a query-scoped parse when the
implementation supports it.
+ ///
+ /// Parsing a search string depends only on the string and the column's
index configuration, so every
+ /// segment of one request would otherwise repeat identical work.
Implementations that parse should
+ /// derive their cache key from the search string, the options and the
configuration that affects
+ /// parsing. A `null` cache means "do not share", and must behave exactly
like the two-argument form.
+ ///
+ /// Only the *parsed query* is shared. Doc ids are per-segment and must
never be cached across readers.
+ default MutableRoaringBitmap getDocIds(String searchQuery, @Nullable String
optionsString,
+ @Nullable QueryScopedCache queryScopedCache) {
+ return getDocIds(searchQuery, optionsString);
+ }
+
/// Returns the number of documents matching the given search query, without
materializing the matching
/// document ids.
///
@@ -61,6 +74,12 @@ public interface TextIndexReader extends IndexReader {
return getDocIds(searchQuery, optionsString).getCardinality();
}
+ /// Counting counterpart of [#getDocIds(String, String, QueryScopedCache)].
+ default int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString,
+ @Nullable QueryScopedCache queryScopedCache) {
+ return getNumMatchingDocs(searchQuery, optionsString);
+ }
+
/// Marker method that allows to differentiate between single-column and
multi-column text index reader .
default boolean isMultiColumn() {
return false;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
