LemonCL opened a new pull request, #70179:
URL: https://github.com/apache/doris/pull/70179
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #59413 (deferred deletion of pending delete bitmaps), #49046 /
#50417 / #51353 (split large values with `blob_put`)
Problem Summary:
`update_delete_bitmap` (lock_id > 0 for load, -2 for schema change) records
all delete bitmap keys written by the request into ONE `PendingDeleteBitmapPB`
and stores it with a plain `txn->put`. On a MoW table with many incremental
versions, this pending value can exceed the FDB single-value limit (100KB,
error 2103 `TXN_VALUE_TOO_LARGE`), so the whole request fails and, for example,
an `ALTER TABLE ADD INDEX` on a high-frequency upsert table is CANCELLED.
The delete bitmap values themselves are already split by `blob_put`; only
the pending value (the list of delete bitmap keys used to clean up orphans of
failed trials) was not. In my reproduction, ~820 keys (~122B/key) are enough to
hit the limit, and a busy MoW tablet can accumulate thousands of them within
one schema change window.
This PR writes the pending delete bitmap with `blob_put` when it is too
large:
- New mutable config `split_pending_delete_bitmap_kv` (default `false`).
When enabled and the pending value exceeds `DEFAULT_BLOB_SPLIT_SIZE`, it is
written with `blob_put` (ver 0); otherwise the original plain `put` is kept, so
the default behavior and the storage format are unchanged.
- Before writing, the whole range `[pending_key, pending_key +
enc(INT64_MAX))` is removed, so stale split chunks and the bare KV of a
previous write never get mixed.
- All readers (`remove_pending_delete_bitmap`, merging pending values of
previous sub txns of an explicit txn, and the checker) switch to `blob_get`,
which is compatible with both the bare and the split format, so the read side
is always on.
- All delete sites (`commit_txn_immediately`, `commit_txn_eventually`,
deferred deletion in the txn lazy committer, schema change finish,
`recycle_tablet`, `recycle_versioned_tablet`) switch to the same range remove,
otherwise split chunks would be leaked.
Rolling upgrade: enable the switch only after every MS instance is upgraded;
before downgrading MS, turn it off and wait until the split pending KVs are
cleaned.
### Release note
Fix the FDB `TXN_VALUE_TOO_LARGE` error (e.g. schema change cancelled)
caused by an oversized pending delete bitmap value on MoW tables, by splitting
it into multiple KVs (disabled by default, enable with MS config
`split_pending_delete_bitmap_kv=true`).
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
New unit tests cover every read/write/delete path of a split pending delete
bitmap:
- `MetaServiceTest.UpdateDeleteBitmapSplitLargePendingDeleteBitmap`: large
value split, small value plain, stale split chunks removed on overwrite,
alternating split/plain writes, switch on/off.
-
`MetaServiceTest.UpdateDeleteBitmapSplitPendingDeleteBitmapWithExplicitTxn`: a
following sub txn reads the split pending value of previous sub txns and merges
its own keys into it.
- `MetaServiceTest.CommitTxnImmediatelyRemoveSplitPendingDeleteBitmap` /
`CommitTxnEventuallyRemoveSplitPendingDeleteBitmap`: committing the load
removes every split KV.
- `MetaServiceJobTest.SchemaChangeJobRemoveSplitPendingDeleteBitmap` (delete
bitmap lock v1/v2): finishing the schema change removes every split KV.
- `RecyclerTest.recycle_tablet_with_split_pending_delete_bitmap` and
`CheckerTest.delete_bitmap_inverted_check_normal_split_pending`.
Manual verification (ASAN build):
- Mutation check: reverting the recycler/checker delete/read sites to the
single-key form makes the new recycler/checker cases fail, so they really guard
this fix.
- `meta_service_test
--gtest_filter='*DeleteBitmap*:*CommitTxn*:*SchemaChange*'`: 52/52 passed.
- `recycler_test`: 204/205 passed; the only failure is
`RecyclerBenchmarkTest.RecycleRowsets`, which requires a real FDB and fails at
`txn_kv` init in the test container (unrelated to this change).
- The same fix has been verified end-to-end on a cloud cluster with
high-frequency upsert plus `ALTER ADD INDEX`, compaction, tablet recycle and
checker.
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
The switch is off by default and the read side is compatible with both
formats.
- Does this need documentation?
- [ ] No.
- [x] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
The new MS config `split_pending_delete_bitmap_kv` should be documented; a
documentation PR will follow if reviewers agree with the switch.
### Unrelated issue found while testing (not addressed here)
When `txn_lazy_commit_defer_deleting_pending_delete_bitmaps=true`,
`commit_txn_eventually` persists `txn_info` before setting
`defer_deleting_pending_delete_bitmaps`, so the lazy commit task reads a stale
`txn_info` without the flag and neither the pending delete bitmap nor the
delete bitmap update lock is removed by MS. This is pre-existing on master and
independent of this PR; I can file a separate issue/PR for it.
--
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]
