mirror of
https://github.com/facebook/rocksdb.git
synced 2026-10-24 19:59:35 +03:00
7be9ffc81a
Summary:
Fixes a correctness bug in read-path range-tombstone synthesis when it races with `IngestExternalFile`. The synthesis path could insert a tombstone into the active memtable at a snapshot's sequence number, while ingestion installed an L0 SST at `LastSequence + 1` — a higher seqno than the synthesized tombstone. This breaks the main assumption of range tombstone reads that all lower levels have lower seqno.
The fix introduces a per-CF `port::RWMutex` (`ColumnFamilyData::ingest_sst_lock_`) plus a per-memtable `ingest_seqno_barrier_`. Ingestion takes the read lock and range tombstone synthesis **tries** to take a write lock.
If iterator lock is successful, then we have a new updated barrier seqno that we can validate the iterator seqno against. An added benefit is we no longer need to gate against empty memtable. This was originally added as an easy fix to prevent memtables from being inserted into while ingestion was happening.
## The bug, by example
`ReadPathRangeTombstoneTest.NewerPointInOlderFileStillVisible` (`db/db_iterator_test.cc:6929`):
1. L0 file `b@1, c@2, d@3`, then L0 file `Delete(b)4, Delete(c)5`.
2. Active memtable: `Put(z)6`. Snapshot taken at seq 6.
3. `IngestExternalFile({c → "vc_live"})` → installed at L0 with seq 7.
4. Iterator at snap 6 walks the deletion run and synthesizes `[b, d) @ seq 6` into the active memtable via `MemTable::AddLogicallyRedundantRangeTombstone`.
5. `Get("c")` at the latest snapshot: memtable returns covering tombstone (seq 6), `Version::Get` short-circuits, **never reads `c@7`** — returns `NotFound` instead of `"vc_live"`.
The invariant `Version::Get` relies on (memtable seqs ≥ any L0 seq for the same key) is broken because synthesis writes at the *snapshot's* seq while ingestion writes at `LastSequence + 1`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14654
Test Plan:
- Updated regression test
- Re-enable crashtests and manually run
| Flavor | Jobs |
|---|---:|
| `fbcode_blackbox_crash_test` | 200 |
| `fbcode_whitebox_crash_test` | 30 |
| `fbcode_asan_blackbox_crash_test` | 30 |
| `fbcode_tsan_blackbox_crash_test` | 30 |
| `fbcode_crash_test_with_atomic_flush` | 30 |
| `fbcode_crash_test_with_wc_txn` | 30 |
| `fbcode_crash_test_with_ts` | 30 |
| **Total** | **380** |
Reviewed By: xingbowang
Differential Revision: D102044512
Pulled By: joshkang97
fbshipit-source-id: 0c69187595edc5a5fa80be24bffdba710a92e56e