Repository navigation
branch-4.1: [fix](inverted index) Share BM25 statistics across parallel scanners of a tablet #69352 - #69418
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.
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
…M25 parallel scan regression Fuzzy mode on branch-4.1 may turn enable_common_expr_pushdown off, and SEARCH() then fails with "SearchExpr should not be executed without inverted index". Enable it explicitly, as the other BM25 suites do.
|
The |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
PR approved by anyone and no changes requested. |
|
PR approved by at least one committer and no changes requested. |
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. 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.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. Withenable_parallel_scan=falsethey all get 3.647686.Fix (same design as #69352, adapted to branch-4.1):
TabletCollectionStatistics(instorage/compaction/collection_statistics.h) holds thers_splitsof a tablet's entire read source. The first scanner of the tablet to prepare collects the statistics once withCollectionStatistics::collect()over all of them, 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: on branch-4.1,
CollectionStatisticslives instorage/compaction/, has noSearchPredicateCollectorconstructor argument and nocollect_full_collection(), so the holder keeps the read source'srs_splitsand callscollect()on them.OlapScanner::Paramsis 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
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.1 with this change.Unit Test:
CollectionStatisticsDetailedTest.CopyCollectedFromScoresLikeSource,CollectionStatisticsTest.TabletCollectionStatisticsCollectsOnceAndCopiesToEveryCaller(not run locally; 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.1 builds on macOS arm64.
enable_parallel_scan=true(split forced)enable_parallel_scan=false95846ddfwith this changeNo need to test or manual test.
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)