guptashivam04 opened a new pull request, #69854:
URL: https://github.com/apache/doris/pull/69854
### What problem does this PR solve?
Issue Number: None
Related PR: #69438 (BM25 statistics of merge-on-write tables count only the
rows the scan returns). This PR carries the review points left open when #69438
was merged.
Problem Summary:
#69438 leaves out of a merge-on-write table's BM25 statistics the rows the
delete bitmap hides, and the rows written with the delete sign, so that every
replica of a tablet scores alike. Review raised three points it did not address
before merging:
1. **Rows with the delete sign are left out even when the query returns
them.** With `skip_delete_sign = true` or `show_hidden_columns = true`, the FE
leaves out the `__DORIS_DELETE_SIGN__ = 0` conjunct, so the scan returns and
scores those rows, but the statistics still hide them. Their scores then differ
from those of a table holding the rows the query returns. Reproduction: the new
suite below fails on master at once (`V2, skip_delete_sign, compared with the
returned rows: 313 rows score differently`).
2. **SNII statistics redo their work on every query, and a buffer goes
untracked.** On SNII the statistics opened each segment's logical index and
summed its norms again on every query; about 39 ms of the 47 ms the statistics
took on a 2 M-row table. On CLucene, each query filtered its hidden rows by the
null bitmap only to build the key of the cached sum. Summing the norms of
selected rows decoded them into one buffer as large as the selection, which the
memory tracker did not see.
3. **CLucene norm reads ignore the query's I/O context.** A CLucene reader
reads a field's norms through a stream it keeps open for its lifetime and
shares between every query of a cached searcher, so the stream carries no
query's context. The statistics' norm reads therefore ignored
`disable_file_cache` and the remote cache write limiter, and counted to no
query's file cache statistics. Setting the query's context on that shared
stream would leave a pointer to the query on a cached reader.
Fix:
- **Delete sign.** Neither setting is sent to the BE, so
`OlapScanLocalState` notes, before the conjuncts are normalized into column
predicates and storage expressions, whether a scan conjunct reads the
delete-sign slot (`MowHiddenRows::filters_delete_sign`). The statistics hide
rows with the delete sign only when it does. Rows the delete bitmap marks stay
hidden whatever the query sets, and a query that filters on the delete sign
itself (`show_hidden_columns = true` with `WHERE __DORIS_DELETE_SIGN__ = 0`)
hides them, as the default does.
- **Cached sums for both formats.** The norm length sums move out of
`CluceneSearcherCacheValue` into a `NormLengthSums` that both
`CluceneSearcherCacheValue` and `SniiLogicalReaderCacheValue` hold: the sum of
a field's non-NULL norm lengths, and the sum of the non-NULL hidden rows'
lengths for the last four sets of hidden rows of a field. A set is keyed by the
hidden rows themselves (count, first, last and an xxHash64 of the row IDs), so
a query whose sums are kept reads neither norms nor NULL rows. The sums are
computed outside the lock, take a few dozen bytes per field, and are evicted
with the reader, so the searcher cache's capacity bounds them.
- **SNII reader from the searcher cache.** The statistics take a segment's
logical index from the searcher cache with the query path's key and cache value
(`SniiIndexReader::_get_logical_reader`), and insert it on a miss, instead of
opening it on every query.
- **Bounded decoding.** `IndexSource::sum_norm_lengths_of` decodes the norms
of selected rows 4,096 rows at a time. The NULL row ID list SNII builds is
charged to the query's memory.
- **Scoped I/O context.** `DorisCompoundReader::ScopedIOContext` gives the
reads a thread makes through sub-file streams without a context of their own
the query's context, for the duration of the statistics' norm read.
`CSIndexInput` drops the context from the shared base stream after every read,
also when the read throws, so no query's pointer stays on a cached reader.
Not covered:
- Rows deleted by a delete predicate (`enable_mow_light_delete`) are still
counted, as the delete bitmap does not mark them. Left for a separate PR.
- A reader that does not fit one shard of the searcher cache (its capacity /
256: about 838 KB on a BE with a 2 GB `mem_limit`, where a 1 M-row SNII
segment's reader takes about 1.4 MB) leaves the cache at once, for the query
path as for the statistics. Such a segment's norms are still summed on every
query, as before this PR.
- The query path's own CLucene scoring reads norms (`SegmentTermDocs`
through `IndexReader::norms(field)`) without a per-read I/O context, as before.
This PR scopes the context only around the statistics' norm reads.
- The BE cannot tell the FE's delete-sign conjunct from a user's, so any
scan conjunct that reads `__DORIS_DELETE_SIGN__` counts as the filter. A query
with `show_hidden_columns = true` and `WHERE __DORIS_DELETE_SIGN__ = 1` returns
only delete-sign rows but scores them on the statistics of the visible rows. It
still scores alike on every replica.
- Like the CLucene statistics before it, the SNII statistics look up and
fill the searcher cache even when the query sets
`enable_inverted_index_searcher_cache = false`, which the query path honours.
- Rows that row-level TTL (#65858) has expired are filtered by the query but
still counted until compaction removes them, as the delete bitmap does not mark
them. On a table with row TTL, a replica that has compacted them away can
therefore score differently. Left for a separate PR, with the light deletes
above.
### Release note
Fix BM25 `score()` on merge-on-write tables with `skip_delete_sign` or
`show_hidden_columns`: the statistics now count the rows written with the
delete sign that the query returns.
### Check List (For Author)
- Test (regression and manual tests on master `d420f9c41b`, which includes
#69698 and #67915, with these commits; release build, macOS arm64, 3 BEs on one
host, `enable_sql_cache = false`. The commits merge cleanly with master
`2a4191c1dc` and with #69810, and the build hygiene checks pass on that merge.)
- [x] Regression test:
`inverted_index_p0/test_bm25_score_delete_sign_statistics` (new). On `V2`, `V3`
and `SNII`, a merge-on-write table with overwritten rows, delete-sign rows
(CDC-style inserts with `__DORIS_DELETE_SIGN__ = 1`) and a `DELETE`. With
`skip_delete_sign = true` and with `show_hidden_columns = true`, every returned
row must score exactly as in a duplicate-key table loaded with the rows the
query returns; by default, as in one loaded with the visible rows. `.out`
results pin the scores of each setting, and a `show_hidden_columns` query that
filters `__DORIS_DELETE_SIGN__ = 0` itself scores as the default. It fails
without this PR and passes with it.
- [x] Existing suites: all 28 other suites that use `score()` (in
`inverted_index_p0`, `search`, `ann_index_p0`, `variant_p0`, `nereids_rules_p0`
and `fault_injection_p0`, including `test_bm25_score_replica_statistics` and
`test_bm25_score_statistics_searcher_io_context` from #69438 and
`test_match_phrase_prefix_score` from #69698) pass on a 3-BE cluster: 29
suites, 0 failed.
- [x] Unit Test: `OlapScanOperatorDeleteSignTest.*` (a scan conjunct on
the delete-sign slot, also once pushed down as a storage expression, filters
the rows; a conjunct on another column, an output-only delete-sign slot, or a
table without one does not),
`CollectionStatisticsTest.DeleteSignRowsCountWhenTheScanReturnsThem` (real
CLucene segment: with `filters_delete_sign = false` only the delete-bitmap rows
are left out, without reading the delete-sign column),
`CollectionStatisticsHiddenRowsTest.ExcludeHiddenDocsKeepsSumsWithTheIndex` (a
repeated query with the same hidden rows reads neither norms nor NULL rows;
other hidden rows read the NULL rows once and only their own norms),
`CollectionStatisticsHiddenRowsTest.SumNormLengthsOfDecodesBoundedBatches`
(10,000 selected rows decode in 4,096-row batches), `NormLengthSumsTest.*`, and
`DorisCompoundReaderTest.ScopedIoContextReachesStreamsWithoutTheirOwn` (a
sub-file stream without a context reads with the scoped one and counts t
o its statistics, a stream with its own keeps it, and the shared base stream
holds no context after a read, also after a failing read). Run locally on macOS
(ASAN): 288 tests in the BM25, collection-statistics, scanner, scan-operator,
compound-reader, searcher-cache, inverted-index-reader, null-bitmap and
null-predicate suites pass. These unit tests ran on #69438's head with these
commits; they have not been rebuilt on current master.
- [x] Manual test.
Replicas: a table like the reproduction of #69438 (4 indexed text
fields, 3 replicas, 8 buckets, 8 loads, 4 rounds of CDC upserts, delete-sign
deletes and a `DELETE`), on V3 and on SNII, queried with
`SEARCH('note_search:jobs OR task_search:jobs OR meeting_search:jobs OR
profile_search:jobs') AND accountid = 12345`. Before any compaction and after a
full compaction of one replica of every tablet (1 rowset on that replica, 15 on
the others), every one of the 1,595 rows has one score across 15 random-replica
runs and `use_fix_replica = 0/1/2`, the scores after compaction equal those
before, and each replica's scores equal those of a table freshly loaded with
the visible rows (0 rows differ). With `skip_delete_sign = true` the two
uncompacted replicas score identically; the compacted one returns 109 rows
fewer, as base compaction has removed the delete-sign rows.
Cost, against master `d420f9c41b`, single-replica tables in one
bucket, `inverted_index_searcher_cache_limit = 1 GB`. Query
`SEARCH('content:jobs OR content:try') ORDER BY score() DESC LIMIT 100`;
`StatisticsCollectTime` and query total from the profile, two alternating
rounds each:
| Table | statistics, repeated query (median of 15) | query total |
statistics, cold query (median of 8, caches cleared) |
|---|---|---|---|
| SNII, 1.8 M rows | 45.5 / 45.4 → 7.8 / 7.7 ms | 105 / 106 → 85 / 82
ms | 46.3 / 45.6 → 46.5 / 45.9 ms |
| CLucene V2, 7.9 M rows | 61.6 / 61.7 → 13.3 / 13.3 ms | 153 / 152 →
139 / 140 ms | 89.9 / 95.4 → 89.5 / 91.7 ms |
| CLucene V2, 6 M rows + 2 M overwritten | 196.3 / 196.1 → 26.0 / 25.6
ms | 158 / 160 → 132 / 119 ms | 224.1 / 224.2 → 218.3 / 218.2 ms |
With the default searcher cache of a BE with a 2 GB `mem_limit`, 1
M-row SNII segments' readers do not stay in the cache, and their statistics
cost about the same as before (44 ms vs 47 ms, measured on #69438's head).
Memory: BE tcmalloc allocated bytes (sum of the 3 BEs) before and
after 1 and 4 queries on each table move within the ±150 MB the sum fluctuates
by, with and without this PR, and grow no more with it.
- Behavior changed:
- [x] Yes. On merge-on-write tables, queries with `skip_delete_sign =
true` or `show_hidden_columns = true` (without their own filter on
`__DORIS_DELETE_SIGN__`) now count the delete-sign rows they return in the BM25
statistics, so their scores change. Default queries score as before.
- 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]
