Fix sequence publication race in allow-write external SST ingestion - #433
Fix sequence publication race in allow-write external SST ingestion#433LykxSassinator wants to merge 1 commit into
Conversation
Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
IngestExternalFiles(..., allow_write=true)allowed foreground writes to run while external SST ingestion assigned and published global sequence numbers. A concurrent multi-key WriteBatch could complete after ingest readLastSequence()but before ingest published its consumed sequence number, causing the visible sequence to move backwards. New RocksDB snapshots could then miss committed data.This change restores serialization only for the final ingestion phase:
Run()and wait for pending writes.Run(),LogAndApply, sequence publication, and SuperVersion installation.This preserves most of the latency benefit of
allow_writewhile restoring the required global sequence linearization.Tests
two_write_queuespaths, with and withoutatomic_flush.make -j4 external_sst_file_testexternal_sst_file_test: 99/99 passedgit diff --checkclang-format --dry-run --Werror