Repository navigation
branch-4.2: [fix](inverted index) Share BM25 statistics across parallel scanners of a tablet #69352 - #69440
Merged
yiguolei merged 1 commit intoOct 23, 2026
Conversation
…el scanners of a tablet apache#69352 A parallel scan splits one tablet into several scanners, and each scanner collected BM25 statistics (doc count, avgdl, term doc freqs) over only the rowsets in its own split. Rows with identical text therefore got different scores depending on which scanner read them, and because the split depends on the tablet's row count and the core count, a row's score and rank moved whenever unrelated data was loaded. Collect the statistics once per tablet over its entire read source and copy them into each scanner, so every row of a tablet is scored against the same idf and avgdl. Each scanner keeps its own copy, so the per-scanner idf/avgdl caches are not shared between threads.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Oct 23, 2026
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
|
PR approved by at least one committer and no changes requested. |
5 of 9 tasks
yiguolei
pushed a commit
that referenced
this pull request
Oct 29, 2026
…tistics of merge-on-write tables #69438 (#69752) ### What problem does this PR solve? Issue Number: None Related PR: #69438 (this is its backport to branch-4.2), #69352 and its branch-4.2 pick #69440 (BM25 statistics shared across the parallel scanners of a tablet) Problem Summary: Backport of #69438 to branch-4.2. On a merge-on-write table, BM25 `score()` gives the same row a different score on each replica of its tablet. A query picks one replica per tablet, so with `replication_num = 3` a row has up to three scores, and pagination over `ORDER BY score()` repeats and skips rows from one page to the next. `CollectionStatistics::process_segment()` takes the document count, the token count and each term's document frequency straight from every segment's inverted index. Rows a later load overwrote or deleted (marked only in the delete bitmap) and rows written with the delete sign (CDC deletes, `DELETE`) stay in the index and are counted until compaction removes them, and each replica compacts on its own schedule. On branch-4.2 (3 BEs, `replication_num = 3`, 8 buckets, CDC-style upserts and delete-sign deletes, the replicas on one BE fully compacted), all 1,731 matching rows of a `SEARCH(... OR ...)` query got more than one score over 15 runs and differed between `use_fix_replica = 0/1/2`. Fix: the same change as #69438. When the scan reads a delete bitmap (merge-on-write), `CollectionStatistics` describes only the rows the scan returns: - `OlapScanner` passes the scan's delete bitmap and read version (`MowHiddenRows`) to `collect()`, or to `TabletCollectionStatistics::copy_to()` for a split tablet (#69440). - Per segment, the hidden rows are the rows the delete bitmap marks at the read version plus the rows whose `__DORIS_DELETE_SIGN__` is set. The delete-sign column is read only for segments whose zone map shows a non-zero sign (or cannot tell), following the query's `enable_segment_cache` and `enable_page_cache`. - Document count: the index's document count minus the hidden rows. Term document frequency: `IndexSource::scoring_doc_freq_among()` subtracts the hidden rows a term's postings list, leaping between them when they are much rarer than the postings, otherwise walking them alongside the decoded postings. - Token count: the sum of the document lengths the norms of the returned non-NULL rows stand for (NULL rows left out through the index's null bitmap), or the returned row count for an index without norms. Every replica counts the same, compacted or not. - CLucene norms are read one segment at a time into a buffer of the statistics' own and not kept; the non-NULL norm length sum of a field and the length sums of its last four sets of hidden rows are kept with the cached searcher (`CluceneSearcherCacheValue`). - A tablet whose rows are all hidden reports zero documents instead of failing the query. Tables without a delete bitmap collect the same statistics as before. For every `score()` query, the statistics' I/O context follows `disable_file_cache` and `inverted_index_snii_read_no_write_file_cache`, and a CLucene searcher the statistics open drops the query's I/O context once its meta is read; a CLucene error while opening it fails the query. Differences from #69438, for branch-4.2: segments are identified by their id instead of `RowsetSegmentView`, and the delete-sign column reader comes from `Segment::get_column_reader()` (branch-4.2 has no `get_column_reader_for_pruning`), a segment without the column reading as no deletes. The tests are adapted the same way. Not covered (as in #69438): merge-on-read unique tables and duplicate tables with `DELETE` predicates; rows deleted by a delete predicate (`enable_mow_light_delete`); rows written with the delete sign that the query returns (`skip_delete_sign`, `show_hidden_columns`) are still left out; an analyzed empty string, and on SNII any text that yields no token, counts as one token on a merge-on-write table; SNII norm length sums are not cached. Statistics stay per tablet. ### Release note Fix BM25 `score()` on merge-on-write tables differing between the replicas of a tablet, and before and after compaction. Statistics now count only the rows a query can return, and the average document length comes from the indexed norms. ### Check List (For Author) - Test - [x] Regression test: - New: `inverted_index_p0/test_bm25_score_replica_statistics` and `fault_injection_p0/test_bm25_score_statistics_searcher_io_context` (same suites as #69438). Updated `.out`: `test_bm25_score.out`, `test_score_filter_matrix.out` (V2, V3 and SNII merge-on-write cases), same values as #69438. - All 26 branch-4.2 suites that use `score()` (`inverted_index_p0`, `search`, `ann_index_p0`, `variant_p0`, `nereids_rules_p0`, `fault_injection_p0`) pass with this change on a 3-BE cluster. Without it, the replica suite, the fault-injection suite (the query fails with `[E-6002] The 'is_index_data' flag should be false for non-meta inverted index files.`) and the two updated suites fail. - [x] Unit Test: the tests of #69438 (`CollectionStatisticsTest.HiddenRowsAreCollectedLikeCompactedAway`, `AllRowsHiddenCollectsEmptyStatistics`, `NullRowsCountAsDocumentsWithoutTokens`, `CollectionStatisticsHiddenRowsTest.*` for `count_listed_docs`, `exclude_hidden_docs` and `exclude_hidden_term_docs`, `CluceneSearcherCacheValueTest.*`, `MultiSegmentCollectorTest.NormLengthSumsReadEachSegment`, `OlapScannerTest.BuildScoreRuntimeCollectionIoContextFollowsQueryFileCacheOptions`), with the existing collection statistics, BM25, searcher cache, multi-segment, query engine, SNII and `SEARCH` function tests: 427 tests in 35 suites pass in an ASAN build on macOS arm64. - [x] Manual test (3 BEs, macOS arm64, `replication_num = 3`, auto compaction off, `enable_sql_cache = false`), base = branch-4.2 without this change, same data and compaction state for both builds: | Check | branch-4.2 | with this change | |---|---|---| | CDC-style table (8 buckets, upserts, delete-sign deletes, NULLs, 4 fields), one BE's replicas fully compacted with inverted index compaction off: rows with more than one score over 15 runs / differing between replicas 0, 1, 2 (of 1,731) | 1,731 / 1,731 | 0 / 0 | | Same, scores differing from a freshly loaded copy of the visible rows | — | 0 of 1,731 | | Partial-column updates, sequence column, stream load with the delete sign and `merge_type: DELETE`, group commit, keyword and lowercase-normalizer indexes, partitions, variant, SNII: rows differing between replicas after one replica compacted | 84–790 per table (8 of 9 checks) | 0 in every check, all equal to a fresh copy | | Merge-on-write vs duplicate table with 50 % NULL rows, V3 and SNII | 0 of 250 differ | 0 of 250 differ | Cost, query `SEARCH('content:jobs OR content:try') ORDER BY score() DESC LIMIT 100`, one bucket, median of 15 warm runs; `StatisticsCollectTime` with one scanner, query total of `SEARCH('content:jobs')` with the default parallel scan: | Table | Statistics | Query total | |---|---|---| | V2, 8 M rows + 200 k overwritten + 50 k delete-sign rows | 0.3 ms → 6.7 ms | 68 ms → 76 ms | | V3, 6 M rows + 2 M overwritten + 200 k delete-sign rows (2.2 M hidden, 6 % NULL) | 0.3 ms → 55 ms | 61 ms → 113 ms | | V2, 6 M rows + 2 M overwritten, no NULL rows | 0.3 ms → 7.2 ms | 56 ms → 61 ms | | SNII, 2 M rows + 500 k overwritten + 200 k delete-sign rows | 0.2 ms → 23 ms | 49 ms → 71 ms | The cost grows with the rows compaction has not removed yet. With NULL rows, most of it is leaving the NULL rows out of the hidden rows on every query (`exclude_hidden_docs`), as in #69438. Memory (BE allocated bytes, sum of 3 BEs, a 4-field merge-on-write table of 41 segments): no growth beyond the noise of the measurement (after 4 queries +26 MB → +25 MB with key pruning; +142 MB → below the starting value scoring all segments). - Behavior changed: - [x] Yes. On merge-on-write tables, BM25 statistics leave out rows hidden by the delete bitmap and rows with the delete sign, and avgdl is computed from the norms of the visible rows, so scores no longer depend on the replica or on compaction. Scores of merge-on-write tables shift once, also without deletes, because norms store long lengths approximately. - Does this need documentation? - [x] No. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label Co-authored-by: Shivam Gupta <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: #69352
Problem Summary:
Backport of #69352 to branch-4.2.
A parallel scan (
ParallelScannerBuilder) splits one tablet into several scanners. EachOlapScannerthen callsCollectionStatistics::collect()with only thers_splitsof its own split, so the BM25 doc count, avgdl and term doc frequencies come from a subset of the tablet's rowsets. Rows with identical text get different scores depending on which scanner reads them. Because the split depends on the tablet's row count and the core count, a row's score and its rank inORDER BY score()change when unrelated rows are loaded, and pagination over a score-ordered search returns repeated and missing rows.Fix (same as #69352):
TabletCollectionStatisticsholds the rowsets of a tablet's entire read source. The first scanner of the tablet to prepare collects the statistics once withcollect_full_collection(), and every scanner of that tablet copies them into its ownCollectionStatistics. Later scanners wait for the first collection and get its result or error.CollectionStatistics::copy_collected_from()copies the collected counters and resets the derived idf/avgdl caches, so no cache is shared between scanner threads.ParallelScannerBuildercreates one holder per tablet, only when the scan has ascore()runtime. Both the by-rowid and by-segment build paths pass it to their scanners throughOlapScanner::Params.Differences from #69352: branch-4.2 does not have the changelog, TSO and bucket fields of
OlapScanner::Paramsor thescan_rangeargument of_build_scanner, so the scanner changes were applied by hand on top of the branch-4.2 code.OlapScanner::Paramsis initialized positionally on this branch, so the new field is appended at the end. branch-4.2 has no test that calls_build_scannerdirectly, so the follow-up test fix from #69352 is not needed here.Statistics remain per tablet. Making them consistent across tablets and BEs is out of scope.
Release note
Fix BM25
score()returning different scores for identical rows of the same tablet when a parallel scan split the tablet into several scanners.Check List (For Author)
Test
Regression test:
inverted_index_p0/test_bm25_score_parallel_scan_statistics(same suite as [fix](inverted index) Share BM25 statistics across parallel scanners of a tablet #69352). Passes on branch-4.2 with this change.Unit Test:
CollectionStatisticsDetailedTest.CopyCollectedFromScoresLikeSource,TabletCollectionStatisticsTest.CollectsOnceAndCopiesToEveryCaller(compiled locally; run left to CI)Manual test: ran the reproduction from [fix](inverted index) Share BM25 statistics across parallel scanners of a tablet #69352 against branch-4.2 builds on macOS arm64.
enable_parallel_scan=true(split forced)enable_parallel_scan=falsea5d2661d, without this changea5d2661dwith this changeNo need to test or manual test.
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)