guptashivam04 opened a new pull request, #69418:
URL: https://github.com/apache/doris/pull/69418
### What problem does this PR solve?
Issue Number: None
Related PR: #69352
Problem Summary:
Backport of #69352 to branch-4.1.
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.
The reproduction in #69352 (8 rowsets with a different share of matching
rows, forced split with `parallel_scan_min_rows_per_scanner=1024`) gives the
2,280 identical matching rows **15 distinct scores (1.34 to 5.81)** on 4.1.4,
4.1.5 and branch-4.1. With `enable_parallel_scan=false` they all get 3.647686.
Fix (same design as #69352, adapted to branch-4.1):
- `TabletCollectionStatistics` (in
`storage/compaction/collection_statistics.h`) holds the `rs_splits` of a
tablet's entire read source. The first scanner of the tablet to prepare
collects the statistics once with `CollectionStatistics::collect()` over all of
them, 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: on branch-4.1, `CollectionStatistics` lives in
`storage/compaction/`, has no `SearchPredicateCollector` constructor argument
and no `collect_full_collection()`, so the holder keeps the read source's
`rs_splits` and calls `collect()` on them. `OlapScanner::Params` is initialized
positionally on this branch, so the new field is appended at the end.
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
- [x] Regression test:
`inverted_index_p0/test_bm25_score_parallel_scan_statistics` (same suite as
#69352). Passes on branch-4.1 with this change.
- [x] Unit Test:
`CollectionStatisticsDetailedTest.CopyCollectedFromScoresLikeSource`,
`CollectionStatisticsTest.TabletCollectionStatisticsCollectsOnceAndCopiesToEveryCaller`
(not run locally; left to CI)
- [x] Manual test: ran the reproduction from #69352 against branch-4.1
builds on macOS arm64.
| Build | `enable_parallel_scan=true` (split forced) |
`enable_parallel_scan=false` |
|---|---|---|
| branch-4.1, without this change | 15 distinct scores (1.34 to 5.81)
| 1 score (3.647686) |
| branch-4.1 `95846ddf` with this change | 1 score (3.647686), same
over 5 repeated runs | 1 score (3.647686) |
- [ ] No need to test or manual test.
- Behavior changed:
- [x] 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?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
