mirror of
https://github.com/facebook/rocksdb.git
synced 2026-07-07 14:47:40 +08:00
update-folly-11.6
13973 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Peter Dillinger
|
fec822a96e | Merge remote-tracking branch 'origin/main' into update-folly-11.6 | ||
|
Peter Dillinger
|
fa8cfaa774 | Fix cmake, add learning to CLAUDE.md | ||
|
anand1976
|
13d55eba73 |
Update version to 11.7.0 for 11.6 release (#14913)
Summary: - Bump version.h from 11.6.0 to 11.7.0 - Add `11.6.fb` to `check_format_compatible.sh` - Sync HISTORY.md `## 11.6.` section from `11.6.fb` - Delete 5 consumed `unreleased_history/` note file(s) from main Part of 11.6 release workflow. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14913 Reviewed By: anand1976 Differential Revision: D110520723 Pulled By: pdillinger fbshipit-source-id: 20f2f9b4be03c08d78d6f2c4e6b70812a408d9f0 |
||
|
Peter Dillinger
|
1192897aad |
Fix table cache entry leak in VersionBuilder::UnrefFile on failed LogAndApply (#14914)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14914 Pull Request resolved: https://github.com/facebook/rocksdb/pull/14720 ### Context `VersionSet::LogAndApply` loads table handlers before the `MANIFEST` update is durable. If that `MANIFEST` update later fails, newly added files can be discarded by `VersionBuilder` without ever being installed in a `Version`. `VersionBuilder::UnrefFile` released the `FileMetaData` `pinned_reader` handle, but releasing that handle only dropped the reference; it did not erase the cache key, so an orphaned table-cache entry could survive for a file that is neither live nor quarantined. When metadata read fault injection prevents the obsolete-file scan from cleaning up the orphan, DB close can trip `TEST_VerifyNoObsoleteFilesCached` with File N is not live nor quarantined. ### Fix After releasing a pinned_reader for a `FileMetaData` whose refcount reaches zero, explicitly evict that file number from the table cache. The new `VersionBuilderDBTest.FailedLogAndApplyEvictsTableCacheEntry` regression test exercises the realistic production path: it creates a real SST, calls `LogAndApply` through the DB's own `VersionSet`, injects a MANIFEST sync failure via the `AfterSyncManifest` sync point, and asserts that the table cache no longer contains the file after `ProcessManifestWrites` destroys the `VersionBuilder`. With the eviction removed, the test fails deterministically with `leaked_cache_entry=true`. Reviewed By: hx235 Differential Revision: D99759696 fbshipit-source-id: 0ebb32b1544ad1982d23a6c06569984be97a0a88 |
||
|
Peter Dillinger
|
f2c0eb41ef | Fixes (thanks claude) | ||
|
anand1976
|
b6ca6d886e | Update folly hash to 2a68075b77d9 for 11.6 release | ||
|
Peter Dillinger
|
a40466d963 |
Three misc CI fixes (#14910)
Summary: Fixes three unrelated nightly/CI failures. Makefile: 'make build_folly' was tripping the build-signature change check, causing the folly-flavored jobs (e.g. nightly clang-21 ASAN+UBSAN+folly, which runs 'make build_folly' then 'USE_FOLLY=1 ... make check') to abort with 'Build parameters changed since the last build'. build_folly only builds the external folly library under third-party/folly and never compiles RocksDB objects into OBJ_DIR, so it should not record or check a build signature; add it to BUILD_SIG_NONBUILD_GOALS alongside checkout_folly. util/rate_limiter_test.cc: RateLimiterTest.Rate's minimum-rate assertion is timing-sensitive and flakes on the small/loaded ARM CI runners, which (unlike x86_64) can't reliably sustain the target rate. Extend the existing CI exemption with an ARM-only GITHUB_ACTIONS branch, preserving the check on x86_64. tools/ldb_test.py: the list_live_files_metadata output includes the full DB path, and the unescaped-dot regex '\d+.sst' could match a <digit><char>sst run inside the random tempfile.mkdtemp suffix (drawn from [a-z0-9_]) that precedes the real filename, collapsing testMap to a single bogus key such as '8asst'. Require a literal '.sst'; temp-dir characters can never contain a dot, so only the real filename matches. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14910 Test Plan: - build_folly: confirmed 'make build_folly' no longer writes .build_signature and that BUILD_SIG_DO_BUILD resolves empty for that goal. - rate_limiter_test: confirmed via the C preprocessor that x86_64 keeps the SANDCASTLE-only exemption while aarch64 selects the new GITHUB_ACTIONS branch. - ldb_test: reproduced the collapse with a poisoned temp suffix (produces {'8asst': ['0', 'mycol2']}) and confirmed the escaped-dot regex yields the correct 4-entry map. Reviewed By: mszeszko-meta Differential Revision: D110405164 Pulled By: pdillinger fbshipit-source-id: a9a73b7a717e726fdf79d7795f085f3dbede940e |
||
|
Anand Ananthabhotla
|
a0ad7887c9 |
Fix null scratch buffer submitted to async read on aligned direct-IO path (#14907)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14907 `RandomAccessFileReader::ReadAsync` took its "already aligned" fast path when the request offset and length were sector-aligned, forwarding the caller's `req` (including its `scratch` pointer) straight to the FileSystem. A null `scratch` trivially satisfied the alignment check (`uintptr_t(nullptr) & (alignment - 1) == 0`), so when a caller submitted an aligned request with `scratch == nullptr` and an `aligned_buf` out-parameter for the reader to allocate into, the null buffer was submitted to the async read. For direct IO with io_uring this becomes a null `iovec` base, which the kernel rejects with `EFAULT` (surfaced as `Req failed: Unknown error -14`). This is exactly how `IODispatcher` submits MultiScan async reads for plain direct IO (`scratch == nullptr`, `aligned_buf` provided), so the failure manifested intermittently as `db_stress` iterator divergence ("Iterator diverged from control iterator") in crash tests with `use_direct_reads=1 --use_multiscan=1 --multiscan_use_async_io=1`. It is intermittent because it only triggers when the coalesced read's offset and length both happen to be sector-aligned. Task T267030385. The fix excludes the aligned fast path when the caller provided no `scratch` but did provide an `aligned_buf`, forcing the allocating path so a valid backing buffer is always handed to the async read. This matches the existing behavior of the synchronous `Read`, which already guards its aligned fast path with `scratch != nullptr`. Reviewed By: xingbowang Differential Revision: D110361301 fbshipit-source-id: 3884f7ddbc746cf51b2566d74918e2fb97233dcb |
||
|
Peter Dillinger
|
6cceef9497 |
Bug fixes: surface embedded-blob resolution errors (#14906)
Summary:
Experimental embedded ("same-file") blob SST support (https://github.com/facebook/rocksdb/issues/14851) overloads blob file number 0 as both kInvalidBlobFileNumber and the same-file sentinel kCurrentFileBlobIndexFileNumber. Same-file references must be resolved by EmbeddedBlobResolvingIterator before they reach any generic file-metadata or integrated-BlobDB path, all of which reject file number 0 as invalid.
When resolving a same-file blob hit an error (e.g. a db_stress-injected blob-region read fault), EmbeddedBlobResolvingIterator::value() fell back to returning the RAW, unresolved same-file value, surfacing the error only via status()/Valid(). Compaction consumed the raw value before consulting status, leaking an unresolved same-file reference into the output stream. Depending on the value type this produced two different crash-test failures, both from this one root cause:
* T277566778 -- wide-column entity variant. The raw same-file entity (kTypeWideColumnEntity) reached FileMetaData::UpdateBoundaries, whose blob-ref scan (correctly) rejected file number 0: Flush failed: Corruption: Invalid blob file number (also observed from a background compaction). The tripwire fired on the leak, so the real injected read error was masked behind a misleading corruption status.
* T277310719 -- whole-value BlobIndex variant, and more dangerous because it escapes that tripwire. ResolveKeyType() rewrites the key type kTypeBlobIndex -> kTypeValue with no I/O (so it always succeeds); on the masked error the emitted entry is therefore {kTypeValue, raw BlobIndex bytes}. UpdateBoundaries only scans kTypeBlobIndex/entity types, so it does NOT reject this record: the corruption is silently written to the compaction output and persists. A later point lookup reads those raw BlobIndex bytes back as the value, which db_stress detects as db_stress: expected_value.cc:102: Assertion `ExpectedValue::IsValueBaseValid(value_base)' failed from TestGet. In the captured crash the aborting db_stress process had injected no faults itself -- it read corruption persisted by an earlier faults-on run (blackbox reuses the DB directory), confirming the persistence route.
Fix: EmbeddedBlobResolvingIterator now resolves eagerly for callers that do not opt into unprepared values (allow_unprepared_value=false, e.g. compaction), via a new EagerEmbeddedBlobResolvingIterator (the caller is selected in BlockBasedTable::NewIterator). Eager callers resolve the value during positioning, so a resolution error (blob-region I/O or corruption) is observable through status()/Valid() BEFORE value() is consumed, and value() never exposes an unresolved same-file BlobIndex. Lazy callers (allow_unprepared_value=true, user iteration) keep value laziness but must honor PrepareValue()'s result. This keeps the "callers never see an unresolved same-file blob" invariant structural, even on the error path. Using a template for EmbeddedBlobResolvingIterator minimizes unnecessary overheads.
Also hardens and documents the integrity tripwires that caught the leak, so they are not "fixed" by weakening them -- doing so would mask real corruption and could persist unresolvable same-file references into ordinary (non-embedded) SSTs, turning a transient error into permanent data loss: FileMetaData::UpdateBoundaries (write/output path), Version::GetBlob and Version::MultiGetBlob (integrated-BlobDB read path), plus the contract note in blob_constants.h.
Adds a test-only sync point
"BlockBasedTable::MaybeResolveEmbeddedValue:InjectError" (compiled out under NDEBUG) to simulate a blob-region resolution fault deterministically.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14906
Test Plan:
New unit tests in db_blob_index_test, both of which fail without the fix and pass with it:
* DBBlobIndexTest.EmbeddedBlobResolveErrorDuringCompactionNotMasked -- entity variant. Without the fix CompactRange fails with the masked "Invalid blob file number"; with the fix it fails with the injected IOError.
* DBBlobIndexTest.EmbeddedBlobResolveErrorWholeValueDuringCompactionNotMasked -- whole-value variant. Without the fix CompactRange SUCCEEDS (silently masking the error and persisting {kTypeValue, raw BlobIndex}); with the fix it fails with the injected IOError.
Confirmed the tests exercise the real root cause by temporarily neutralizing the eager resolution (MaybeEagerlyMaterialize): both tests then fail in their respective pre-fix modes (entity -> "Invalid blob file number"; whole-value -> compaction succeeds), then pass again once restored.
End-to-end: the db_stress reproducer for T277566778 (blackbox crash test with embedded-blob ingestion and fault injection) reproduced the "Invalid blob file number" crash before the fix and no longer reproduces with it.
Reviewed By: anand1976
Differential Revision: D110361228
Pulled By: pdillinger
fbshipit-source-id: 1faaa9a9ead726f17b611ff150418fa59ed75fd4
|
||
|
generatedunixname1395027625275998
|
1f7a286413 |
Fix HyperClockCache naive Lookup fast-path chain indexing to skip full fallback (#14901)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14901 Reviewed By: anand1976 Differential Revision: D109906106 fbshipit-source-id: 660a47f51250415286af4f9ddcb0c40410ef04cd |
||
|
Xingbo Wang
|
de7b8e8f7d |
Add RocksDB flush reason and atomic flush request metrics (#14898)
Summary:
This PR adds flush observability for debugging write stalls and flush bottlenecks.
Changes include:
- Add flush reason tickers for physical flush jobs:
- rocksdb.flush.reason.write_buffer_full
- rocksdb.flush.reason.write_buffer_manager
- rocksdb.flush.reason.memtable_max_range_deletions
- Add flush memtable size histograms:
- rocksdb.flush.memtable.memory.bytes
- rocksdb.flush.memtable.total.data.size.bytes
- rocksdb.flush.write_buffer_full.memtable.memory.bytes
- rocksdb.flush.write_buffer_manager.memtable.memory.bytes
- Add FlushReason::kMemtableMaxRangeDeletions and propagate it from memtables when memtable_max_range_deletions triggers a flush.
- Preserve atomic flush as a single request-level reason. For atomic flush, the reason is derived only from CFs that scheduled the flush, not every CF selected into the atomic flush cut.
- Add atomic flush request-level counters:
- rocksdb.atomic_flush.request.reason.write_buffer_full
- rocksdb.atomic_flush.request.reason.write_buffer_manager
- rocksdb.atomic_flush.request.reason.memtable_max_range_deletions
- rocksdb.atomic_flush.request.reason.other
- Keep existing per-CF flush job reason counters unchanged. Under atomic flush, rocksdb.flush.reason.* still counts physical CF flush jobs, while rocksdb.atomic_flush.request.reason.* counts one logical atomic flush request.
- Wire the new flush reason, ticker, and histogram types through Java and JNI bindings.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14898
Test Plan: CI
Reviewed By: anand1976
Differential Revision: D110214337
Pulled By: xingbowang
fbshipit-source-id: 4a445c4175d2e09ff5edbe66fde81fa00bb9e8da
|
||
|
Peter Dillinger
|
741539da9b |
Fix backward iteration over embedded blob SSTs (#14902)
Summary: D110140135/https://github.com/facebook/rocksdb/issues/14896 worked around a db_stress failure by disabling test_backward_scan whenever embedded-blob SSTs are ingested. The real limitation was in EmbeddedBlobResolvingIterator: DBIter requires the underlying iterator's value to be pinned for backward iteration (Prev/SeekForPrev), and rebuilt wide-column entity values (with same-file blob columns) were stored in an iterator-owned std::string that was cleared on every reposition, so IsValuePinned() returned false and DBIter returned NotSupported (or tripped the IsValuePinned() assertion in FindValueForCurrentKeyUsingSeek). Whole-value same-file blobs were already pinned and worked. This change makes the rebuilt wide-column value pinnable like a whole-value blob: it is moved into a heap buffer and pinned into resolved_pinned_value_, whose cleanup ResetState() hands to the PinnedIteratorsManager across repositioning so the value survives backward iteration. The now-redundant resolved_value_ member is removed and value()/IsValuePinned() simplified. The Get/MultiGet resolution path (MaybeResolveEmbeddedValue) is unchanged. The db_stress test_backward_scan workaround is removed now that backward iteration over embedded blobs (including wide-column entities) is supported. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14902 Test Plan: Added DBBlobIndexTest.EmbeddedBlobBackwardIteration: ingests an embedded-blob SST with whole-value blobs plus a mixed wide-column entity (inline default column + embedded same-file blob column), then iterates backward via SeekToLast/Prev and SeekForPrev, verifying values and columns. Verified it fails before the fix (NotSupported at the entity) and passes after. Full db_blob_index_test and sst_file_reader_test suites pass. make format-auto clean. Direct db_stress run with ingest_external_file_with_embedded_blobs=1, use_put_entity_one_in=3, test_backward_scan=1, iterpercent>0 ran with no divergence/NotSupported/ assert. Reviewed By: anand1976 Differential Revision: D110271453 Pulled By: pdillinger fbshipit-source-id: 501a50d3c97bac9a217b604c43c8748abbb563c9 |
||
|
Hui Xiao
|
7e2223f4ab |
CPU corruption injector: randomize non-preset db_stress flags (#14867)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14867 Adds an opt-in --randomize_stress_flags mode to the CPU corruption injection runner. The fixed DB_STRESS_PRESET keeps an injected corruption isolated and observable, but every run then exercises the same stress test flags (default values for non DB_STRESS_PRESET flags). To widen what a runner covers, we want every other db_stress knob to vary run to run. Therefore this PR makes each run's db_stress flags are a fresh db_crashtest.py roll (its full blackbox parameter space) with DB_STRESS_PRESET pinned on top. A small CLOSURE gate set stops db_crashtest's finalize_and_sanitize() from un-pinning preset flags, and an assertion fails the run fast if any preset flag is changed. This is considered as a work-around until we can refactor the flag generation of db_crashtest.py into a isolated function to take a set of flags to honor. This PR also adds some randomization into `max_keys` and local vs remote compaction as they are subjected to some special constraint unique to the cpu corruption injection framework. The per-run roll is seeded by the run's seed, so randomized runs' flags are reproducible with --seed (though the actual db stress test behavior may still have some difference under the same flags). **Test plan:** Functional test - build_runs(..., randomize=True) produces a full randomized flag set (~286 flags vs ~49 for the fixed preset) and the preset survives the db_crashtest roll (the _assert_preset_survived check never fires). - randomized runs are reproducible: same base_seed -> same flags; run i seeded base_seed+i. Integration test ``` for op in write flush compaction; do python3 tools/cpu_corruption_injector/runner.py --op $op --runs 30 --randomize_stress_flags --parallel 8 --stress_cmd "$BIN" --report_dir /tmp/icc-rand/$op done ``` - every run still carries the pinned DB_STRESS_PRESET values (preset-survived assertion never fires across the campaign). - the workload genuinely varies run to run (e.g. max_keys, different compaction_style / memtablerep / *_one_in values across runs), confirming randomization is active. - SDC still appears across write / flush / compaction -- randomization widens coverage without losing the SDC signal; NO_INJECTION and ERROR stay ~0. **Runs' Outcomes (`summary.json`):** | SDC | CORRUPTION | CRASH | NO_EFFECT | NO_INJECTION | ERROR | | --- | --- | --- | --- | --- | --- | | 2 | 7 | 7 | 43 | 1 | 0 | Local vs remote compaction. With `--randomize_stress_flags` a compaction run may flip to the remote compaction service (`remote_compaction_worker_threads=1`), so the injector breaks on `CompactionServiceCompactionJob::Run` instead of the local `CompactionJob::Run`. Splitting the runs by injection entry_fn: | where | runs | SDC | CORRUPTION | CRASH | NO_EFFECT | NO_INJECTION | | --- | --- | --- | --- | --- | --- | --- | | local | 25 | 1 | 1 | 3 | 20 | 0 | | remote | 35 | 1 | 6 | 4 | 23 | 1 | Reviewed By: pdillinger Differential Revision: D108551605 fbshipit-source-id: d3dc5687e70ab57779ad6e4f87cac7ed0086da34 |
||
|
Hui Xiao
|
eddfee8f56 |
CPU corruption injector: runner + db_stress preset flags (#14866)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14866 Orchestration layer of the CPU corruption injector -- the glue between detection (https://github.com/facebook/rocksdb/pull/14852) and injection (https://github.com/facebook/rocksdb/pull/14858 ). One CPU-corruption injection says little on its own. What matters is the OUTCOME DISTRIBUTION over many injections -- how often a corruption is silently absorbed (NO_EFFECT), crashes the process (CRASH), is caught by an integrity check (CORRUPTION), or more importantly slips through as a silent data corruption (SDC) and the paths frequently leading to those outcomes. A trustworthy distribution needs a (somewhat) repeatable and reproducible harness as well as a db_stress configuration in which an injected corruption is both reachable and attributable to the chosen stress test op (i.e, write, foreground compaction or flush). Therefore this PR implements a runner that can launch N independent runs for a chosen op type (i.e, write, foreground compaction or flush). Each run picks where to inject, runs db_stress under gdb via the `injector,py` (https://github.com/facebook/rocksdb/pull/14858), and is classified into one outcome bucket (https://github.com/facebook/rocksdb/pull/14852). The runner has DB_STRESS_PRESET -- the pinned db_stress config that isolates a single injected corruption (single-threaded, integrity checks on, other fault injection off, auto-compaction off). The runner also does gdb preflight that fails fast when the build or gdb cannot support injection because, for example, the hard-coded `target_fn` has changed its name, provides parallel launching of many runs and one summary.json per campaign (the outcome distribution plus each run's record). The whole run set is reproducible from one logged base_seed (run i uses base_seed + i). **Test plan:** Build: `make DEBUG_LEVEL=0 EXTRA_CXXFLAGS="-g -fno-inline" db_stress` # 1. Preflight (`verify_injection_site`) catches a build that can't support injection Before doing any work, the runner has gdb confirm — on this exact binary — that every injection-site function resolves and gdb can read its source line. A good build logs: ``` INFO gdb check OK for op=compaction: rocksdb::CompactionJob::Run, rocksdb::CompactionIterator::NextFromInput, rocksdb::BlockBuilder::Add ``` A build that cannot support injection (functions renamed, fully inlined, or absent) fails fast with exit 2 before any run — forced here by pointing `--stress_cmd` at a non-db_stress binary: ``` $ python3 tools/cpu_corruption_injector/runner.py --op compaction --runs 1 --stress_cmd /bin/ls --report_dir /tmp/icc/preflight_demo ERROR gdb could not set a breakpoint on these functions in ls (renamed, fully inlined, or not in this build?): rocksdb::CompactionJob::Run, rocksdb::CompactionIterator::NextFromInput, rocksdb::BlockBuilder::Add Function "rocksdb::CompactionJob::Run" not defined. Function "rocksdb::CompactionIterator::NextFromInput" not defined. # exit code 2 ``` So a broken/inlined build is rejected up front instead of silently producing `NO_INJECTION` runs. # 2. Compaction op -- 100 runs ``` $ python3 tools/cpu_corruption_injector/runner.py --op compaction --runs 100 --stress_cmd /data/users/huixiao/rocksdb/db_stress --report_dir /tmp/icc/preflight_demo ``` **Runs' outcomes (`summary.json`):** | SDC | CORRUPTION | CRASH | NO_EFFECT | NO_INJECTION | ERROR | | --- | --- | --- | --- | --- | --- | | 9 | 5 | 6 | 79 | 1 | 0 | **Spread:** - target_fn x outcome: `NextFromInput` {SDC 9, CORRUPTION 3, CRASH 2, NO_EFFECT 34, NO_INJECTION 1}; `BlockBuilder::Add` {CORRUPTION 2, CRASH 4, NO_EFFECT 45}. - corruption_type x outcome: `bit_flip` {SDC 8, CORRUPTION 3, CRASH 6, NO_EFFECT 32}; `flag_flip` {SDC 1, CORRUPTION 2, NO_EFFECT 42}; `lane_bit_flip` {NO_EFFECT 5}. **Analysis:** all 9 SDCs land on the read/iterate path (`NextFromInput`); corrupting the output writer (`BlockBuilder::Add`) never produced an SDC (its blocks are checksummed -- corruption there is caught or inert). The 5 detected `CORRUPTION`s are compaction's key-order and record-count cross-checks firing (both CompactRange and CompactFiles origins appear), correctly bucketed by the fix. ### A representative compaction SDC: `run_00000` What we corrupted (`inject.json`): ```json {"op":"compaction","op_index":17,"entry_fn":"rocksdb::CompactionJob::Run","target_fn":"rocksdb::CompactionIterator::NextFromInput","injection_result":"injected","db_stress_crash_signal":null, "corruptions":[{"instruction":"mov %rdx,0x160(%r12)","register":"rdx","corruption_type":"bit_flip","before":"0x10","after":"0x18", "details":{"source":"rocksdb::CompactionIterator::NextFromInput @ db/compaction/compaction_iterator.cc:719"}}]} ``` The recorded silent corruption (`data_corruption.<tid>.json`): ```json {"kind":"wrong-value","cf":0,"key":70,"value_from_db":"010000000504070609080B0A0D0C0F0E070F105E78787878","value_from_expected":"010000000504070609080B0A0D0C0F0E","op_status":"Get: OK"} ``` **Walkthrough:** a single bit flip on `rdx` (`0x10 -> 0x18`, 16 -> 24) at `CompactionIterator::NextFromInput` (`compaction_iterator.cc:719`) -- `rdx` holds the value LENGTH stored into the iterator's record (offset `0x160`). The length reads 24 instead of 16, so compaction copies a value 8 bytes too long into the output SST, absorbing adjacent bytes. The internal key is untouched (`ParseInternalKey` passes); the over-long value is the single Slice fed to both the paranoid validator and the SST builder, so the file is self-consistent and every checksum agrees. On read-back `Get(key=70)` returns OK with the wrong bytes -- `value_from_db` is the expected value (`...0F0E`) **plus 8 trailing bytes** (`070F105E78787878`). Silent: read OK, all checks pass, the value visibly grew. `classify()` routes `kind=wrong-value` to the SDC bucket. ### A representative compaction CORRUPTION (detected): `run_00007` What we corrupted (`inject.json`): ```json {"op":"compaction","op_index":41,"entry_fn":"rocksdb::CompactionJob::Run","target_fn":"rocksdb::CompactionIterator::NextFromInput","injection_result":"injected","db_stress_crash_signal":"SIGABRT", "corruptions":[{"instruction":"mov (%rbx),%rdi","register":"rdi","corruption_type":"bit_flip","before":"0x7fffeee8c1c0","after":"0x7fffeee8c1c2", "details":{"source":"rocksdb::IterKey::SetKeyImpl @ ./db/dbformat.h:941","call_chain":["rocksdb::IterKey::SetKeyImpl @ ./db/dbformat.h:941","rocksdb::CompactionIterator::NextFromInput @ db/compaction/compaction_iterator.cc:781"]}}]} ``` The recorded detection (`data_corruption.<tid>.json`): ```json {"kind":"detected-corruption","cf":-1,"key":-1,"value_from_db":"","value_from_expected":"","op_status":"compactfiles: Corruption: Compaction sees out-of-order keys."} ``` **Walkthrough:** a bit flip on `rdi` (`...c1c0 -> ...c1c2`, a key pointer) at `IterKey::SetKeyImpl` (`dbformat.h:941`), reached from `NextFromInput`, mis-sets the iterator's key so the next emitted key is out of order. Compaction's key-order check catches it and returns `compactfiles: Corruption: Compaction sees out-of-order keys`. The op then takes `SIGABRT`, but `classify()` reads the recorded `data_corruption` result before the crash signal, so the run is correctly bucketed `CORRUPTION` (the bucketization fix; pre-fix this surfaced as `CRASH`). `classify()` routes `kind=detected-corruption` to the `CORRUPTION` bucket. # 3. Flush op -- 100 runs ``` $ python3 tools/cpu_corruption_injector/runner.py --op flush --runs 100 --stress_cmd /data/users/huixiao/rocksdb/db_stress --report_dir /tmp/icc/preflight_demo ``` **Runs' outcomes (`summary.json`):** | SDC | CORRUPTION | CRASH | NO_EFFECT | NO_INJECTION | ERROR | | --- | --- | --- | --- | --- | --- | | 2 | 12 | 11 | 74 | 1 | 0 | **Spread:** - target_fn x outcome: `BlockBuilder::Add` {SDC 1, CORRUPTION 5, CRASH 5, NO_EFFECT 49}; `NextFromInput` {SDC 1, CORRUPTION 7, CRASH 6, NO_EFFECT 25, NO_INJECTION 1}. - corruption_type x outcome: `bit_flip` {SDC 2, CORRUPTION 9, CRASH 9, NO_EFFECT 32}; `flag_flip` {CORRUPTION 3, CRASH 2, NO_EFFECT 42}. **Analysis:** flush mirrors compaction's mechanisms (shared iterator/builder). The 2 SDCs are a value/key-pointer corruption that slips past the checksums; the 12 corruptions are caught by the flush-time key-order / key-size integrity checks. ### A representative flush SDC: `run_00027` What we corrupted (`inject.json`): ```json {"op":"flush","op_index":16,"entry_fn":"rocksdb::FlushJob::Run","target_fn":"rocksdb::BlockBuilder::Add","injection_result":"injected","db_stress_crash_signal":null, "corruptions":[{"instruction":"mov (%rdi),%rax","register":"rax","corruption_type":"bit_flip","before":"0x7fffef059400","after":"0x7fffef059440", "details":{"source":"rocksdb::Slice::data @ ./include/rocksdb/slice.h:58","call_chain":["rocksdb::Slice::data @ ./include/rocksdb/slice.h:58","rocksdb::BlockBuilder::AddWithLastKeyImpl @ table/block_based/block_builder.cc:351"]}}]} ``` The recorded silent corruption (`data_corruption.<tid>.json`): ```json {"kind":"lost","cf":0,"key":763,"value_from_db":"","value_from_expected":"010000000504070609080B0A0D0C0F0E","op_status":"Get: NotFound"} ``` **Walkthrough:** a bit flip on `rax` (a key/value data pointer, `...9400 -> ...9440`) at `Slice::data` (`slice.h:58`), reached from `BlockBuilder::AddWithLastKeyImpl` while the flush builds the output block, makes the builder read key bytes from the wrong address, so key 763's entry is written wrong and the key is dropped from the flushed SST. On read-back `Get(key=763)` returns `NotFound` for a committed key -- silent. `classify()` routes `kind=lost` to the SDC bucket. ### A representative flush CORRUPTION (detected): `run_00047` What we corrupted (`inject.json`): ```json {"op":"flush","op_index":7,"entry_fn":"rocksdb::FlushJob::Run","target_fn":"rocksdb::CompactionIterator::NextFromInput","injection_result":"injected","db_stress_crash_signal":"SIGABRT", "corruptions":[{"instruction":"cmp $0x7,%rax","register":"eflags","corruption_type":"flag_flip","before":"0x216","after":"0x256", "details":{"source":"rocksdb::ParseInternalKey @ ./db/dbformat.h:523","call_chain":["rocksdb::ParseInternalKey @ ./db/dbformat.h:523","rocksdb::CompactionIterator::NextFromInput @ db/compaction/compaction_iterator.cc:731"]}}]} ``` The recorded detection (`data_corruption.<tid>.json`): ```json {"kind":"detected-corruption","cf":-1,"key":-1,"value_from_db":"","value_from_expected":"","op_status":"flush: Corruption: Corrupted Key: Internal Key too small. Size=16. "} ``` **Walkthrough:** a flag flip (`eflags 0x216 -> 0x256`) on a `cmp $0x7,%rax` branch in `ParseInternalKey` (`dbformat.h:523`), reached from `NextFromInput`, makes the parser mis-judge the internal-key size, so the flush emits a malformed key and the key-size integrity check returns `flush: Corruption: Corrupted Key: Internal Key too small`. The op takes `SIGABRT`; `classify()` reads the recorded `data_corruption` before the signal and buckets `CORRUPTION` (bucketization fix). `classify()` routes `kind=detected-corruption` to the `CORRUPTION` bucket. # 4. Write op (`MemTable::Add`) -- two key spaces ``` $ python3 tools/cpu_corruption_injector/runner.py --op write --runs 100 --stress_cmd /data/users/huixiao/rocksdb/db_stress --report_dir /tmp/icc/preflight_demo ``` A write injection corrupts a single `MemTable::Add` (a Put/Delete/DeleteRange). The corruption is reachable and attributable, but whether it surfaces as a *silent* write SDC depends heavily on the key space. A silent write SDC needs the affected/mispositioned key to have other live versions to fall through to -- which only happens in a dense, multi-version memtable. We therefore run two write campaigns: the default `max_key=1000`, then a small `max_key=8`. The contrast is what motivates randomizing `max_key` (see PR https://github.com/facebook/rocksdb/pull/14867 for `--randomize_stress_flags`). ### 4a. Default `max_key=1000` -- 100 runs (no silent write SDC) **Runs' outcomes (`summary.json`):** | SDC | CORRUPTION | CRASH | NO_EFFECT | NO_INJECTION | ERROR | | --- | --- | --- | --- | --- | --- | | 0 | 31 | 13 | 56 | 0 | 0 | With a 1000-key space almost every write touches a distinct key, so a corrupted entry has no older live version to mask it: a value/key byte flip is caught at write by the per-key checksum (`VerifyEncodedEntry` -> `CORRUPTION`), and a structural flip tends to crash (`CRASH`) rather than silently mis-read. `ERROR=0`, `NO_INJECTION=0`. No write op silently corrupted data -- every reachable corruption was caught or crashed. ### 4b. Small `max_key=8` -- 100 runs (surfaces 2 silent write SDCs) **Runs' outcomes (`summary.json`):** | SDC | CORRUPTION | CRASH | NO_EFFECT | NO_INJECTION | ERROR | | --- | --- | --- | --- | --- | --- | | 2 | 33 | 8 | 57 | 0 | 0 | Shrinking the key space makes each key hold ~125 versions (`ops_per_thread` / `max_key`), so a misplaced entry can fall through to an older version of the *same* key and be returned silently -- the per-key checksum (bytes intact) and on-seek verify cannot see a pure link-position error. ### A representative write SDC: `run_00028` (skiplist misposition -> silent stale read, flush catches) What we corrupted (`inject.json`): ```json {"op":"write","op_index":317,"entry_fn":"rocksdb::MemTable::Add","target_fn":"rocksdb::MemTable::Add","injection_result":"injected","db_stress_crash_signal":null, "corruptions":[{"instruction":"cmp %rbx,-0xb8(%rbp)","register":"eflags","corruption_type":"flag_flip","before":"0x216","after":"0x217", "details":{"source":"rocksdb::MemTable::Add @ db/memtable.cc:1319", "call_chain":["rocksdb::MemTable::Add @ db/memtable.cc:1319"]}}]} ``` The recorded silent corruption (`data_corruption.<tid>.json`): ```json {"kind":"resurrected","cf":0,"key":1,"value_from_db":"110000001514171619181B1A1D1C1F1E0100030205040706","value_from_expected":"","op_status":"Get: OK"} ``` **Walkthrough:** a flag flip (CF, `eflags 0x216 -> 0x217`) on the `cmp` that produces `KeyIsAfterNode` inside `InlineSkipList::Insert` (`inlineskiplist.h:1253`; the `@ memtable.cc:1319` in the record is inlining line-drift) inverts the key comparison, so the Delete tombstone for key 1 is linked at the wrong position. The stored bytes and per-key checksum are intact, so neither the checksum nor on-seek verify sees anything wrong -- on read-back `Get(key=1)` returns OK with key 1's live value for a key that was Deleted (`kind=resurrected`, silent). A follow-up `Flush()` in unit test repro *does* catch it: the full-scan order check returns `Corruption: Out-of-order keys found in skiplist` -- caught only after the silent read, not during it. ### A representative write CORRUPTION (detected) `max_key=1000 or 8` : `run_00018` Where `run_00028`'s pure link-position error is invisible to the per-key checksum, this run shows a byte-level corruption that the checksum *catches* at write time. What we corrupted (`inject.json`): ```json {"op":"write","op_index":106,"entry_fn":"rocksdb::MemTable::Add","target_fn":"rocksdb::MemTable::Add","injection_result":"injected","db_stress_crash_signal":null, "corruptions":[{"instruction":"mov %rsi,(%rdi)","register":"rsi","corruption_type":"bit_flip","before":"0x7fffeec2a21c","after":"0x7fffeec2a25c", "details":{"source":"rocksdb::Slice::Slice @ ./include/rocksdb/slice.h:39", "call_chain":["rocksdb::Slice::Slice @ ./include/rocksdb/slice.h:39","rocksdb::GetVarint32 @ ./util/coding.h:280","rocksdb::MemTable::VerifyEncodedEntry @ db/memtable.cc:1102","rocksdb::MemTable::Add @ db/memtable.cc:1189"]}}]} ``` The recorded detection (`data_corruption.<tid>.json`): ```json {"kind":"detected-corruption","cf":-1,"key":-1,"value_from_db":"","value_from_expected":"","op_status":"put: Corruption: ProtectionInfo mismatch"} ``` **Walkthrough:** a bit flip on `rsi` (`0x7fffeec2a21c -> 0x7fffeec2a25c`) at `Slice::Slice` (`slice.h:39`) while `MemTable::Add` re-parses the just-encoded entry through `VerifyEncodedEntry` (`memtable.cc:1102`) corrupts the Slice the verifier reads, so the recomputed per-key protection info no longer matches and the put returns `Corruption: ProtectionInfo mismatch`. Reviewed By: pdillinger Differential Revision: D108367345 fbshipit-source-id: 250b069c8e9d63a2dc257f6de86f1d9040284dd6 |
||
|
Hui Xiao
|
3745f2e234 |
CPU corruption injector: gdb register flip into one db_stress op (#14858)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14858 This PR is the injection layer of the CPU corruption injector, runs inside gdb and randomly corrupts a register by bit flip in exactly one db_stress op (i.e, write, foreground compaction and flush) per stress test run. Detection layer is at db_stress (https://github.com/facebook/rocksdb/pull/14852); orchestration layer is coming up. __How one run works__ - The orchestration layer, coming up, randomly picks which stress test `op` instance (so corruption can land at different points in the LSM shape journey) and which `target_fn` of that `op` (so to cap instructions to step under a reasonable limit; `injector.py` in this PR randomly picks which instruction within the `target_fn` to inject (so corruption can land at different points of a `target_fn`). - Attach: gdb starts with injector.py's parameters passed via -iex and the db_stress command after --args, so db_stress runs unmodified. Example: ``` gdb --batch --nx \ -iex "py import sys; sys.argv=['injector.py','--op','write','--op_index','42','--entry_fn','rocksdb::MemTable::Add','--target_fn','rocksdb::MemTable::Add','--corruptions_per_op','1','--seed','7','--dir','<rundir>']" \ -x tools/cpu_corruption_injector/injector.py \ --args <db_stress> --threads=1 --verify_cpu_corruption_dir=<rundir> ... ``` - Navigate: The orchestration layer will pick op_index. `entry_fn` is called exactly once per stress test run's op so the op_index-th op is its op_index-th call. `injector_navigate.py` breaks on `entry_fn` and set a gdb ignore-count of op_index-1 to fast-forward to op_index-th one. It also breaks at the first `target_fn` within that `entry_fn`. - Warm up: `injector_critical_instruction.py` will choose "critical instruction" (those that move key/value bytes with general-purpose or vector registers or set a branch flag) uniformly within the chosen `target_fn` by the orchestration layer. In order to do that, it needs to approximate how many such instructions within the `target_fn`. Hence we have this warm-up phase. It single-steps the instruction within the first encoutering of `target_fn` to count and draw the critical instruction index, then corrupt that index at a later call. - Corrupt: on a later call of `target_fn`, `injector_critical_instruction.py` single-step to the m-th critical instruction and bit-flip the register through `injector_register_corruption.py`. The way to corrupt register depends on what instruction it is. If the current call of `target_fn`'s m-th instruction is not a critical instruction, we will try next `target_fn` till running out of `target_fn`. - Record: `injector_telemetry.py` provides telemetry to capture the corruption for later analysis. **Test plan:** 1. Isolated tests (real gdb-captured x/i fixtures): test_inject_critical_instruction 2. E2E test on navigation, inject, telemetry will be done in the later orchestration PR. Below is inject.json from such run ``` { "injection_result": "injected", "db_stress_crash_signal": null, "op": "write", "op_index": 279, "entry_fn": "rocksdb::MemTable::Add", "target_fn": "rocksdb::MemTable::Add", "critical_instruction_index": 37, "corruptions": [ { "instruction": "mov %rsi,0x8c8(%rbx)", "register": "rsi", "corruption_type": "bit_flip", "before": "0x7fffee4c64d8", "after": "0x7fffee4c64c8", "details": { "source": "rocksdb::Arena::AllocateAligned @ ./fbcode/internal_repo_rocksdb/repo/memory/arena.cc:135", "call_chain": [ "rocksdb::Arena::AllocateAligned @ ./fbcode/internal_repo_rocksdb/repo/memory/arena.cc:135", "rocksdb::ConcurrentArena::AllocateAligned(unsigned long, unsigned long, rocksdb::Logger*)::{lambda()#1}::operator()() const @ fbcode/internal_repo_rocksdb/repo/memory/concurrent_arena.h:65", "rocksdb::ConcurrentArena::AllocateImpl<rocksdb::ConcurrentArena::AllocateAligned(unsigned long, unsigned long, rocksdb::Logger*)::{lambda()#1}>(unsigned long, bool, rocksdb::ConcurrentArena::AllocateAligned(unsigned long, unsigned long, rocksdb::Logger*)::{lambda()#1} const&) @ fbcode/internal_repo_rocksdb/repo/memory/concurrent_arena.h:145", "rocksdb::ConcurrentArena::AllocateAligned @ fbcode/internal_repo_rocksdb/repo/memory/concurrent_arena.h:63", "rocksdb::InlineSkipList<rocksdb::MemTableRep::KeyComparator const&>::AllocateNode @ fbcode/internal_repo_rocksdb/repo/memtable/inlineskiplist.h:868", "rocksdb::InlineSkipList<rocksdb::MemTableRep::KeyComparator const&>::AllocateKey @ fbcode/internal_repo_rocksdb/repo/memtable/inlineskiplist.h:855", "rocksdb::(anonymous namespace)::SkipListRep::Allocate @ ./fbcode/internal_repo_rocksdb/repo/memtable/skiplistrep.cc:36", "rocksdb::MemTable::Add @ ./fbcode/internal_repo_rocksdb/repo/db/memtable.cc:1157" ] } } ], "ops_seen": 279, "critical_instructions_seen": 38 } ``` Reviewed By: pdillinger Differential Revision: D107999835 fbshipit-source-id: 0863715e5d596d69abff8b8e4d9a252fa8c24b6c |
||
|
Hui Xiao
|
0f493506d9 |
Verify CPU corruption after op via --verify_cpu_corruption_dir (#14852)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14852 Detection layer of the CPU corruption injector (https://github.com/facebook/rocksdb/pull/14858). With `--verify_cpu_corruption_dir=<dir>`, db_stress reads back the full keyspace after every write/manual flush/manual compaction op and compares it to the expected-values model, classifying any mismatch by `kind`: `lost` / `resurrected` / `wrong-value` (silent data corruption) or `detected-corruption` (a status/checksum-caught error). Each finding is written to `<dir>/data_corruption.<tid>.json` ({kind, cf, key, value_from_db, value_from_expected, op_status}) and routed through db_stress's standard `VerificationAbort` for a clean exit-1. A startup guard requires `--threads=1` and all fault injection off so the read-back is single-writer and the only corruption present is the injected one Bonus: a minor refactoring into the surrounding error handling code in these ops **Test plan:** 1.Startup guard rejects misconfiguration: ``` --threads=2 -> exit 1: "--verify_cpu_corruption_dir requires --threads=1" --read_fault_one_in=5 -> exit 1: "requires all fault injection off" ``` 2.No false positive (clean CORE preset run, no injection): ``` $ db_stress --verify_cpu_corruption_dir=<dir> --threads=1 (full protections, all *_fault_one_in=0) ... exit 0; no data_corruption.<tid>.json produced; "Verification successful" ``` 3.Write-path cpu corruption injection (coming up, e.g, gdb flips a register inside MemTable::Add), then the immediate post-op read-back catches it. Real `<dir>/data_corruption.<tid>.json`: silent data corruption -- write returned OK but the key is gone on read-back: ``` {"kind":"lost","cf":0,"key":9814,"value_from_db":"","value_from_expected":"010000000504070609080B0A0D0C0F0E","op_status":"Get: NotFound"} ``` detected corruption -- read-back Get returns Corruption via the memtable per-key checksum: ``` {"kind":"detected-corruption","cf":0,"key":139,"value_from_db":"","value_from_expected":"","op_status":"Get: Corruption: Corrupted memtable entry, per key-value checksum verification failed." ``` 4.See PR https://github.com/facebook/rocksdb/pull/14866 test plan's spread in the outcome for verification of detection Reviewed By: pdillinger Differential Revision: D107999834 fbshipit-source-id: 18decbf51dc56eec62b735136e2dfb4e1175b773 |
||
|
Anand Ananthabhotla
|
779c7b58d1 |
Disable db_stress backward scan with embedded blob SSTs (#14896)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14896 Replace the earlier mitigation that disabled test_backward_scan when separate_key_value_in_data_block is set (D109837793). That gate was incorrect: separate_key_value_in_data_block does not affect value pinning -- it was only coincidentally correlated with the failing crash-test runs (the crash test biases it on ~2/3 of the time). Root cause: embedded blob SSTs are read through EmbeddedBlobResolvingIterator, which resolves same-file blob references (and wide-column entities with same-file blob columns) into an iterator-owned buffer. Those resolved values are not pinnable, but DBIter requires pinned values for backward iteration (Prev/SeekForPrev), returning NotSupported (surfaced as "Iterator diverged from control iterator") or hitting the IsValuePinned() assertion in FindValueForCurrentKeyUsingSeek (db_iter.cc). Gate test_backward_scan on ingest_external_file_with_embedded_blobs instead. This is a stress-test mitigation only; the underlying limitation (backward iteration over unpinnable resolved values) should be fixed separately by making the resolved value pinnable via the PinnedIteratorsManager. ___ Differential Revision: D110140135 fbshipit-source-id: 5288dab67ab753cbba22806e09fb8fec1e6f5a01 |
||
|
Anand Ananthabhotla
|
ffb7788d45 |
Disable backward scan in stress test when separate_key_value_in_data_block is enabled (#14889)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14889 When `separate_key_value_in_data_block=true`, the data block iterator stores keys and values in separate sections. Under certain conditions during backward iteration (Prev/SeekForPrev), `DBIter::FindValueForCurrentKey` encounters entries whose values cannot be pinned, causing a `NotSupported` error: "Backward iteration not supported if underlying iterator's value cannot be pinned." This manifests in the crash test as `TestIterateAgainstExpected` failures when the random flag combination includes `separate_key_value_in_data_block=1` (added in 47cfeb656ff3) with `test_backward_scan=1` (the default). The fix automatically disables `test_backward_scan` when `separate_key_value_in_data_block` is enabled, matching the pattern used for other option incompatibilities in the stress tool. Reviewed By: xingbowang Differential Revision: D109837793 fbshipit-source-id: 04230e6c0af42e98fb82d0ddcc3d857419e879e3 |
||
|
Xingbo Wang
|
75c1ffd4e3 |
Add RocksDB rate limiter I/O usage metrics (#14884)
Summary: Add rate limiter statistics that make it easier to diagnose whether RocksDB background IO is being throttled by the GenericRateLimiter. New ticker counters: - rocksdb.rate.limiter.bytes.read/write: bytes granted by the limiter, split by operation type. - rocksdb.rate.limiter.requests.read/write: requests granted by the limiter, split by operation type. - rocksdb.rate.limiter.delayed.requests.read/write: requests that actually had to wait for a future refill because available tokens were exhausted. - rocksdb.rate.limiter.total.wait.micros.read/write: cumulative wait time for delayed requests. New histograms: - rocksdb.rate.limiter.wait.micros.read/write: per-request wait latency for delayed requests, enabling percentile debugging. GenericRateLimiter now overrides the OpType-aware Request() path so these stats preserve the configured limiter mode. The legacy three-argument Request() path is treated as write IO for compatibility. Existing NUMBER_RATE_LIMITER_DRAINS is left intact, but the new delayed request counters are the clearer signal for out-of-token throttling because one delayed request can observe multiple drain intervals. Also update the Java ticker and histogram mappings so Java consumers can access the new statistics. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14884 Test Plan: - make format-auto - make -j16 rate_limiter_test statistics_test - ./rate_limiter_test - ./statistics_test - make check-sources Reviewed By: anand1976 Differential Revision: D109694030 Pulled By: xingbowang fbshipit-source-id: e92756788ba629f25655958c7beb91099184cda4 |
||
|
Peter Dillinger
|
f6fabdb64f |
Improve build and test experience for Make builds (#14883)
Summary: Local Make builds silently reuse object files even when build parameters (DEBUG_LEVEL, sanitizers, ASSERT_STATUS_CHECKED, RTTI, etc.) change, since all object files share the same paths. This leads to confusing linker errors, sanitizer false negatives, ODR violations, and "phantom" bugs that are easy to miss -- a pitfall for humans and especially for AI agents driving builds non-interactively. This change improves the Make experience on three fronts: 1. Build-parameter change detection in the Makefile. After flags are fully resolved, we hash the effective compile/link inputs (CC|CXX|CFLAGS|CXXFLAGS|LDFLAGS|EXEC_LDFLAGS) into a per-OBJ_DIR .build_signature stamp and compare against the previous build: - Default: stop with a clear error when parameters changed. This is optimized for the expert who might not intend a full rebuild, and for avoiding CI inefficiency. - AUTO_CLEAN=1: automatically run clean-rocks and proceed. - ALLOW_BUILD_PARAMETER_CHANGE=1: skip the check (e.g. intentionally mixing DEBUG_LEVEL=1 and DEBUG_LEVEL=2 objects). The check is skipped for dry runs (-n) and for non-building / self-cleaning targets (clean*, format, check-*, tags, gen-pc, check-progress, watch-log, release, coverage, the asan_*/ubsan_* variants, etc.). make_config.mk is removed only by the top-level `clean` target, not by clean-rocks, so the auto-clean does not delete the make_config.mk already included by the running make. The signature is removed last in clean-rocks so an interrupted clean keeps change detection meaningful. 3. New build_tools/rockstest.sh and build_tools/rocksptest.sh helpers that build and run unit tests in one step. They set AUTO_CLEAN=1 and build with -j<NCORES> (computed like the Makefile). rocksptest.sh runs one or more binaries under the checked-in gtest-parallel (sharing one parallel worker pool); rockstest.sh runs a single binary directly for a small number of cases. Both reject a leading-option first argument; `rockstest.sh install` writes ~/bin/rockstest and ~/bin/rocksptest shims that defer to the in-tree scripts (with a friendly message when run outside a source root). This is a big win for AI-agent workflows: one command instead of a separate `make` then test-run invocation, removing the overhead of monitoring two long commands, while avoiding serial builds and serial test execution. 4. CLAUDE.md guidance updated to recommend AUTO_CLEAN=1 for manual make invocations and the rocks(p)test.sh helpers for running tests Although there might be drive to consolidate around the BUCK build for internal use, I'll note that last I checked it was around ~20 minutes to run all the unit tests under buck and ~2 minutes under `make check`. Bonus: the first revision of this had copyright/license mistakes, and this change corrects similar errors elsewhere, with CLAUDE.md advice updated. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14883 Test Plan: Manual verification of the Makefile change detection: - Confirmed the resolved-flags hash is deterministic across runs for the same goal, and differs between configs (e.g. default/shared vs static_lib, and with/without ASSERT_STATUS_CHECKED). - Unchanged parameters: rebuild does not error. - Changed parameters: errors by default; AUTO_CLEAN=1 runs clean-rocks and proceeds; ALLOW_BUILD_PARAMETER_CHANGE=1 bypasses the check. - Dry runs (make -n) and exempt targets (e.g. `make check-progress`, list_all_tests, gen-pc) do not trip the check and leave the stamp untouched. - Reproduced the ASSERT_STATUS_CHECKED=1 auto-clean path end to end and confirmed it no longer fails with "No rule to make target 'make_config.mk'"; verified `make clean` removes make_config.mk while `clean-rocks` does not, and that the signature is deleted as the final clean-rocks step. Manual verification of the helper scripts: - shellcheck-clean and `bash -n` pass for both scripts. - Usage/guard behavior: missing argument and leading-option first argument are rejected with a clear message. - `rockstest.sh install` (validated against a temp HOME) generates both shims; running a shim outside a source root prints "(Not in a rocksdb source root directory?)" and exits non-zero. - rocksptest.sh argument splitting verified for single binary, multiple binaries, and binaries followed by gtest-parallel/test args. - .gitignore patterns verified with `git check-ignore` for .build_signature (root and jl/jls) and build_tools/__pycache__/. Reviewed By: xingbowang Differential Revision: D109713512 Pulled By: pdillinger fbshipit-source-id: a801d29afa91f53a2dce717db9fe5e5d485cc154 |
||
|
Anand Ananthabhotla
|
8fae7ff39d |
Fix use-after-free in EmbeddedBlobResolvingIterator when key() called before value()
Summary: ### Embedded Blob Resolving Iterator Fix #### Use-After-Free Bug Fix This diff fixes a use-after-free bug in the `EmbeddedBlobResolvingIterator` class. The bug occurred when the `key()` method was called before the `value()` method, causing the `resolved_internal_key_` buffer to be freed prematurely. #### Changes The fix involves modifying the `EmbeddedBlobResolvingIterator` class to ensure that the `resolved_key` is not moved into `resolved_internal_key_` if it has already been resolved (`key_resolved_` flag is set to `true`). This prevents the `resolved_internal_key_` buffer from being overwritten and freed when `MaterializeValue()` is called. Reviewed By: xingbowang Differential Revision: D109796428 fbshipit-source-id: d89fb36428ba62e8d0be53c2acfa3cdb9bda76f4 |
||
|
Jay Huh
|
43d60bfe9e |
Revert reverse-comparator handling in range tree lock manager (#14887)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14887 This reverts two changes to the range-lock endpoint comparator and restores the prior, direction-agnostic behavior: - https://github.com/facebook/rocksdb/pull/14831 ("Fix range lock manager crash with reverse comparator"), which made RangeTreeLockManager::CompareDbtEndpoints reverse-aware, and - https://github.com/facebook/rocksdb/pull/14880 (a follow-up that only corrected the equal-length / point-range branch). Why: #14831 taught the endpoint comparator about reverse-ordered column families by flipping the suffix tie-break based on comparator direction. However, callers that use range locking on a reverse-ordered CF already flip the start/end endpoints themselves before calling lock_range() -- for example, MyRocks does this in ha_rocksdb::set_range_lock() (see its "RangeFlagsShouldBeFlippedForRevCF" comment). The two corrections stack and produce left > right again, which: (a) trips paranoid_invariant(compare(left, right) <= 0) in toku::locktree::try_acquire_lock() and aborts on equality/point locks, and (b) for range scans, mis-orders the endpoints and produces an incorrect lock range, leading to silent correctness issues under concurrency. #14880 only addressed (a) (the equal-length / point-range branch), not (b), so it was an incomplete fix at the wrong layer. Decision: keep CompareDbtEndpoints direction-agnostic and make it an explicit caller requirement to pass a flipped (swapped start/end, with negated infimum/supremum flags) range for range locking to work on a reverse-ordered column family. A function-level comment documenting this contract is added so reverse-awareness is not reintroduced here. Removes the RangeLockWithReverseComparator unit test added by #14831 (it asserts the reverted direction-flipped ordering). Keeps a regression test that a point range lock [infimum(K), supremum(K)] works on a reverse-comparator column family. Reviewed By: xingbowang Differential Revision: D109713989 fbshipit-source-id: 4ea0999c92645e1adc9cd1114d924cad142f2001 |
||
|
Peter Dillinger
|
8c0790bdf8 |
Refresh platform010 build dependencies and remove TBB (#14882)
Summary: Refresh the fbcode platform010 toolchain and library pins to current third-party2 versions, and remove the now-unused Intel TBB dependency. Toolchain / deps: - Fix update_dependencies.sh: the gcc and binutils trees moved from centos8-native to centos9-native, so the old paths resolved to nothing and a re-run emitted empty GCC_BASE/BINUTILS_BASE. Point them at centos9-native. - Bump clang 15 -> 21; binutils 2.37 -> 2.43; libunwind 1.4 -> 1.8; valgrind 3.19 -> 3.22; plus hash-only refreshes for the LATEST-tracked libs. Regenerate dependencies_platform010.sh. - Keep GCC pinned at 11.x. The only newer GCC in third-party2 (13.x) is built for glibc >= 2.35 (its libgcc_s needs _dl_find_object@GLIBC_2.35), but platform010 ships glibc 2.34, so GCC 13 will not link/run here. - zlib stays 1.2.8 (the only version with an x86_64 platform010 build). - Document why GCC/libgcc/glibc/zlib remain pinned in update_dependencies.sh. Remove TBB (no longer used) from the build system: the get_lib_base entry, both fbcode_config*.sh, build_detect_platform detection, the CMake WITH_TBB option / find_dependency, and cmake/modules/FindTBB.cmake. Dockerfiles and the HISTORY.md changelog are left untouched. (TBB was used by the old clock cache, long ago removed.) Although this change was originally motivated by upgrading gcc for its libasan not to hit process lifetime thread limits, upgrading gcc proved impractical under platform010. Bonus: fix USBAN+gcc build by making GetParam() valid by the time it is called in several test class constructors. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14882 Test Plan: a variety of local builds (gcc, clang; various sanitizers) using fbcode tooling No production code changes Reviewed By: xingbowang Differential Revision: D109631787 Pulled By: pdillinger fbshipit-source-id: 9c466c59b039c2a67ee0318c0ccbac02e19f537b |
||
|
Peter Dillinger
|
a004c2d850 |
Add experimental embedded blob SST support (#14851)
Summary:
Add EXPERIMENTAL embedded blob SST support for SstFileWriter through
OpenWithEmbeddedBlobs(). Eligible large values are written as same-file blob
records inline in a block-based SST as values are added (interleaved with data
blocks), while table entries store same-file BlobIndex references that readers
resolve for Get, MultiGet, and iteration, including mixed embedded and
non-embedded wide-column values.
Embedded-blob handling is folded directly into BlockBasedTableBuilder rather than
living in SstFileWriter or a separate table-builder wrapper: SstFileWriter only
selects the mode (via TableBuilderOptions::embedded_blob_options), and the
builder writes blob records inline using its own file writer and running offset.
This is enabled by disabling index-value delta encoding for these SSTs — delta
encoding reconstructs a data block's offset from the previous block and so
requires byte-contiguous data blocks, which interleaved blob records would break.
With full (non-delta) index values, blob records can sit between data blocks, so
no entry buffering/replay is needed. To keep inline blob appends correctly
ordered with data-block writes, these SSTs use single-threaded (non-parallel)
compression. The mode is the only entry point today but the placement keeps it
open to generalization beyond SstFileWriter; regardless, this experimental
feature is expected only to have niche applications.
(Previous revisions of this change allowed delta-encoded index blocks by
putting all blobs at the beginning of the file, but that was a more awkward
and memory-hungry implementation due to buffering all the data blocks before
writing.)
The on-disk record format (SimpleGen2Blob: payload bytes followed by a 5-byte
trailer of a compression marker plus a builtin checksum that is context-modified
by the record's absolute file offset) lives in db/blob/blob_gen2_format.{h,cc},
which now owns both the read (ReadAndVerifySimpleGen2BlobRecord) and write
(WriteSimpleGen2BlobRecord) sides so the format is defined in one place. This is
expected to be reused for upcoming "blog file" support.
Readers need no record-layout metadata: same-file blob resolution is purely
absolute-offset keyed, and the per-record offset-modified checksum (plus a cheap
"record fits within the file" bound) is the corruption guard. The reader's only
embedded-blob metadata is presence: a best-effort auxiliary table property
(blob count and payload-byte totals, for diagnostics) whose mere presence signals
that the SST contains embedded blobs.
Reads route through the column family's BlobSource when a DB is attached, so
embedded payloads are served from / inserted into the blob value cache and
recorded in BLOB_DB_* statistics; the cache key is derived from the
SimpleGen2Blob offset scheme (the same scheme block-based SST blocks use), so
embedded blob records stay collision-free with data blocks even when the blob
cache and block cache are the same cache. Non-DB openers (SstFileReader,
sst_dump, repair, ingestion prevalidation) have no BlobSource and fall back to a
direct, uncached read.
Same-file BlobIndex references use blob file number 0 as the marker. That value
also serves as the invalid blob-file-number sentinel in broader metadata code,
but the meanings do not conflict when used carefully: only the embedded-SST
reader/writer path interprets 0 as same-file, while generic file-metadata paths
continue to reject it as invalid. Using 1 would be worse because legacy
"stackable" BlobDB can use low blob file numbers, including 1, so reserving it
would collide with real blob files.
Compression options remain in the public API as placeholders, but embedded blob
compression support is deferred. Integrating compression with
BlockBasedTableBuilder while avoiding copied CompressAndVerifyBlock-style logic
is tricky enough to deserve a separate, focused PR.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14851
Test Plan:
- Added basic feature coverage to the crash test.
- Added BlobIndexTest.SameFileBlobIndex and BlobGarbageMeterTest.SameFileBlobIndex
coverage for same-file BlobIndex encoding, display, recognition, and ignoring
same-file references in blob-garbage accounting.
- Extended FileMetaDataTest.UpdateBoundariesBlobIndex to preserve the generic
zero-file-number corruption check while keeping same-file embedded blob
semantics at the table-reader/writer layers.
- SstFileReader embedded blob coverage: round-trip Get, MultiGet, and iterator
reads; format_version gating; ignored placeholder compression options; the
2048-byte default min_blob_size; wide-column mixed embedded/non-embedded
values; and early append-error surfacing.
- Added an interleaved-layout test (small block_size with alternating
small/large values) asserting the SST property index_value_is_delta_encoded==0,
more than one data block, and that blob records are interspersed with data
blocks (not a strict front prefix), with all values read back correctly via
Get and iteration; replaces the old "ignored bytes before/after the blob record
prefix" test.
- Added an embedded-record corruption test: flipping a byte inside a blob
record's payload yields Corruption on read with verify_checksums (the
offset-keyed record checksum is the guard now that the range pre-check is gone).
- Exercised the cached read path through BlobSource, including blob-cache
hit/miss behavior and the shared blob_cache == block_cache configuration, in
db_blob_index_test.
- Normal-path CPU regression check: release (DEBUG_LEVEL=0) db_bench on a
non-embedded DB comparing this change vs upstream main, 3 interleaved reps of
fillseq, fillrandom, readrandom, and readseq (5M keys, value_size=100,
compression none, DB on /dev/shm). All deltas were within run-to-run noise
(~1%), i.e. no measurable regression from adding the embedded-mode branch to
the builder hot path.
Reviewed By: xingbowang
Differential Revision: D108564468
Pulled By: pdillinger
fbshipit-source-id: 5f01ffb1d40c6fd5b82d2451ec3342abb5040ca6
|
||
|
Xingbo Wang
|
fe053a26aa |
Macos CI fix for jdk dependency issue (#14885)
Summary: - Current 3rd party jdk8 installation fails due to trust issue in homebrew. - Detect and reuse an existing Jdk installation on macOS runners before installing a new cask. - Still target on jdk 8 binary build. - Stop using 3rd party jdk installation, Pull Request resolved: https://github.com/facebook/rocksdb/pull/14885 Test Plan: - CI Reviewed By: jaykorean Differential Revision: D109696828 Pulled By: xingbowang fbshipit-source-id: cc71958a05bd58acaac5f05fe3349ffd3e57880f |
||
|
Josh Kang
|
8ba4204b28 |
Do not allow read only DBs to delete obsolete files (#14881)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14881 Read-only DB instances can share a directory with a live writer, so their view of live files is only a snapshot from the time they opened. After read-only DB open began setting `opened_successfully_` for its intended open-success semantics, that state had an unintended side effect: the common close path treated a successful read-only open like a read-write open and ran obsolete-file cleanup. If the writer created or made files live after the read-only handle opened, that cleanup could use the stale read-only live set and delete files still needed by the writer. This change records whether a `DBImpl` was opened read-only and skips close-time obsolete-file cleanup for read-only DBs instead of simply keeping `opened_successfully_=false`, which could be confusing and easily mistaken in the future again. For completeness I also updated other callsites of `opened_successfully_`, but those should not be real bugs as ReadOnly DBs do not run flushes/compactions or write to WALs. Reviewed By: xingbowang Differential Revision: D109622445 fbshipit-source-id: d7be4b20fce86ccb218a63ac6f5b707b316aac79 |
||
|
Jay Huh
|
b0ecf86f6e |
Fix range lock manager assertion crash on reverse-comparator CF point locks (#14880)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14880 The RocksDB 11.5.0 reverse-comparator range-lock fix (the HISTORY.md entry "Fixed a bug where the range lock manager would crash with an assertion failure when using a reverse comparator column family") made `RangeTreeLockManager::CompareDbtEndpoints` reverse-aware by wrapping the endpoint-suffix tie-break results in an `is_reverse` flip. The length-disparity (different user key) branches need that flip, but the equal-length / equal-user-key branch was flipped too, and that is incorrect. Range-lock endpoints are encoded as `[suffix_byte][user_key]` where the suffix is `SUFFIX_INFIMUM` (0x0) or `SUFFIX_SUPREMUM` (0x1). When two endpoints share the same user key and differ only in the suffix byte, the suffix is a positional boundary marker for that key, not key content, so the infimum endpoint must sort before the supremum endpoint in any total order, regardless of the column family sort direction. Flipping it for a reverse comparator makes `m_cmp(infimum(K), supremum(K)) > 0`. MyRocks issues an equality lock (e.g. `UPDATE t1 SET a=123 WHERE a=35`) as the range `[infimum(K), supremum(K)]` over the same user key K. On a reverse (`rev:`) column family — which uses the stock `ReverseBytewiseComparator()`, so `is_reverse` is true — the flipped comparator reports `left > right`, tripping `paranoid_invariant(m_cmp(left_key, right_key) <= 0)` in `toku::locktree::try_acquire_lock` and aborting the server with SIGABRT. This broke many MyRocks `---range_locking` MTR tests (partial_index*, bypass_select_range_sk*, alter_table_online_debug, allow_no_primary_key_with_sk) after RocksDB was bumped to 11.5.0 in D108936924. Fix: in the equal-length / equal-user-key branch of `CompareDbtEndpoints`, compare the suffix bytes without the `is_reverse` flip so infimum always precedes supremum. The length-disparity branches keep the reverse-aware flip (still covered by `RangeLockWithReverseComparator`). Note: this lands in `internal_repo_rocksdb/repo`, the RocksDB development tree; it reaches `fbcode/rocksdb/src` (what MyRocks/ZippyDB link) through the RocksDB release sync. Tracked in T276980728. Reviewed By: xingbowang Differential Revision: D109591908 fbshipit-source-id: 078b16e6e51f4f365b4a8d539ac5b968a7461729 |
||
|
zaidoon
|
1cec28d82d |
Finish C API code generation (continues #14572) (#14868)
Summary: This continues and finishes **https://github.com/facebook/rocksdb/issues/14572** ("Add semi-automated code generation for RocksDB C API bindings") by xingbowang. The original author is unavailable to finish it, so I've taken it over. **All 13 of the original commits are preserved** (this branch was created from the PR head and builds on top of it — `git log` shows the original `Xingbo Wang` authorship intact); my follow-up work is in the commits prefixed `C API codegen:`. The underlying design is unchanged and is the original author's: hand-written source templates (`tools/c_api_gen/c_base.h` / `c_base.cc`) plus two generators (auto-discovery from the C++ headers + a spec-driven generator) are inlined into a single, self-contained, `generated` `include/rocksdb/c.h` and `db/c.cc`. This grows the public C API by **668 functions** while keeping `c.h` a single includable header with no `-I` requirement (so `bindgen` and other FFI tools keep working unchanged). This branch reconciles the PR with ~4 months of `main` and addresses the outstanding review feedback (clang-tidy bot, the automated code review, the `c.h` self-containedness discussion, and pdillinger's points about `include/rocksdb` hygiene and `generated` marking). ## What changed on top of the original PR ### Reconciled with current `main` - Merged current `main` (conflicts were confined to the generated/test files) and regenerated. Reconciled the 14 C API functions `main` added since the merge-base (e.g. `rocksdb_set_db_options`, the backup-engine rate limiters, `memtable_batch_lookup_optimization`, `optimize_multiget_for_io`, …) and restored 5 enum constants that upstream had added by hand (`rocksdb_txndb_write_policy_*`, `..._index_block_search_type_auto`, `rocksdb_blob_cache_read_byte`). ### Maintainer feedback (pdillinger) - **No non-user-includable files in `include/rocksdb`.** Moved the hand-written templates out of `include/rocksdb/` and `db/` to `tools/c_api_gen/c_base.{h,cc}`. They were `#include`-ing generated fragments, which broke `make check-headers` and was shipped by `make install`. `include/rocksdb/` now contains only the user-facing, self-contained, `generated` `c.h`. - `c.h` / `c.cc` carry the `// generated` marker. ### Backward compatibility (zero ABI break) - The generator derived each wrapper's C type purely from the C++ field, which had silently changed **5 already-shipped signatures** (e.g. `rocksdb_writeoptions_disable_WAL` `int` → `unsigned char`). Added an ABI type-pinning layer (`tools/c_api_gen/abi_type_overrides.json`) so already-shipped functions keep their historical C signature (the body still casts to the real field type). A repo-wide diff against the merge-base now reports **0 ABI drift**. - New `check_api_compatibility.py` gate (wired into CI + `make`) fails on any removed/changed public function **or** removed enum/typedef symbol, vs a reference revision. Intentional changes go in an allowlist with a reason. ### Correctness (from the automated review) - Restored 5 option setters that were declared in `c.h` but **defined nowhere** (link failure for downstream bindings such as `rust-rocksdb`). Added `check_api_completeness.py` (dependency-free; runs in CI + `make`) asserting every declared function has exactly one definition — this is the gate that would have caught it. - `CopyStringVector` now null-checks `malloc`; the WAL filter `std::move`s the `WriteBatch`; the backup exclude-files callback captures by value instead of the wrapper pointer. ### Build / CI robustness - Removed the dead `C_API_CODEGEN_STAMP` Makefile prerequisite (it was a silent no-op). - The `make check` staleness check is now opt-out-able (behind `SKIP_FORMAT_BUCK_CHECKS`) and skips gracefully when `clang++` is unavailable, so `make check` works without the codegen toolchain. CI remains the authoritative gate. - Pinned `clang-format` consistently through `regen_all.py` / `verify_generated_up_to_date.py` (CI uses clang-format-21) so regeneration is byte-reproducible across environments. - Cleared all 20 `clang-tidy` warnings the bot reported on `db/c.cc` changed lines (fixed in the `c_base.cc` template, not the generated output). - Updated the internal Buck `c_test_bin` wrapper to expose generated `c_api_gen/*.inc` fragments as headers, so sandboxed Buck builds can compile `db/c_test.c` after the generated round-trip tests are included. ### Test coverage - Added `gen_roundtrip_tests.py`, which derives **462 set→get→assert round-trip checks across 25 option objects** from the same generated fragments and wires them into `db/c_test.c`. Coverage now tracks the generated surface automatically. ### Docs - Added the `unreleased_history/public_api_changes` note and fixed `claude_md/add_public_api.md`, which still told contributors to hand-edit the now-`generated` `c.h`/`c.cc`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14868 Test Plan: - `make c_test && ./c_test` — **passes**, including the 462 generated round-trip assertions (a successful link also confirms the API is complete). - `python3 tools/c_api_gen/check_api_completeness.py` — all 1737 declared functions defined exactly once. - `python3 tools/c_api_gen/check_api_compatibility.py --ref <release>` — 1070 reference functions + 229 enum/typedef symbols preserved, 0 removed/changed. - `python3 tools/c_api_gen/verify_generated_up_to_date.py` — generated output is stable. - `include/rocksdb/c.h` confirmed self-contained (only `<stdbool.h>`, `<stddef.h>`, `<stdint.h>`). cc xingbowang - `buck2 build --flagfile fbcode//mode/dev fbcode//internal_repo_rocksdb/repo:c_test_bin` — passes. - `buck2 build --flagfile fbcode//mode/dev --config fbcode.arch=aarch64 fbcode//internal_repo_rocksdb/repo:c_test_bin` — passes. Reviewed By: pdillinger Differential Revision: D109149150 Pulled By: xingbowang fbshipit-source-id: 3417375345f360a4c78bdfe27e9850b89d0a226a |
||
|
Peter Dillinger
|
30295a4b92 |
Fix db_stress rollback retry exhaustion under concurrent Resume Busy (#14877)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14877 The rollback-after-failed-commit retry loop in `db_stress_test_base.cc` (added in b0b52e1d14f3) counted every loop iteration — including iterations where `Resume()` returned Busy — against a single 100-count limit. With 32 concurrent threads, when write fault injection causes all prepared transaction commits and rollbacks to fail simultaneously, `Resume()` returns Busy (recovery in progress from another thread) for the entire 100-iteration budget (100 × 10ms = 1 second). Recovery itself keeps failing due to ongoing fault injection causing repeated background flush errors, so no thread ever gets a chance to perform an actual rollback. The fix separates two budgets: - `kMaxRollbackRetries` (100): counts only actual rollback attempts after `Resume()` returns OK - `kMaxResumeBusyWaits` (3000): allows up to 30 seconds of busy-waiting for recovery to complete on another thread Key triggering options: `write_fault_one_in=1000` + `txn_write_policy=1` (write-prepared) + `two_write_queues=1` + 32 threads + `error_recovery_with_no_fault_injection=1`. Reviewed By: joshkang97 Differential Revision: D108561314 fbshipit-source-id: 03f4578ea33289685ad982208451f0584bf0535f |
||
|
Xingbo Wang
|
1f20db7a31 |
Add FIFO KV-ratio compaction blog post (#14871)
Summary: - Add a blog post explaining FIFO KV-ratio compaction for BlobDB-backed TTL workloads. - Describe why the existing FIFO intra-L0 picker can repeatedly compact small BlobDB SST files and how the new KV-ratio target plus tiered merging strategy works. - Add diagrams for the old picker behavior and the KV-ratio tiering flow, along with the required blog author metadata. ## Testing - `git diff --check upstream/main...HEAD` Pull Request resolved: https://github.com/facebook/rocksdb/pull/14871 Reviewed By: joshkang97 Differential Revision: D109243987 Pulled By: xingbowang fbshipit-source-id: 7ad07953f38c55c77ffb6510824ba8500db78c0e |
||
|
Josh Kang
|
f8987bc7b9 |
Include PR and version for range tombstone blog post (#14875)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14875 title Reviewed By: pdillinger Differential Revision: D109353179 fbshipit-source-id: c14c9ecbd29a8118b1d0f0689e2f29c162bd15e8 |
||
|
Maciej Szeszko
|
30f42e00ba |
fix package fault injection log parser in db stress fbpkg (#14874)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14874 ### Problem RocksDB [alert](https://www.internalfb.com/monitoring/alert/?alert_id=twshared38314.04.ldc2%40%23%24host_health.crashlooping_key.tw_agent.tw_agent.task_failure.app.count.60%40%23%24uhc%3A%20tupperware%20task%20crashlooping) fired for `tsp_ldc/RocksDB/rocks_on_ws_stress_test` after the job started crashlooping with `ModuleNotFoundError: No module named fault_injection_log_parser`. ### RCA D101973626 added `import fault_injection_log_parser` to `db_crashtest.py`, but `rocksdb_db_stress_internal_repo` only packaged `crashtest.py` and `rocks_db_stress`, so the `release_test` fbpkg rolled to TW without the new helper module. ### Fix Export `tools/fault_injection_log_parser.py` and include it in the `rocksdb_db_stress_internal_repo` fbpkg `path_actions` so it is installed next to `crashtest.py`; also add the BUCKLINT-required `network_access = network_access_utils.none()` on the existing `db_stress_auto_tuner_test` target. Reviewed By: xingbowang Differential Revision: D109346224 fbshipit-source-id: 3f770a7ad306d246f279641bf0f19142d05560c4 |
||
|
Josh Kang
|
6ccfbd4a86 |
Blog post for range tombstone conversion (#14862)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14862 Add blog post for range tombstone conversion. Reviewed By: xingbowang Differential Revision: D108946953 fbshipit-source-id: f2ded24a5452d2f56a12928b979a8f44930dde03 |
||
|
Xingbo Wang
|
5b324a2160 |
Fix CI: detect Windows MSVC toolchain (#14869)
Summary: Update Windows CI to stop hardcoding Visual Studio 2022. The Windows build action now detects the installed Visual Studio C++ toolchain with vswhere, derives the matching CMake generator, and passes the corresponding version range to microsoft/setup-msbuild@v2. Also update the Windows build steps to use the preinstalled JDK instead of installing Liberica JDK8 through Chocolatey, and build Snappy/RocksDB through cmake --build, instead of assuming fixed .sln names. Rename Windows CI jobs from VS2022-specific names to MSVC-neutral names, and move the nightly AVX2 Windows job to windows-8-core so PR and nightly CI use the same runner/toolchain path. ## Testing - CI Pull Request resolved: https://github.com/facebook/rocksdb/pull/14869 Reviewed By: jaykorean Differential Revision: D109176291 Pulled By: xingbowang fbshipit-source-id: 848c5964922cf6e09b727b1ed96cbf475b325142 |
||
|
Maciej Szeszko
|
02941016d0 |
Fix seqno sync race with WriteUnprepared (#14864)
Summary: # Summary Pull Request resolved: https://github.com/facebook/rocksdb/pull/14864 With `two_write_queues`=`true`, `WRITE_UNPREPARED` can allocate sequence numbers through `FetchAddLastAllocatedSequence()` before those numbers are published through `SetLastSequence()`. Error recovery can then create new memtable/WAL boundaries from a stale `LastSequence()`, which can later surface as "sequence number going backwards" corruption. Fix this by syncing `LastSequence()` to `LastAllocatedSequence()` at the recovery entry point after draining both write queues, and again at the recovery flush fence after `FlushAllColumnFamilies()` has released/reacquired the DB mutex and before `SwitchMemtable()` consumes `LastSequence()`. Apply the same fence to atomic recovery flushes. # Perf comparative results TLDR; no visible regression. I ran a deterministic local recovery benchmark to check whether the new two-write-queue recovery fence adds measurable recovery latency. ## Workload (P2385173320) - Optimized build. - `TransactionDB` with `WRITE_UNPREPARED`. - `two_write_queues=true`. - Each iteration dirties the memtable with WUP transactions, forces a retryable flush IO error through `FaultInjectionTestFS`, re-enables the filesystem, then measures `DB::Resume()`. - 50 warmup recoveries + 500 measured recoveries per run. ## Why this is relevant This targets the changed recovery path directly rather than measuring generic write throughput. The change only runs during recovery, so the useful signal is recovery latency around `ResumeImpl()` and the recovery flush fence. ## Code path exercised - `DB::Resume()` - `ErrorHandler::RecoverFromBGError(true)` - `DBImpl::ResumeImpl()` - new `two_write_queues_` recovery queue fence - non-atomic recovery `FlushMemTable()` fence before `SwitchMemtable()` This does not cover atomic flush recovery or a writer-backlog stress case. Comparative results: | Revision | Attempt | resume_p50_us | resume_p95_us | Retryable BG errors | | Parent | | 1555.75 | 2092.22 | 550 | | Fix | 1 | 1537.68 | 1905.65 | 550 | | Fix | 2 | 1475.10 | 1883.26 | 550 | | Fix | 3 | 1617.52 | 1993.97 | 550 | Reviewed By: xingbowang, anand1976 Differential Revision: D108946310 fbshipit-source-id: 75fcb4bf0c9a932b5fbed7e934c7f14d981a1bf2 |
||
|
Maciej Szeszko
|
2d13302949 |
Fix Windows CI: run build-windows-vs2022 on windows-2022 runner (#14872)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14872 ### Motivation The `build-windows-vs2022` job in `pr-jobs.yml` was running on the `windows-8-core` runner, whose image no longer provides a Visual Studio 2022 installation. The job sets `CMAKE_GENERATOR: Visual Studio 17 2022`, so CMake's generator probes for an installed VS 2022 instance and aborts the Snappy dependency configure with: ``` CMake Error: Generator Visual Studio 17 2022 could not find any instance of Visual Studio. ``` The failure surfaces during the thirdparty Snappy build, before any RocksDB code is compiled, so it blocks the entire Windows PR signal. ### Changes Switch the `build-windows-vs2022` job from `runs-on: windows-8-core` to `runs-on: windows-2022`. The GitHub-hosted `windows-2022` image ships VS 2022 with the Desktop C++ workload preinstalled, so the `Visual Studio 17 2022` generator resolves out of the box and the `CMAKE_GENERATOR` value stays consistent with the runner's installed toolchain. This also brings `pr-jobs.yml` in line with `nightly.yml`, whose `build-windows-vs2022-avx2` job already runs on `windows-2022`. NOTE: `windows-2022` is the standard 2-core hosted runner (vs. the larger `windows-8-core` pool), so Windows builds will be somewhat slower; ccache keeps the compile cost mostly in cache hits. Reviewed By: pdillinger Differential Revision: D109229258 fbshipit-source-id: b9f6d9ef8b4080b1b1129ed0ae6a9b1da2b7b14c |
||
|
Xingbo Wang
|
214869aacd |
Persist fault injection logs and fail fast on expected-state trace writes (#14651)
Summary: - switch fault injection error recording from an in-memory ring buffer to per-run fixed-record binary logs under `TEST_TMPDIR/fault_injection_logs` (or `/tmp/fault_injection_logs`) so crash paths survive DB reopen cleanup - keep the raw and decoded fault logs for external artifact collection/cleanup, and make `db_crashtest` print consistent blackbox/whitebox summaries after decoding - make expected-state tracing fail fast on trace write failures and document offline trace inspection via `trace_analyzer` - add coverage for binary log persistence/decoding/truncated-tail handling and keep info logs excluded from fault injection Reviewed By: hx235 Differential Revision: D101973626 fbshipit-source-id: fdcb5b6370cf92a046e09b8d3391e80eecb66c23 |
||
|
anand1976
|
5bf78183db |
Update version to 11.6.0 for 11.5 release (#14861)
Summary: - Bump version.h from 11.5.0 to 11.6.0 - Add `11.5.fb` to `check_format_compatible.sh` - Cherry-pick HISTORY.md from `11.5.fb` - Update folly hash in `folly.mk` - Delete `unreleased_history/` note files Part of 11.5 release workflow. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14861 Reviewed By: anand1976 Differential Revision: D108971785 Pulled By: jaykorean fbshipit-source-id: d4076d34f860c9caddfac38a5406e02112436cc2 |
||
|
Josh Kang
|
e292aa95b3 |
Allow skipping lmax index and filter block prefetches for external file ingestion
Summary: Bulk external file ingestion can spend significant commit time prefetching index/filter blocks for large Lmax files. This adds `IngestExternalFileOptions::prefetch_lmax_index_and_filter_blocks` (default true) so bulk-load callers can defer that metadata prefetch when the cache doesn't need immediate warming. The option threads through `FileMetaData::skip_index_and_filter_blocks_prefetch` to drive the existing `LoadTableHandlers()` prefetch logic. Benchmarks show ~40% commit latency reduction (3459 → 2112 μs) and ~55% lower IO time (2560 → 1145 μs) when disabled. Benchmarks: ``` db_bench --benchmarks=ingestexternalfile --num=2200000 --ingest_external_file_num_batches=1 --ingest_external_file_batch_size=1 --ingest_external_file_use_file_info=true --ingest_external_file_fill_cache=true --cache_index_and_filter_blocks=true --bloom_bits=10 --statistics=true --stats_level=3 --ingest_external_file_prefetch_lmax_index_and_filter_blocks=<true|false> ``` | `ingest_external_file_prefetch_lmax_index_and_filter_blocks` | `rocksdb.ingest.external.file.run.micros` | `rocksdb.table.open.io.micros` | index/filter cache adds | last-level read bytes | | `true` | `3459 us` | `2560 us` | `1 / 1` | `3551559` | | `false` | `2112 us` | `1145 us` | `0 / 0` | `1333` | Reviewed By: xingbowang Differential Revision: D108678511 fbshipit-source-id: e8951e3aad36cc0accffb33ecc3cc1b4aeb89459 |
||
|
Peter Dillinger
|
123030ffee |
Graceful shutdown on fatal benchmark errors (#14855)
Summary: db_bench worker operations historically responded to a fatal error (failed Get/Put/Merge/Delete, checksum failure, etc.) by calling db_bench_exit() -> exit() in place. When that happened on a worker thread while sibling workers and background (compaction/flush) threads were still running, exit() began static/global destruction underneath those live threads. A thread mid-call into a user callback (e.g. a custom Env/ FileSystem) could then touch a global/singleton that had already been destroyed -> cross-thread use-after-free. This is the same class of bug fixed for db_stress in https://github.com/facebook/rocksdb/issues/14850, but it also matters for db_bench's secondary role as an *integration validation* tool: we want production-like processes to shut down cleanly after an error (so a custom FileSystem can log the adverse condition and run its teardown) rather than crash or _exit at the first sign of trouble. This change makes fatal errors on benchmark worker threads unwind instead of exiting in place, and performs an ordered shutdown on the main thread: * SharedState gains a cooperative fatal flag + first-error Status/message and SetFatal(). A worker that hits a fatal error records it and returns out of its operation (releasing its DbUseGuard) instead of exiting. * Op loops stop promptly via Duration: the loop-termination helper now consults the run's fatal flag at its existing throttled check boundary (~every FLAGS_ops_between_duration_checks ops), so peers wind down with no added per-key cost. * After RunBenchmark joins all workers, the (guard-free) main thread runs the ordered shutdown -- DeleteDBs() closes all DBs, joining background threads and running user Env/FileSystem teardown -- and only then exits nonzero. No static destructors run while RocksDB threads or user callbacks are still live. * The worker fatal sites that previously called db_bench_exit() (BGWriter, DoDelete, RandomWithVerify, UpdateRandom, XORUpdateRandom, MergeRandom, ReadRandomMergeRandom, VerifyChecksum, VerifyFileChecksums, RandomReplaceKeys, Replay, DoDeterministicCompact) now SetFatal + return. Flush (main-thread) routes to ErrorExit() which closes DBs first. Supporting changes: * Duration is now a nested member type of SharedState, constructible only via SharedState::MakeDuration() (movable, non-copyable). This binds every Duration to its run's fatal flag by construction and removes the previous mutable global (g_cooperative_abort_flag) and its set/clear lifecycle. A `using Duration = SharedState::Duration;` alias keeps call sites tidy. * New test-only flag --fault_injection_read_after_reads: wraps the FileSystem so random-access reads start returning a non-retryable IOError after N reads, to exercise the fatal/shutdown path on demand. * New flag --fatal_shutdown_watchdog_seconds (default 180, 0 disables): arms a one-shot detached watchdog on the first fatal error (and before ErrorExit's cleanup) that forces port::ImmediateExit() if the careful shutdown itself hangs (e.g. a background thread or misbehaving Env that never returns). This is a backstop, not the path under test. * Added an "Error handling background" comment documenting the rationale and the machinery, and an invariant comment at db_bench_exit covering pre-Open vs worker vs main-thread exit rules. Existing _Exit()/abort() sites are intentionally left unchanged. Future work: * Worker ops that still abort() on error (ReadRandom, ReadRandomFast, ReadToRowCache, MultiReadRandom, MixGraph, AppendRandom, UpdateRandom's read path, BGScan, RandomTransaction, CreateNewCf) are not the use-after-free class (abort() skips static destruction) but are not graceful either; converting them would extend clean-shutdown coverage. RandomTransaction needs a rollback decision; MultiReadRandom needs a structured break-out. * Post-Open main-thread db_bench_exit() sites in Open()/Run()/ VerifyDBFromDB() are the harder, https://github.com/facebook/rocksdb/issues/14850-class case: Open() is reachable both under and outside a DbStateMutationGuard, so it cannot unconditionally call ErrorExit() without deadlock. Needs guard-aware handling or restructuring Open() to return Status. * Pre-Open setup exits (option/flag parsing, cache setup) correctly stay plain exit() -- no live threads -- and are out of scope. * Fault injection currently covers read faults only; verifying graceful shutdown for write/open/transaction error paths would need write-fault, open-fault, and conflict injection. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14855 Test Plan: Built debug db_bench (make db_bench) and validated manually: * Correctness of loop termination (unchanged by the Duration refactor): - Count-bounded is exact: `db_bench --benchmarks=fillrandom --num=300` reports "300 operations". - Time-bounded honors duration and is not capped by max_ops: `db_bench --benchmarks=readrandom --use_existing_db --duration=5 --reads=3000` ran 5.002 s / ~2.0-2.7M operations. * Graceful shutdown on a fatal error (multi-threaded, peers live): `db_bench --benchmarks=xorupdaterandom --use_existing_db --threads=8 --duration=30 --fault_injection_read_after_reads=5000` -> exit code 1, ZERO "Received signal", prints 'Fatal error during "xorupdaterandom"' after the ordered shutdown. * Confirmed this is a real fix, not just masking: rebuilt pristine main + only the fault-injection flag (graceful-shutdown changes reverted) and ran the same command -> SIGSEGV (rc=139), crash stack in __run_exit_handlers -> jemalloc free, triggered from XORUpdateRandom -> db_bench_exit() -> exit() with sibling threads still live. With the fix in place the same workload is graceful (above). * Watchdog: - Default (180 s): graceful runs above complete with no watchdog message (no spurious firing). - --fatal_shutdown_watchdog_seconds=0 disables it (still graceful). - Positive: temporarily inserted a 60 s sleep before DeleteDBs in the fatal path, ran with --fatal_shutdown_watchdog_seconds=3 -> process exited at ~3 s (not 60 s) with the watchdog message and rc=1; reverted the temporary sleep and rebuilt clean. * No benchmark performance impact (normal, error-free runs): - Structural: the cooperative-abort check adds no per-key work. It is evaluated only at Duration's pre-existing throttled boundary (~every FLAGS_ops_between_duration_checks ops, same site as the existing time check); there is no atomic load on the per-op hot path. Fatal recording (SetFatal) lives only in cold error branches. A Duration is constructed once per benchmark via the factory (returned by value, elided), not per op. - Observed: error-free fillrandom / fillseq / readrandom throughput (e.g. fillrandom ~300K ops/s, fillseq ~800K-870K ops/s) was in line with pre-change runs; no regression observed. * make format-auto applied; debug build is clean. Reviewed By: anand1976 Differential Revision: D108690137 Pulled By: pdillinger fbshipit-source-id: 7ffdf4abc665b4c2172b718670c5c2fa1da58500 |
||
|
Josh Kang
|
5f66923d1e |
External file ingestion, support fast prepare path from DB generated files (#14854)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14854 External SST ingestion can already skip the open-and-scan metadata path when ingesting files produced by `SstFileWriter`. This extends that fast path to live SST files produced by a DB, so applications that move DB-generated files between DBs can request prepared metadata directly from the source DB and pass it through ingestion. The API is intentionally limited to live table files whose metadata is already available from the DB and cached table reader. Callers remain responsible for keeping the source file alive and unchanged until ingestion completes. Reviewed By: xingbowang Differential Revision: D108440584 fbshipit-source-id: e732278ea194aac71f046b54234238b35c6fe0e5 |
||
|
Josh Kang
|
7785264e5a |
Make file opens multi-threaded for commit external sst file ingestion (#14853)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14853 `IngestExternalFileOptions` now exposes `file_opening_threads` so external file ingestion can open table readers for newly committed SSTs in parallel. The requested thread count is carried from each ingestion job into commit and then into manifest `LogAndApply`, where `VersionBuilder::LoadTableHandlers()` uses it while installing the ingested files. This is intended for workloads that ingest many external SSTs in one call, where serial table-reader opens can dominate commit latency. The ingestexternalfile benchmark now has a flag for this option, and `db_stress` now creates multiple data SSTs per ingest operation so stress runs cover multi-file ingestion, file-info ingestion, and the parallel file-opening path together. Benchmarks Ran `db_bench --benchmarks=ingestexternalfile --use_existing_db=0 --num=2200000 --compression_type=none --statistics --use_direct_reads=true --ingest_external_file_batch_size=200 --ingest_external_file_num_batches=1 --ingest_external_file_use_file_info=true --ingest_external_file_fill_cache=false` while varying `--ingest_external_file_file_opening_threads`. The generated SST files were about 256 MiB each. | `file_opening_threads` | `rocksdb.ingest.external.file.prepare.micros` | `rocksdb.ingest.external.file.run.micros` | File opens | | 1 | 16,150 us | 327,136 us | 200 | | 32 | 16,665 us | 38,349 us | 200 | At the same ~256 MiB SST size, 2,000 files is roughly 500 GiB of SST data. A straight 10x extrapolation of the 200-file ingest histograms puts prepare+run at about 3.43s with `file_opening_threads=1` and 0.55s with `file_opening_threads=32`; run alone is about 3.27s vs. 0.38s. Reviewed By: pdillinger Differential Revision: D108347952 fbshipit-source-id: 49efa48524df64cace010b881030f63f771cbb95 |
||
|
Josh Kang
|
ad43a5fbe8 |
Pass file metadata to IngestExternalFile to improve ingestion latency (#14837)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14837 External file ingestion (`DB::IngestExternalFile[s]`) re-opens every SST file and scans it -- footer, properties, index, filter, and the first/last data blocks -- to recompute the boundary keys, sequence-number bounds, and table properties before committing. For cold, I/O-bound files this scan dominates ingest latency, even when the file is moved/linked rather than copied. This change lets a caller skip that work when it already has the file's metadata. `SstFileWriter::Finish()` now returns a `PreparedFileInfo` with the file size, table properties, and prepared internal `smallest`/`largest` bounds, and a new `IngestExternalFileArg::file_infos` field carries one `PreparedFileInfo` per file into `IngestExternalFiles()`. When set, ingestion reuses that metadata instead of re-opening and scanning each file. The file is still copied/linked, and the checksum is still verified when `verify_checksums_before_ingest` is set (the fast path opens the file only for that). Point-key and range-deletion bounds are folded into the same prepared bound pair, and user-defined-timestamp files (including the "UDT in Memtables only" format, whose boundary keys carry no timestamp) are handled. Internally, ingestion-job metadata acquisition was split into `GetIngestedFileInfoFromFileInfo` (reuse caller metadata) and `GetIngestedFileInfoFromFile` (open + scan). Prepared boundary updates use RocksDB comparators, and the timestamp-stripping path pads prepared bounds back to the internal timestamp shape before installing the file. The `ingestexternalfile` `db_bench` benchmark now also exposes `--ingest_external_file_fill_cache` so ingestion-read block-cache effects can be controlled independently while comparing the file-info fast path against the normal open-and-scan path. A future PR will support creating `PreparedFileInfo` from a existing DB-generated file, so this optimization is not just limited to SstFileWriter. ## Benchmark Results Existing `db_bench ingestexternalfile` benchmark, release build, using an XFS flash-backed filesystem. Files are linked (`move_files`) so the measurement isolates the ingest path rather than file-copy throughput. These numbers used direct reads and `fill_cache=false` to avoid warming the block cache while comparing the file-info fast path against the normal open-and-scan path. Each run used 1M keys/SST, one ingest call, and `db_bench` reported a 62.9 MB estimated file size. The comparison varied the number of files in that ingest call across 10, 30, and 50 files. | Files/batch | Config | Prepare P50 | Run P50 | Run P50/file | Total ingestion P50 | Total P50 drop | | --- | --- | --- | --- | --- | --- | --- | | 10 | Baseline | 10.909 ms | 7.593 ms | 0.759 ms/file | 18.502 ms | -- | | 10 | `file_info=true` | 1.127 ms | 7.541 ms | 0.754 ms/file | 8.668 ms | 53.2% | | 30 | Baseline | 29.049 ms | 23.161 ms | 0.772 ms/file | 52.210 ms | -- | | 30 | `file_info=true` | 2.734 ms | 23.384 ms | 0.779 ms/file | 26.118 ms | 50.0% | | 50 | Baseline | 49.036 ms | 39.224 ms | 0.784 ms/file | 88.260 ms | -- | | 50 | `file_info=true` | 4.133 ms | 39.417 ms | 0.788 ms/file | 43.550 ms | 50.7% | With `fill_cache=false` and direct reads, the metadata fast path cuts `Prepare()` by roughly 90-92%. `Run()` is roughly 0.75-0.79 ms per file across these runs. End-to-end `db_bench` micros/op is almost unchanged because this benchmark includes SST generation and compaction work; use `rocksdb.ingest.external.file.prepare.micros` and `rocksdb.ingest.external.file.run.micros` to isolate ingestion. Benchmark args: `--benchmarks=ingestexternalfile --use_existing_db=0 --num=1000000 --compression_type=none --statistics --use_direct_reads=true --ingest_external_file_batch_size=<10|30|50> --ingest_external_file_num_batches=1 --ingest_external_file_use_file_info=<false|true> --ingest_external_file_fill_cache=false`. Reviewed By: xingbowang Differential Revision: D107721261 fbshipit-source-id: b06ecb7b35f260ec02acf837e7986057bc23cdbf |
||
|
Mateo Correa
|
f7677f0b6b |
add rocksdb_set_db_options for dynamic DB option updates (#14615)
Summary: # Context The C API lacks a SetDBOptions wrapper, so callers using the C bindings cannot dynamically reconfigure DB-level options at runtime without dropping down to the C++ API. # Changes - Add `rocksdb_set_db_options` to the C API as a wrapper around `DB::SetDBOptions` - Adds new tests in `c_test.c` covering both valid and invalid option updates Pull Request resolved: https://github.com/facebook/rocksdb/pull/14615 Reviewed By: joshkang97 Differential Revision: D101010593 Pulled By: xingbowang fbshipit-source-id: 3feac94bd0e3b7d6a4913ce9bc014e1233292309 |
||
|
Xingbo Wang
|
c9252b9ac0 |
Clarify wide column memory ownership in public APIs (#14626)
Summary: We have seen multiple coding error with the usage of WideColumn API. Hence, improving the documentation to reduce the chance of this happening again. Clarify that `WideColumn` and `WideColumns` are non-owning views over caller-managed memory. Update the public `PutEntity()` API documentation to state that column name and value storage must remain valid until the call returns. Add safe and unsafe usage examples in `wide_columns.h` to make lifetime requirements explicit. ## Testing Not run. Documentation-only change. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14626 Reviewed By: archang19 Differential Revision: D101280295 Pulled By: xingbowang fbshipit-source-id: 076103321f1d54103dd1c2772aa9e84affdfd2ad |
||
|
Josh Kang
|
4d30cfd1fc |
Split ingest external files into prepare and commit APIs (#14849)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14849 This change splits ingestion into two public calls: `PrepareFileIngestion()` performs all of the off-mutex work and returns an opaque `FileIngestionHandle`, and `CommitFileIngestionHandle()` makes the prepared files visible under the mutex. Even though the "Prepare" phase was off the DB mutex, the application layer may still have application logic that depends on the completion of `IngestExternalFile`. `IngestExternalFiles(args)` is also just `PrepareFileIngestion(args)` followed by `CommitFileIngestionHandle()`, so its behavior is unchanged. `CommitFileIngestionHandles()` also supports committing multiple handles atomically. However, it is possible that a CF may be present in multiple handles. In this case, we merge the ingestion jobs together. This allows applications to prepare file ingestions at different times, but still provide a single atomic commit. Reviewed By: xingbowang Differential Revision: D108225105 fbshipit-source-id: 082e149f13983a4b14a8fd711d2113728485421f |
||
|
Xingbo Wang
|
61c3e93a74 |
Wire external table reads into RocksDB metrics (#14839)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14839 Wrap the filesystem passed to external table readers when statistics or file IO listeners are configured so reads performed by implementations such as Nimble update the same RocksDB metrics as regular SST reads. The wrapper records `SST_READ_MICROS`, activity-specific `FILE_READ_*` histograms, last/non-last-level and temperature read tickers, and forwards `OnFileReadFinish`/`OnIOError` to configured listeners so ZippyDB file-read counters observe external-table IO. `MultiRead` records the elapsed read histogram once and applies byte/listener accounting to each completed request. Add a table test that makes a dummy external table reader read through `ExternalTableOptions::fs` and verifies both `Statistics` counters/histograms and file-read listener updates. Add an unreleased-history entry for the new external table read metrics coverage. This revision also applies the RocksDB `make check-format` clang-format output for `external_table.cc`. Reviewed By: pdillinger Differential Revision: D108012449 fbshipit-source-id: 6b6c993c0aa99479e6e043171124bd48f0416bec |
||
|
Xingbo Wang
|
521cd8b31d |
Add parallel gtest flake reproduction runner (#14827)
Summary: - Add `tools/gtest_parallel_repro.py` to reproduce gtest flaky tests that depend on process-level CPU contention and scheduler delays. - Run many fresh gtest processes concurrently, isolate each process with its own `TEST_TMPDIR`, and optionally pin the workload to a smaller CPU set with `taskset`. - Record per-run logs, write `failures.jsonl`, summarize failure keys, support stopping on first failed batch, and optionally rebuild with `COERCE_CONTEXT_SWITCH=1`. - Document when to use the runner in `CLAUDE.md` for CI-style flaky tests that single-process `--gtest_repeat` or coerce mode alone does not reproduce. - Using the new runner, we found and fixed one flaky test and improved c_test harness - Flaky test: EnvPosixTest.ReadAsyncQueueFull simulated a full io_uring submission queue by overwriting the SQE pointer after io_uring_get_sqe() had already consumed a submission slot. That left stale state in the thread-local ring, so later AbortIO tests could process an unexpected completion and crash. Add a skip_io_uring_get_sqe syncpoint before the io_uring_get_sqe() call and use it from the test so the Busy path is exercised without mutating the ring. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14827 Test Plan: - CI Reviewed By: pdillinger Differential Revision: D107758097 Pulled By: xingbowang fbshipit-source-id: 3256612ae1ae98b66901593371743b3de7e5bb53 |
||
|
Peter Dillinger
|
77d9ed7f63 |
Use _exit(1) instead of exit(1) after DB is open to avoid UAF (#14850)
Summary: When `FinishInitDb()` or `Open()` calls `exit(1)` after the DB has been opened, background compaction/flush threads are still running. `exit()` triggers static object destruction (including the `KillPoint` singleton and its `rocksdb_kill_exclude_prefixes` vector) while those threads are still accessing them via `TestKillRandom()`, causing a heap-use-after-free detected by ASAN. This became more likely to trigger after the multi-DB support commit (3d0d60101e7f) which runs `RunStressTestImpl` on worker threads, making the race window larger when one DB fails initialization while other DBs background threads are active. The fix replaces `exit(1)` in all error paths that fire after the DB has been opened with a wrapper around `_exit(1)`. `_exit()` terminates immediately without running atexit handlers or destroying static objects, avoiding the race with background threads. Also updated CLAUDE.md to help get cross-platform compatibility right the first time. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14850 Reviewed By: hx235 Differential Revision: D108298839 Pulled By: pdillinger fbshipit-source-id: 8e87fcb259c273e2be5eb26b4eaf6009f5b998f1 |
||
|
Peter Dillinger
|
6d4a8144e0 |
Unify LZ4 and LZ4HC compression levels (#14819)
Summary: kLZ4Compression and kLZ4HCCompression share the same on-disk format and decompressor, but historically kLZ4Compression only honored negative (acceleration) levels while kLZ4HCCompression only honored positive levels. This unifies them so `compression_opts.level` alone selects the variant: level <= 0 uses LZ4 fast (acceleration = -level) and level >= 1 uses LZ4HC (1..12), regardless of which of the two types is configured. The configured type now only determines the default compression level (LZ4: acceleration 1, equivalent to level -1; LZ4HC: level 9). For code simplicity, the recorded per-block type comes from the compression type derived from the level, which could differ from the configured type. To preserve the originally configured choice for debugging/tracking, it is recorded as a `_type=<decimal>` pseudo-option in the rocksdb.compression_options SST table property. Out-of-range non-default levels are clamped to the nearest effective value (LZ4 acceleration capped at 65537, which also avoids signed overflow negating INT_MIN; LZ4HC level capped at 12). The cost-aware (auto-tune) compressor's LZ4 level grid is changed to negative accelerations so it actually exercises fast LZ4 (positive levels now route to LZ4HC). Related inclusion: The ZSTD library has a discontinuity at level=0, which maps to level 3, which is more aggressive than levels 1 and 2, which are more aggressive than levels -1, -2, etc. For better friendliness to auto-tuning (etc.), we now map level 0 to be the same as level -1, so that increasing compression level numbers have non-decreasing aggressiveness. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14819 Test Plan: New unit tests in compression_test.cc: - UnifiedLZ4LZ4HCLevels: for a representative set of non-default levels, both configured types produce identical output and the same recorded type (selected by the level), levels that clamp to the same effective parameter compress identically, and each round-trips; plus per-type default-level behavior. - ConfiguredCompressionTypeRecordedInProperties: the `_type=` pseudo- option appears in the SST table property for each configured type. - ZSTDLevelZeroMapsToMinusOne: level 0 behaves like -1, not like 3. Reviewed By: joshkang97 Differential Revision: D107536580 Pulled By: pdillinger fbshipit-source-id: a1281956f70a75d0620cb73d0bfb9ad76c52cca3 |