Bound FST regexp traversal and fall back to scan when the limit is exceeded#18759
Bound FST regexp traversal and fall back to scan when the limit is exceeded#18759xiangfu0 wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #18759 +/- ##
=========================================
Coverage 64.76% 64.77%
Complexity 1309 1309
=========================================
Files 3380 3380
Lines 209573 209587 +14
Branches 32805 32809 +4
=========================================
+ Hits 135735 135755 +20
+ Misses 62914 62904 -10
- Partials 10924 10928 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
4ad85c5 to
044bddd
Compare
CI fix
How the default (100,000) was chosenAdded
Takeaways that justify 100,000:
Caveat: the fallback's latency depends on the scan regex engine. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a bounded FST/IFST-backed REGEXP_LIKE traversal with a configurable path budget, falling back to dictionary scan when the traversal is too broad to be memory-efficient.
Changes:
- Introduces query option
fstRegexpTraversalLimit(default100_000) and plumbing to read it. - Extends
TextIndexReaderwith a boundedgetDictIds(String, int)overload and wires the traversal limit through FST/IFST regexp evaluators. - Updates regexp matchers/readers/tests and adds perf experiments/benchmarks for limit selection and validation.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java | Adds new query option key/value default for traversal limit. |
| pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/TextIndexReader.java | Adds bounded getDictIds overload with default implementation. |
| pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/fst/IFSTBuilderTest.java | Adds termination + traversal-limit tests for IFST matcher/reader. |
| pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/fst/FSTBuilderTest.java | Adds termination + traversal-limit tests for FST matcher/reader. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/RegexpMatcherCaseInsensitive.java | Refactors matcher to stream results + enforce traversal cap. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/RegexpMatcher.java | Refactors matcher to stream results + enforce traversal cap. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/FSTTraversalLimitExceededException.java | Introduces unchecked exception used as a “fall back to scan” signal. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/LuceneIFSTIndexReader.java | Uses bounded matcher API and propagates traversal-limit/termination exceptions. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/LuceneFSTIndexReader.java | Uses bounded matcher API and propagates traversal-limit/termination exceptions. |
| pinot-perf/src/main/java/org/apache/pinot/perf/FstTraversalLimitExperiment.java | Adds exploratory experiment for choosing a sensible default limit. |
| pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkFSTRegexpMatcher.java | Adds JMH benchmark harness for FST/IFST regexp matching. |
| pinot-core/src/test/java/org/apache/pinot/queries/FSTBasedRegexpLikeQueriesTest.java | Adds end-to-end test validating scan fallback correctness. |
| pinot-core/src/test/java/org/apache/pinot/core/plan/FilterPlanNodeTest.java | Updates mock to stub bounded getDictIds overload. |
| pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/PredicateEvaluatorProvider.java | Reads traversal limit option and falls back to scan on limit exception. |
| pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/IFSTBasedRegexpPredicateEvaluatorFactory.java | Passes traversal limit into bounded getDictIds. |
| pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/FSTBasedRegexpPredicateEvaluatorFactory.java | Passes traversal limit into bounded getDictIds. |
| pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java | Adds query option accessor for fstRegexpTraversalLimit. |
52dc12d to
f5357e4
Compare
33cf491 to
0ffb895
Compare
A REGEXP_LIKE backed by an FST/IFST index walks the query automaton over the FST. A broad pattern (anything with a leading `.*`) over a high-cardinality column gives the prefix walk no pruning, so it visits a large fraction of the FST and allocates proportionally -- a per-query heap spike that, under concurrency, drives server OOMs. The live frontier is already bounded (DFS), but the allocation throughput is not. Bound it: the matchers now accept a `maxPaths` cap and throw FSTTraversalLimitExceededException once that many paths are visited. The FST/IFST readers expose a `getDictIds(query, maxTraversalPaths)` overload (new default method on TextIndexReader) and let the signal propagate unwrapped. PredicateEvaluatorProvider passes the configured limit and, on breach, falls back to the existing dictionary-scan evaluator -- which is correct and, for broad patterns, typically far cheaper in memory (benchmarks show scan allocates ~190x less than FST for `.*`). Selective prefix/fuzzy queries visit a small subtree and never trip the cap, so the index keeps its large win there. The cap is configurable via the `fstRegexpTraversalLimit` query option, defaulting to 100,000 visited paths. Tests: matcher-level limit (broad walk throws, generous limit completes, selective query stays under a modest budget) for both FST and IFST; and an end-to-end query test asserting that a tiny limit forces the scan fallback and returns results identical to the FST path.
0ffb895 to
81e2a62
Compare
|
close as not needed |
Problem
A
REGEXP_LIKEbacked by an FST/IFST index walks the query automaton over the FST. A pattern that does not prune — anything with a leading.*— gives the prefix walk nothing to descend into, so it traverses essentially the whole FST (~cardinality paths) and allocates proportionally. A single such query is fine, but a burst of them saturates server heap (the EpicGames OOM). The companion fix #18754 (merged) removed the retained-result overhead and made the walk interruptible; this PR bounds the walk itself.Benchmarks (default
java.util.regexscan, 500K keys × 40 chars): for broad patterns the dictionary scan is faster and allocates ~190× less than the FST walk, while for selective prefix/fuzzy patterns the FST is ~1600× faster. So the right move is to cap the FST work and fall back to scan only when the walk is actually broad.Approach
The matcher reports completion instead of throwing — no exception is used as control flow:
RegexpMatcher/RegexpMatcherCaseInsensitivereturnfalseoncemaxPathsFST paths have been visited (walk abandoned), ortrueif it completed.TextIndexReadergains a backward-compatible@Nullable getDictIds(String, int maxTraversalPaths)default method (delegates to the single-arg version, so other readers are unaffected). The two Lucene readers returnnullwhen the walk is abandoned.nullonnull, andPredicateEvaluatorProviderfalls back to the existing dictionary-scan evaluator — correct, and typically far cheaper in memory for broad patterns. Selective patterns visit a small subtree, never trip the cap, and keep the FST's large speed advantage.Default cap = column cardinality
The
fstRegexpTraversalLimitquery option overrides the cap with an absolute value (non-positive disables it); when unset it defaults to the column cardinality (dictionary length). This is the natural, self-tuning unit: an experiment across cardinalities shows a selective query visits paths proportional to its match count (≈150 paths for 100 matches, independent of cardinality), while a non-pruning.*walk visits ≈1.1× the cardinality. So capping at cardinality makes a non-pruning pattern fall back to scan (where scan is the better tool anyway) while every selective prefix/fuzzy query stays on the FST — at any column size, with no magic constant..*(matchAll).*<rare-suffix>(leading.*)No on-disk format change; the SPI addition is a default method (rolling-upgrade safe); the new query option is additive.
Testing
FSTBuilderTest,IFSTBuilderTest): a tiny budget abandons the walk (matcher returnsfalse, reader returnsnull); an unbounded walk completes; a cap equal to the cardinality makes a.*walk fall back (null) while a selective prefix stays on the FST; a non-positive cap disables the bound.FSTBasedRegexpLikeQueriesTest): a query withfstRegexpTraversalLimit = 1forces the scan fallback and returns results identical to the FST path (256 and 512 rows), proving fallback correctness.