Skip to content

branch-4.2: [fix](inverted index) Share BM25 statistics across parallel scanners of a tablet #69352 - #69440

Merged
yiguolei merged 1 commit into
apache:branch-4.2from
guptashivam04:backport/4.2-bm25-tablet-statistics
Oct 23, 2026
Merged

yiguolei merged 1 commit into
apache:branch-4.2from
guptashivam04:backport/4.2-bm25-tablet-statistics

Conversation

@guptashivam04

Copy link
Copy Markdown
Contributor

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. Each OlapScanner then calls CollectionStatistics::collect() with only the rs_splits of 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 in ORDER BY score() change when unrelated rows are loaded, and pagination over a score-ordered search returns repeated and missing rows.

Fix (same as #69352):

  • TabletCollectionStatistics holds the rowsets of a tablet's entire read source. The first scanner of the tablet to prepare collects the statistics once with collect_full_collection(), and every scanner of that tablet copies them into its own CollectionStatistics. 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.
  • ParallelScannerBuilder creates one holder per tablet, only when the scan has a score() runtime. Both the by-rowid and by-segment build paths pass it to their scanners through OlapScanner::Params.
  • The non-parallel path is unchanged. Each of its scanners already receives the tablet's full read source.

Differences from #69352: branch-4.2 does not have the changelog, TSO and bucket fields of OlapScanner::Params or the scan_range argument of _build_scanner, so the scanner changes were applied by hand on top of the branch-4.2 code. OlapScanner::Params is initialized positionally on this branch, so the new field is appended at the end. branch-4.2 has no test that calls _build_scanner directly, 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

  • Behavior changed:

    • Yes. Score queries that use a parallel scan now score every row of a tablet against statistics of the whole tablet. Scores can differ from earlier releases but are now independent of the scanner split.
  • Does this need documentation?

    • No.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

…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.
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@yiguolei

Copy link
Copy Markdown
Contributor

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 36.07% (22/61) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 64.27% (29899/46523)
Line Coverage 49.12% (315379/642116)
Region Coverage 44.98% (254115/564971)
Branch Coverage 46.61% (119899/257247)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 96.72% (59/61) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 76.39% (34513/45182)
Line Coverage 61.23% (389846/636737)
Region Coverage 58.15% (329086/565884)
Branch Coverage 59.04% (151717/256955)

@yiguolei
yiguolei merged commit 7ad1367 into apache:branch-4.2 Oct 23, 2026
28 of 29 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Oct 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants