Repository navigation
[improvement](be) Speed up dense unscored inverted index conjunctions - #69172
Conversation
…d index conjunction ### What problem does this PR solve? Issue Number: None Related PR: apache#68758 Problem Summary: An unscored AND of terms on one field, such as MATCH_ALL, lists the rarest term's rows into an array and narrows it by the other terms. A term held by every row cannot remove any row, but the chain still reads its postings. For `MATCH_ALL 'http status'`, where both terms are held by every row, it copies every row id twice and rebuilds the result bitmap from the array. On a 2M-row V2 segment the index filter took 14.4 ms, about 10 times what it took before apache#68758. A term whose document frequency equals the source's document count is held by every row the source lists, so it cannot narrow the conjunction. The chain now skips such terms, and a conjunction whose terms are all held by every row returns every row, or every candidate, without reading postings. Scored queries and conjunctions without such a term run as before. Index filter time of MATCH_ALL on a 2M-row V2 segment, as the median ratio of 8 paired runs on fresh backends; each column is a separate comparison and lower is faster: | Query | Rows holding each term | vs apache#68758 | vs before apache#68758 | |---------------|------------------------|--------------------------|--------------------------| | `http status` | 100%, 100% | 0.01 (14.43 -> 0.15 ms) | 0.11 (1.53 -> 0.16 ms) | | `rare http` | 0.05%, 100% | 0.34 (0.62 -> 0.21 ms) | 0.11 (2.24 -> 0.25 ms) | | `sparse http` | 0.5%, 100% | 0.24 (1.18 -> 0.29 ms) | 0.31 (1.04 -> 0.34 ms) | | `get http` | 90%, 100% | 0.88 (14.15 -> 12.20 ms) | 1.22 (10.25 -> 12.83 ms) | | `alpha beta` | 50%, 50% | 1.02 | 1.36 | The end-to-end time of `MATCH_ALL 'http status'` falls to 0.64 of apache#68758, in line with that before apache#68758 (0.96). On SNII segments, which take the same path, the `http status` filter drops from 13.67 ms to 0.37 ms, and no dense query is slower. Two-term MATCH_ALL queries on short text and the 18 queries used to accept apache#68758 (MATCH_PHRASE, MATCH_ANY and SEARCH phrases on V2 and SNII) stay between 0.97 and 1.03 of apache#68758, and every 95% interval includes 1. Dense conjunctions without a term held by every row, such as `alpha beta`, are not changed by this PR. ### Release note None ### Check List (For Author) - Test: Unit Test. ListedTermsTest.AConjunctionSkipsATermHeldByEveryRow and ListedTermsTest.AConjunctionOfTermsHeldByEveryRowHoldsEveryRow fail before the change, because the chain read the term held by every row, and pass after it. Manual test: the benchmark above; every result matched aggregates computed independently from the corpus. - Behavior changed: No. Results are unchanged; only fewer postings are read. - Does this need documentation: No
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed all four changed files at 7cf31319d52edfd67b781f4610680e5fe17f8809. No new actionable findings and no existing inline P0/P1 findings to carry forward. The review converged in round 1: both complete reviewers and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS; every candidate and initial risk was independently verified and dismissed with evidence.
The production path inspected was MATCH_ALL/SEARCH unscored AND → same-source ListedTerms → cursor open → universal-term pruning or remaining cursor chain → TruthSet intersection → result bitmap. CLucene uses maxDoc and one document per Doris row, including empty documents for NULLs; SNII uses total document count and validates unique bounded posting IDs. Doris deletion filtering remains outside these posting lists, and final scan consumers intersect results, so synthesizing the full source domain does not restore excluded scan rows. Candidate IDs are sorted and bounded by the caller/source contract.
Critical checkpoint conclusions:
- Goal and tests: the change removes redundant universal operands from unscored AND. The two added unit tests assert the expected partial/all-universal rows and absence of universal-term prefetch, including a supplied candidate subset. These assertions were inspected, not executed.
- Scope and clarity: the four-file change is focused; both cursor overloads reuse one implementation.
- Concurrency and locking: query-local synchronous execution; no new shared mutation, threads, locks, or lock order.
- Lifecycle and initialization: owning cursors outlive the raw-pointer views; reserved wrapper storage keeps addresses stable; SNII destructors cancel pending callbacks. No new static initialization or ownership cycle.
- Configuration: no configuration or dynamic propagation change.
- Compatibility and rolling upgrades: the owning overload remains; no storage-format, RPC, or external ABI removal. No new libc dependency is apparent from the changed code; no release ELF was built or audited.
- Thrift/protobuf: no generated-definition change or cross-branch ID obligation.
- Parallel paths: both CLucene and SNII reach the shared unscored AND path. Scored AND retains universal terms' score contributions; disjunction, phrase, gram, and nested NOT behavior retain their required paths.
- Special conditions: the skip invariant is documented; absent terms return before pruning. Sorted candidate clipping, zero-row sources, empty candidates, duplicate terms, scalar/array NULLs, and parent TRUE/UNKNOWN intersections were traced without finding a new result difference.
- Coverage: new tests exercise partial pruning for both source batch settings and all-universal full/subset results. Existing NULL, absent, scored, candidate, and multi-wave tests were inspected. The new tests use fakes rather than real-format I/O; production universality was checked separately through writer/reader/factory/compaction control flow.
- Test results: expected fake intersections and the every-row helper are correct by inspection. No golden/result files changed; author-reported executions and performance numbers were not independently reproduced.
- Observability: existing search initialization/execution and filter timers enclose the optimization; no new operational state needs separate logging or metrics.
- Transactions and persistence: no EditLog, commit/failover, visible-version selection, or persistent metadata mutation changes.
- Writes and atomicity: no data-write protocol or write-side lifecycle changes. Delete-bitmap subtraction and monotonic scan-result intersection remain intact.
- FE/BE variables: none added; no transport send path needs updating.
- Performance, memory, and errors: all-universal conjunctions avoid document-array materialization; partial conjunctions trade O(term count) observer-vector work for skipped posting enumeration. Ownership and Status propagation remain sound. SNII can still fetch preludes already queued with other cursors, which limits I/O savings without changing correctness. The pure-text build-hygiene gate passed.
- Other invariants and focus: no additional rowset/segment-identity, visibility, NULL algebra, or silent-error regression was substantiated. No additional user-provided review focus was supplied.
Validation limits: static source review and a pure-text hygiene check only. No build, unit/regression test, sanitizer, runtime ABI audit, or benchmark was run. Third-party CLucene internals were not available in this checkout; its production construction and compaction boundaries were reviewed. No unresolved behavioral candidate or uncovered changed-file scope remains.
Review converged after 1 round(s).
TPC-H: Total hot run time: 29773 ms |
TPC-H: Total hot run time: 29675 ms |
TPC-DS: Total hot run time: 151553 ms |
ClickBench: Total hot run time: 24.56 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
…date and list a lone term straight into the result ### What problem does this PR solve? Issue Number: None Related PR: apache#68758 Problem Summary: After terms held by every row are skipped, two dense MATCH_ALL cases on V2 stay slower than before apache#68758. For `get http`, the term left alone is listed into an array of every row it holds before the result bitmap is built from it, which costs about 1,200 extra page faults per query on 2M rows. For `alpha beta`, two unrelated terms each held by half the rows, the bit-set intersection tests each candidate with a branch that mispredicts about half the time; it takes 30% of the scan thread's cycles. A lone term now lists its blocks straight into the result, a dense block as a range. The bit-set intersection writes every candidate and keeps it only when its bit is set, so no branch depends on whether a candidate matches. On a 2M-row V2 segment the index filter of `get http` drops to 0.74 and that of `alpha beta` to 0.73 of the previous commit, 0.98 and 0.92 of their time before apache#68758; on SNII they drop to 0.79 and 0.70. ### Release note None ### Check List (For Author) - Test: Unit Test. ListedTermsTest.ALoneTermListsItsRowsOrThoseAmongTheCandidates and CursorChainedPostings.IntersectionHandlesRandomDensities; each fails when the code it covers is broken on purpose. Manual test: the dense MATCH_ALL benchmark of the previous commit, and the 18 queries used to accept apache#68758, which stay within 0.97 and 1.02 of the previous commit. - Behavior changed: No. - Does this need documentation: No
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed all five changed files at 58005f7 and the relevant callers, providers, writers, compaction and result consumers. No new actionable findings and no existing inline blockers to carry forward. The review converged in round 1: both complete-coverage reviewers and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS after their missed-risk rechecks; all candidates and initial risks were independently verified and dismissed.
Critical checkpoint conclusions:
- Goal and correctness: the shortcut removes universal terms only from unscored AND. CLucene uses maxDoc; SNII counts all physical rows, including NULL/empty/array rows, and validates unique bounded postings. These support the universal-term predicate and synthetic range. Doris delete-bitmap restrictions remain applied to scan rows. Added unit cases check matching rows and skipped prefetches; they were inspected, not executed.
- Scope and reuse: focused changes in three production files and two existing unit-test files. Both chain overloads share one implementation; list_blocks reuses the existing block-to-sink traversal.
- Concurrency and locking: no new threads, locks or shared mutable state. Cursor mutation and results stay within synchronous query execution; no new lock ordering issue found.
- Lifecycle and static initialization: narrowing pointers borrow live owning cursors; reserved chain storage keeps references stable. No escaping pointers, ownership cycles or new static initialization found.
- Configuration: none added or changed; dynamic-update behavior is not applicable.
- Compatibility: no wire, storage-format, dependency or rolling-upgrade changes. Existing owning-cursor API remains available alongside the internal pointer overload; no new libc API usage identified.
- Thrift/protobuf: untouched.
- Parallel paths: V2 and SNII use the common execution path. Scored conjunction retains all term contributions; OR/NOT and phrase positional semantics remain preserved. Shared phrase callers were traced.
- Special conditions: comments explain universal-term elimination, sole-term direct listing and branchless writes. Candidate vectors are sorted by their Roaring producer. Bounded block slices and resize(prefix + candidate_count) cover every unconditional store; callers keep input and output buffers separate.
- Test coverage: new cases cover mixed/all-universal terms and lone narrowing terms, with and without candidates and both batching modes where applicable. The random test checks both ordinals and document output; existing tests cover bounded/wide blocks, shifted IDs, dense ranges, NULLs, absent terms, mixed clauses and scoring. No new real-format end-to-end cases were added. Explicit empty/out-of-range universal candidates, universal scored/nullable integration, injected helper errors and a multi-block bitset append-prefix case remain test opportunities; source tracing found no unresolved defect in them.
- Test results: added expected row sets agree with the fixtures and independent intersection logic. No regression golden files were changed.
- Observability: existing index-search/filter timers and error propagation cover this local optimization; no additional operation or failure class requires new instrumentation.
- Transactions and persistence: no transaction, version-visibility, EditLog/replay or failover logic changes. Physical-row and compaction-count invariants were traced.
- Data writes and crash atomicity: read-only evaluation; no new write or recovery lifecycle.
- FE/BE variables: none added; no propagation changes required.
- Performance, memory and errors: universal terms avoid postings decoding; the lone-term sink avoids an intermediate row vector. Branchless scratch is bounded by the candidate slice and preserves the existing prefix. New Status calls propagate errors; no new ignored failure found. Benchmark claims were not independently measured.
- Other concerns: rechecked absent/zero-row cases, repeated tokens, NULL/empty/array fields, deletion/compaction, dense blocks, ordinal consumers and borrowed-pointer lifetime; no additional substantiated issue found.
No additional user focus was supplied. Limits: static review only; no builds or tests run and no source modifications. CLucene submodule internals are absent locally and were left uninitialized; its Doris wrappers, writer/compaction interfaces and existing tests were inspected. Runtime behavior and author benchmark results remain unverified. No changed file or correctness candidate remains unresolved within the required static scope.
Review converged after 1 round(s).
TPC-H: Total hot run time: 29485 ms |
TPC-DS: Total hot run time: 152363 ms |
ClickBench: Total hot run time: 24.59 s |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…ads on demand ### What problem does this PR solve? Issue Number: None Related PR: apache#68758 Problem Summary: An unscored conjunction of several terms lists every row of its cheapest term into an array before the other terms narrow it into another, so a dense conjunction holds arrays of up to 8 bytes per row of the segment in each scanner (review comment M2 on apache#68758). For a source that reads on demand, such as a V2 segment, the conjunction now streams: the cheapest term lists at most 2^16 of its rows at a time, the other terms narrow them, and the rows left go straight into the result. The other terms keep their current block from one batch to the next, so each block is still decoded once, and no array holds more than a batch; a cheapest term of at most 2^16 rows is listed in one batch, as the chain lists it. A batching source, such as SNII, keeps the chain, which reads each term's blocks in one round. On a 2M-row V2 segment the index filter of `alpha beta` drops to 0.76 of the previous commit, that of `get alpha` to 0.84 and that of `alpha beta get` to 0.89, and their page faults per query from 458 to 192, 484 to 47 and 447 to 37. Conjunctions whose cheapest term holds fewer rows, such as `rare alpha`, `sparse alpha` and `rare sparse`, stay between 0.99 and 1.02, and the scan threads run within 1% of the previous commit's instructions. ### Release note None ### Check List (For Author) - Test: Unit Test. CursorChainedPostings.AStreamHandsOnABatchOfRowsAtATime, AStreamReadsOnlyTheBlocksTheCheapestRowsReach, AStreamEndsWhenACursorEnds and AStreamListsTheRowsTheChainLists. ListedTermsTest.AConjunctionOnAStreamingSourceStreamsItsTerms replaces AConjunctionOnAStreamingSourceChainsItsTermsToo, which recorded that such a source chained its terms; it fails before this change. Each stream test fails when the code it covers is broken on purpose. Manual test: the dense MATCH_ALL benchmark with seven more shapes, and the 18 queries used to accept apache#68758, which stay between 0.98 and 1.04 of the previous commit. - Behavior changed: No. - Does this need documentation: No
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed all five changed files at 42beacb, including the MATCH_ALL/boolean invocation chain, CLucene and SNII adapters, shared scored/phrase/gram callers, and changed tests. No substantiated new finding or existing blocking inline finding remains. Review converged in 1 round: both complete-review agents and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS, and the main review independently verified their conclusions.
Critical checkpoint conclusions:
- Goal: the code implements universal-term skipping, direct lone-term bitmap emission, branchless block intersection, and bounded streaming for on-demand unscored conjunctions. Unit cases exercise each behavior; performance claims were not reproduced.
- Scope and reuse: the five-file change is focused and reuses the existing intersection, chain and document-sink mechanisms.
- Concurrency: no new threads, locks, atomics or shared mutable query state; groups and cursor operations remain synchronous and local to a query.
- Lifecycle and initialization: owning cursors outlive raw observers; lead rows are copied before block advancement, and followers retain cursor-owned blocks only until that cursor advances. Skipped SNII cursors unregister pending callbacks. New static data is constexpr, with no cross-translation-unit initialization dependency.
- Configuration: none added or changed.
- Compatibility: no persisted format, protocol, dependency or platform API change; the original owning chain_cursors overload is preserved. Release ELF compatibility was not tested.
- Thrift/protobuf: no definitions changed, so release-branch field-ID comparisons are not applicable.
- Parallel paths: candidate-restricted and batching conjunctions retain chaining; scored conjunction/disjunction and phrase ordinal selection preserve their behavior. Both real sources use the physical row domain for doc_count. NULL documents and scan deletion filtering preserve the universal-term shortcut's semantics.
- Conditions: dispatch, block-boundary and EOF branches reflect concrete algorithm states and include reasoning comments; no new silent error suppression was found.
- Test coverage: inspected universal/lone-term and candidate cases, both source modes, dense/sparse intersections, cross-batch follower reuse, selective seeks, early EOF and randomized intersections. The random stream oracle independently computes set intersections; the density test checks document output as well as ordinals. Existing NULL, scored and phrase paths were also inspected. No new SQL regression or injected-error case was added.
- Test results: no golden-result files changed; expected unit rows, batches and decoded-block counts agree with static control flow. No test execution is claimed.
- Observability and errors: new prefetch/read/seek/sink Status values are propagated through existing exception-conversion boundaries and search timers. No additional operational component requiring metrics was introduced.
- Transactions and persistence: no logging, replay, transaction, failover or version-selection logic changed.
- Writes and atomicity: no write path changed. Existing scan restrictions subtract deletion bitmaps, and index results only narrow the surviving row bitmap.
- FE-BE propagation: no new variable or serialized metadata.
- Performance and memory: redundant universal reads and lone-term row materialization are removed; streamed scratch vectors each hold at most 65,536 logical rows, with possible geometric capacity slack. Branchless writes stay within the resized output and production callers use distinct input/output buffers. Full allocator accounting and timing were not runtime-verified.
- Other checks: no additional correctness, bounds, ownership, include-hygiene or shared-caller regression was substantiated. No additional user focus was supplied.
Validation was static only, as required: no builds, tests, benchmarks or source modifications. The new algorithm tests use fake/blocked cursors; production block ownership, row-domain, seeking and deletion behavior were checked against current Doris code and the exact pinned CLucene dependency. No unresolved review candidate or uncovered changed path remains. There were no pre-existing inline threads to duplicate.
Review converged after 1 round(s).
TPC-H: Total hot run time: 29253 ms |
TPC-DS: Total hot run time: 151688 ms |
ClickBench: Total hot run time: 24.57 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…conjunctions (#69172) (#69239) ### What problem does this PR solve? Issue Number: N/A Related PR: #69172, #69206 Problem Summary: Backport #69172 after #69206 introduced the shared inverted-index query engine to branch-4.2. Dense unscored conjunctions now skip terms held by every row, write a lone term directly to the result bitmap, use branch-free block intersection, and bound streaming-source batches. Query results are unchanged. Source PR: #69172 Source squash commit: `f937d950d45313f20864d287df453e1b90f5c615` (cherry-picked with `-x`). ### Release note None ### Check List (For Author) - Test - [x] Regression test: `test_match_format_parity` passed on an isolated branch-4.2 FE/BE cluster after rebasing onto `ecd322ad31193cda19cf4f3b08e0f3f9d12dd67c` (1 suite, 0 failures). - [x] Unit Test: `ListedTermsTest.*:CursorChainedPostings.*` passed (41 tests in 2 suites); all 9 source-added tests were listed and executed. No sanitizer report. - [x] Manual test: `./build.sh --be -j64` and `./build.sh --fe -j8` passed; isolated backend reported `Alive=true`, and `SELECT 1` succeeded. - Behavior changed: - [x] No. Unscored conjunction execution is faster; query results are unchanged. - Does this need documentation? - [x] No. ### Validation - `build-support/check-format.sh`: PASS. - Complete merged-tree format preflight against `origin/branch-4.2`: PASS. - Complete PR diff English description check: PASS (5 source files, no exceptions). - Branch-4.2 has no `build-support/run-clang-tidy.sh`; the branch's BE build and format gates passed. ### Hunk audit The source squash patch ID matches the final source PR head. The cherry-pick was clean, and every changed file has a byte-identical postimage to the source commit. | Source file | Disposition | | --- | --- | | `be/src/storage/index/inverted/query/engine/boolean_query/listed_terms.cpp` | Ported (verbatim) | | `be/src/storage/index/inverted/query/exec/cursor_chained_postings.cpp` | Ported (verbatim) | | `be/src/storage/index/inverted/query/exec/cursor_chained_postings.h` | Ported (verbatim) | | `be/test/storage/index/inverted/query/engine/boolean_query/listed_terms_test.cpp` | Ported (verbatim) | | `be/test/storage/index/inverted/query/exec/cursor_chained_postings_test.cpp` | Ported (verbatim) | `git range-diff` shows only the `-x` cherry-pick provenance line; the code patch is unchanged.
What problem does this PR solve?
Issue Number: N/A
Related PR: #68758
Problem Summary:
#68758 runs an unscored AND of terms on one field, such as MATCH_ALL, as a chain: the rarest term's rows are listed into an array and the other terms narrow them into another. On dense terms this was slower than the bitmap intersection it replaced on V2 segments, and its arrays grew with the segment (review comment M2 on #68758):
MATCH_ALL 'http status', where both terms are held by every row, it copied every row id twice and rebuilt the result bitmap from the array: 14.4 ms on a 2M-row V2 segment, about 10 times what it took before [refactor](be) Unify MATCH and SEARCH execution across inverted index formats #68758.getinget httponcehttpis skipped, was listed into an array of all its rows before the result bitmap was built from it. For a term in 90% of 2M rows that cost about 1,200 extra page faults per query.alpha beta), the branch mispredicted about half the time, and the intersection took 30% of the scan thread's cycles.This PR:
Scored queries keep their path, and results are unchanged.
Index filter time of MATCH_ALL on a 2M-row V2 segment, as the median ratio of 8 paired runs on fresh backends; each column is a separate comparison and lower is faster:
http statusrare httpsparse httpget httpalpha betaIn the comparison with #68758, the
http statusfilter drops from 16.9 ms to 0.15 ms and its end-to-end time to 0.63, and the end-to-end time ofalpha betato 0.81. SNII segments take the same paths except the stream; there these queries take 0.02 to 0.83 of #68758 and 0.02 to 0.93 of their time before #68758. Seven more shapes, fromrare alphaandsparse alphatoalpha beta getand the emptyget post, take 0.20 to 0.89 of their time before #68758 on V2 and 0.57 to 1.00 on SNII. Two-term MATCH_ALL queries on short text take 0.86 to 1.01 of #68758, and the 18 queries used to accept #68758 (MATCH_PHRASE, MATCH_ANY and SEARCH phrases on V2 and SNII) stay between 0.97 and 1.03 of it, every 95% interval including 1.On V2 the stream holds at most two arrays of 2^16 rows per scanner, and it cuts the page faults per query of
alpha betafrom 458 to 192 and those ofget alphafrom 484 to 47. Its scan threads run within 1% of the chain's instructions, and a cheapest term of at most 2^16 rows is listed in one batch, as the chain lists it.Release note
None
Check List (For Author)
Test
Unit test:
ListedTermsTest.AConjunctionSkipsATermHeldByEveryRowandListedTermsTest.AConjunctionOfTermsHeldByEveryRowHoldsEveryRowfail before the first commit, because the chain read the term held by every row, and pass after it.ListedTermsTest.AConjunctionOnAStreamingSourceStreamsItsTermsreplacesAConjunctionOnAStreamingSourceChainsItsTermsToo, which recorded that such a source chained its terms, and fails before the last commit.ListedTermsTest.ALoneTermListsItsRowsOrThoseAmongTheCandidates,CursorChainedPostings.IntersectionHandlesRandomDensitiesand theCursorChainedPostings.AStream*tests cover the lone-term listing, the intersection and the stream; each fails when the code it covers is broken on purpose.Manual test: a 2M-row corpus with terms held by 100%, 90%, 50%, 10%, 0.5% and 0.05% of the rows was loaded into a V2 table and an SNII table. Each of 8 pairs started two fresh backends, one per build, swapping which backend ran which build on alternate pairs; each query ran 4 warm-up and 16 measured times on each build, alternating between the builds. Every result matched aggregates computed independently from the corpus. Scan-thread CPU time, cycles, instructions and page faults were sampled with
perf statover 200 runs of each query on each build.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)