guptashivam04 opened a new pull request, #69352:
URL: https://github.com/apache/doris/pull/69352
### What problem does this PR solve?
Issue Number: None
Related PR: #69313 (BM25 statistics after compaction), #66979 (deleted
documents in score TopN)
Problem Summary:
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.
The split depends on the tablet's total row count,
`parallel_scan_max_scanners_count` (defaulting to the core count) and
`parallel_scan_min_rows_per_scanner`. A row's score and its rank in `ORDER BY
score()` therefore change when unrelated rows are loaded into the tablet, and
differ between BEs with different core counts. Pagination over a score-ordered
search returns repeated and missing rows.
Reproduction (one bucket, one replica, no concurrent writes):
```sql
CREATE TABLE t (
id BIGINT,
content TEXT,
INDEX idx_content (content) USING INVERTED PROPERTIES("parser" =
"english")
) UNIQUE KEY(id) DISTRIBUTED BY HASH(id) BUCKETS 1
PROPERTIES ("replication_num" = "1", "enable_unique_key_merge_on_write" =
"true",
"disable_auto_compaction" = "true");
-- 8 loads (8 rowsets), each with a different share of matching rows
INSERT INTO t SELECT 0 * 2000 + number, IF(number % 2 = 0, 'we should try
these jobs today', 'call the candidate about salary') FROM numbers("number" =
"2000");
INSERT INTO t SELECT 1 * 2000 + number, IF(number % 5 = 0, 'we should try
these jobs today', 'call the candidate about salary') FROM numbers("number" =
"2000");
-- ... batches 2..7 with modulus 8, 11, 14, 17, 20, 23
SELECT /*+ SET_VAR(enable_parallel_scan=true,
parallel_scan_min_rows_per_scanner=1024, parallel_scan_max_scanners_count=16) */
score() AS s, count(*)
FROM (SELECT score() FROM t WHERE SEARCH('content:jobs OR content:try')
ORDER BY score() DESC LIMIT 100000) x
GROUP BY s;
```
On 4.1.4, the 2,280 matching rows all have the same text but get **15
distinct scores (1.34 to 5.81)**. With `enable_parallel_scan=false` they all
get 3.647686. On a production-sized table, the default thresholds give the same
behavior.
Fix:
- `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 per-segment build paths pass it
to their scanners.
- The non-parallel path is unchanged. Each of its scanners already receives
the tablet's full read source.
Statistics remain per tablet. Making them consistent across tablets and BEs
(as with Elasticsearch's `dfs_query_then_fetch`) 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`. It fails on 4.1.4
with 15 distinct scores and expects one score, equal to the serial-scan score.
- [x] Unit Test:
`CollectionStatisticsDetailedTest.CopyCollectedFromScoresLikeSource`,
`TabletCollectionStatisticsTest.CollectsOnceAndCopiesToEveryCaller` (not yet
run locally; left to CI)
- [x] Manual test: built master with and without this change on macOS
arm64 and ran the reproduction above against each.
| Build | `enable_parallel_scan=true` (split forced) |
`enable_parallel_scan=false` |
|---|---|---|
| master `33c460d7`, without this change | 15 distinct scores (1.34 to
5.81) | 1 score (3.647686) |
| master with this change | 1 score (3.647686) | 1 score (3.647686) |
The new regression suite fails on the build without this change
(`expected: <1> but was: <15>`) and passes with it. A 4-bucket table queried
with a multi-field `SEARCH()` and a non-index filter returned the same per-row
scores across 8 forced-split runs, matching the serial scan.
- [ ] 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]
