693 Commits

Author	SHA1	Message	Date
Yoshinori Matsunobu	d6a7847d37	Add a DB option to log long-held global mutexes and release sites (#15436) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15436 ## Why Long-held global mutexes can stall foreground and background work. Existing mutex statistics measure acquisition waits, not continuous ownership. This adds opt-in logging to identify the mutex, owning thread, operation and release site. ## Enable for an investigation Provide an INFO logger and shared `Statistics` object when opening the DB, then: - Set the shared statistics level to `StatsLevel::kAll`. - Set `mutex_hold_log_threshold_us` through `DB::SetDBOptions`; `100000` means 100 ms. - Restore threshold 0 and the previous statistics level afterward. The threshold defaults to **0 (disabled)**. `kAll` alone does not enable these hold probes; threshold 0 leaves existing statistics timers unchanged. ## Output Illustrative records, not production observations: ``` SLOW_GLOBAL_MUTEX mutex=DBImpl::mutex_ mutex_addr=0x12340000 owner_thread_id=12345 thread_operation=Compaction hold_us=200000 threshold_us=100000 observed_end_us=1792205400000000 buffer_dropped_records=0 unlock=db_impl_compaction_flush.cc:4660 BackgroundCallCompaction() SLOW_GLOBAL_MUTEX mutex=DBImpl::wal_write_mutex_ mutex_addr=0x12340100 owner_thread_id=12346 thread_operation= hold_us=35000 threshold_us=25000 observed_end_us=1792205401000000 buffer_dropped_records=0 scope_exit=db_impl_write.cc:2250 PreprocessWrite() ``` The first proves a completed 200 ms DB-mutex ownership interval on a thread tagged `Compaction`. The second uses a 25 ms threshold; an empty operation tag means none was available. Neither record reports acquisition wait or proves the cause of a stall. Release attribution uses compiler-embedded strings available in optimized, stripped builds, with a GCC/Clang fallback where standard source locations are unavailable. Direct unlock and condition wait identify their call sites; scope exit and temporary unlock identify guard declarations, not closing braces. No stack unwinding or symbol lookup. ## Scope and safety - Track six selected mutexes: DB, WAL, point-lock wait graph, two prepare-log bookkeeping mutexes and transaction name map. This is not comprehensive synchronization tracing. - Acquisition waits and condition-variable release gaps are excluded from hold duration. Configuration is pre-first-use; repeated setup retains the first valid context. - Sample at most once per mutex per second, with eight deferred records per thread. `buffer_dropped_records` counts capacity overflow only; rate limiting, pause and emission contention can omit samples. - Emit after covered outer lock scopes release. Emission is nonblocking best-effort; shutdown drains/pauses emission and stops diagnostics before the owned logger closes. Closing from that logger's own callback is unsupported. - No data-file format change. The mutable option is persisted in OPTIONS and wired into bench/stress tools. Typed C binding is deferred; string DB-option APIs are available. Disabled bookkeeping still has a cost. Release A/B measurements found about 3 ns added per selected mutex pair at level 1, and about 70 ns for hold timing over `kAll`/0. Get and reused-iterator Seek showed no resolved small probe effect; fresh iterator+Seek at 16 threads showed +8.7% CPU/query and -9.4% QPS. No logs were emitted in those benchmarks; logging cost and broader workload qualification remain open. Reviewed By: xingbowang Differential Revision: D125790955 fbshipit-source-id: 1096163f3bd25ffee585dc6ec8ebfd7f1759851a	2026-10-22 09:50:20 -07:00
zaidoon	553b52b4a7	Wire IndexFactory through block-based tables (#14963) ... Summary: Build block-based table indexes through `IndexFactory`, with modes for writing both indexes or only the custom index and a per-read index selector. Built-in reads retain their existing cache and prefetch paths. The review fixes preserve the existing UDI behavior at the migration and rollback boundaries: - Reopening an SST with index caching enabled reuses the serialized custom block. Each table reader constructs its own factory reader over those bytes and passes `NewReader()` a `Slice` view stored in the table reader. That view stays valid for the reader's lifetime, so a legacy reader can advance it or keep a pointer to it without changing the cached block. The view and the raw block are destroyed after the custom reader. - `IndexFactoryReader::ApproximateMemoryUsage()` keeps its documented meaning: the reader's own allocations, excluding the serialized block. `MemoryUsageIncludesIndexBlock()` defaults to false. A reader whose estimate includes the block opts in, and the wrapper subtracts the block size from that estimate. - Copying factory options and clearing a legacy UDI flag now rolls back that flag's policy. Legacy-derived options keep the requested default and resolve their effective mode without changing the inputs. Explicit factory modes retain precedence through cloning, failed updates, reconstruction, and the C API, including an explicitly selected `kStandardDefault`. - OK empty custom-index output still produces a zero-size meta block and reads through the full standard index. This covers point data and files containing only range tombstones. Custom-only mode rejects empty output because it has no standard fallback. - Default primary reads retain the table factory's precedence over a legacy per-read factory pointer. Explicit `kPreferCustom` still checks a supplied factory name; `kBuiltin` ignores that pointer. - OPTIONS files write equivalent legacy booleans for `kStandardDefault`, `kStandardRequired`, and `kCustomDefault`. Older releases retain those policies when loading newer files with `ignore_unknown_options=true`; the explicit-mode marker remains authoritative for current readers. Custom-only SSTs carry a footer feature bit so older readers reject them. Reject unsupported filter/index combinations before constructing filters. Trie size estimates account for shared prefixes, and block offsets or sizes beyond the 32-bit format limit return `NotSupported` instead of truncating. ## Safety and compatibility - Custom-only files require format version 6 or later and the factory. Rewrite every custom-only SST with a full standard index before removing the factory or downgrading. The other modes have no equivalent legacy configuration beyond the three listed above. - A raw C++ options struct cannot distinguish an explicitly assigned `kStandardDefault` from its default value. Configure the factory to make that choice explicit. `GetEffectiveIndexMode()` reports the resolved policy while preserving legacy inputs for copy-and-clear rollback. - A custom factory with user-defined timestamps is rejected in every mode except `kStandardOnly`, which ignores the factory. - Trie construction rejects a data-block offset or size greater than `UINT32_MAX`, including in `kStandardDefault`. That can fail flush, compaction, or `SstFileWriter::Finish()`; total SST size above 4 GiB alone is not the rejection condition. File-size targets do not split L0 or `SstFileWriter` outputs. - Custom-index cache hits, misses, and adds are reported under `BLOCK_CACHE_INDEX_*` instead of `BLOCK_CACHE_DATA_*`. The existing cache setting still controls ownership and cache use. - A table reader now charges a custom index block it owns (`cache_index_and_filter_blocks=false`); before, that block was not charged anywhere. Cached blocks are still charged only to the block cache. A reader whose `ApproximateMemoryUsage()` includes the serialized block, such as IntegerTrie or TimeSeries, must override `MemoryUsageIncludesIndexBlock()` to return true, or that block is charged twice. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14963 Test Plan: For this commit, on macOS: `table_test`, `trie_index_test`, `trie_index_db_test`, and `block_based_table_reader_test` pass on the full stack (11,416 cases), and the full `table_test` passes at this PR's head alone (6,969 cases). The four custom-reader compatibility tests pass 100 times with `ASSERT_STATUS_CHECKED=1` and `COERCE_CONTEXT_SWITCH=1`, and 84 UDI table cases pass under ASAN with stack-use-after-return detection. Release-mode syntax checks with `-DNDEBUG -fno-rtti` on the changed sources, source checks, formatting checks, and `git diff --check` pass. Two new regressions cover the latest review, and both were checked failing before the fix: - `RetainedReaderSliceOutlivesTableOpen`: a reader keeps a pointer to the `Slice` it received, the stack is overwritten after open, and the reader then serves a seek and a full scan. It covers all three custom-index modes with owned and cached blocks, including a cached reopen. Without the fix, the retained `Slice` no longer matched the block. - `AuxiliaryReaderMemoryIsFullyCharged`: a reader that implements only the older reader methods reports a fixed allocation of half and twice the raw block size. Table usage must grow by that amount with owned and cached blocks, and a strict cache sized just below the required reservation must reject the open with `MemoryLimit`. Without the fix, usage fell short by `min(allocation, raw block size)` and the strict-cache open succeeded. The consuming-reader and strict-cache accounting tests are kept, now on shared helpers. From the previous revision: 498 focused C++ cases passed (82 table/factory, 156 trie, and 260 trie DB), along with the C API test and generated C API verification. Coverage includes both comparators, copied-options rollback, C factory reconstruction, explicit modes, OPTIONS serialization, SST Get/iteration, point and range-only flushes, compaction, reopen, cache disabled/enabled/evicted, and legacy factory precedence. The rollback, empty-output, primary-selector, and OPTIONS regressions were checked failing before their fixes. The cache test measures file reads at the custom block's actual offset: one on a cold open, zero on cached reopens, and one after eviction or with caching disabled. Disabling custom-block cache reuse makes that assertion fail. The full suite and Linux/Windows/TSAN coverage are left to CI. Earlier full-suite runs had failures/timeouts (173/2095 jobs normally and 152/2088 with Status checking); that broad suite was not rerun for this commit. ## Series Previous: https://github.com/facebook/rocksdb/issues/14962 (landed through import). Next: https://github.com/facebook/rocksdb/issues/14965. Reviewed By: jaykorean Differential Revision: D122796629 Pulled By: xingbowang fbshipit-source-id: 4c9d6f3f548c588a37b08fd94de25e8c0b5ef743	2026-10-21 20:20:38 -07:00
Xingbo Wang	7454655969	Small fix and stress coverage for MANIFEST-bound OPTIONS recovery (#15431) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15431 A filesystem failure during column family creation can leave the MANIFEST and OPTIONS snapshots at different points in the column family lifecycle. Recovery must select the snapshot consistent with the MANIFEST without deleting a newer valid legacy snapshot. Fix: When the MANIFEST committed an older tracked OPTIONS file, recovery previously rejected the highest-numbered candidate without reading its tracking marker. Writable open then deleted a newer legacy file, or an unreadable highest file, as a rejected tracked orphan; read-only open also reported the older file number. Recovery now inspects every higher candidate. A newer legacy snapshot supersedes the older committed snapshot, while an unreadable higher candidate is retained rather than deleted. Add: Crash-test invocations randomize `track_options_file_number_in_manifest`. Before writable open can replace evidence, db_stress compares the public latest-OPTIONS choice with read-only recovery, using only currently live column family descriptors and running before requested missing families are appended. After open, it verifies the persisted tracking marker and live column family set. TransactionDB still skips the unsupported read-only probe, but it now reads the MANIFEST first so an existing commit enables the post-open verification. The regression test also checks that writable open preserves the newer legacy file before creating another column family. Change: Reopen MANIFEST-avoidance checks use the loaded `options_` value directly. This is a behavior-preserving simplification, not a separate correctness fix. Safety and limitations: There is no API, file-format, or production hot-path change. Ordinary writable open still receives the caller options and complete requested column family set. The extra read-only open is stress-only and runs when tracking verification is active. The pre-open comparison remains skipped for best-efforts recovery, TransactionDB, and stacked BlobDB; post-open verification remains skipped for best-efforts recovery, TTL, and stacked BlobDB. Random crash coverage is probabilistic, and this parent diff does not add randomized column family operations; the dependent lifecycle diff adds that profile. Reviewed By: pdillinger Differential Revision: D125029493 fbshipit-source-id: 5e291deff735e3cbe6d7df0513f940e84b20f387	2026-10-20 18:28:13 -07:00
Josh Kang	014e1c43c1	Apply readability fixes (isolate decls, braces) (#15394) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15394 ## Summary RocksDB's `.clang-tidy` enables several readability checks that existing code still violates: `readability-isolate-declaration` (476 findings), `readability-braces-around-statements` (49), `readability-redundant-string-init` (25), `modernize-use-using` (10), `misc-definitions-in-headers` (3) and `readability-duplicate-include` (1). They keep getting flagged whenever nearby lines change. This diff fixes 553 of the 564; upstream has since fixed the other 11. What changed: - Multi-variable declarations such as `std::string a, b;` are split into one declaration per variable. Pointer and reference declarators are kept per variable. - Braces are added around single-statement `if`/`else`/`for`/`while` bodies, and empty spin loops get an empty `{}` body. - `std::string s = ""` / `member("")` initializers are removed. - Remaining `typedef`s in the touched C++ files become `using`. C headers and `extern "C"` code are untouched. - `inline` is added to non-template function definitions in test headers. - A duplicate `#include "util/udt_util.h"` in `db_impl.cc` is removed. These are clang-tidy's own fix-its, applied mechanically and then clang-formatted on the changed lines. Three kinds of broken fix-it output were repaired by hand. Removing a `member("")` initializer left a dangling comma in constructor initializer lists (`ExternalSstFileInfo`, `ColumnFamilyMetaData` and others). The function-type typedefs in `block_test.cc` came out malformed. Empty `while (cond);` spin loops got a `{;` body. Splitting declarations also exposed two vectors in `db_etc2_test.cc` to `performance-inefficient-vector-operation`, so a `reserve()` was added for each. The lint bot reports every finding in a file a diff touches. This diff splits declarations in `options/configurable_test.cc`, so it also fixes that file's 14 older `cert-err58-cpp` and `cppcoreguidelines-avoid-non-const-global-variables` findings, which come from seven mutable file-level maps. Six are now `const` and built where they are used (function-local statics, or locals in their only user). The unused `imm_struct_option_info` is deleted. Compatibility: no behavior change. In public headers, the only edits are removed redundant `""` initializers, split declarations and added braces. Reviewed By: rban1 Differential Revision: D123186310 fbshipit-source-id: f9c8e88b00e997db0196f8725c7c8d880cf798da	2026-10-13 14:06:55 -07:00
Josh Kang	4b983da055	Spell out special member functions (rule of five) (#15387) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15387 ## Summary RocksDB's `.clang-tidy` enables `cppcoreguidelines-special-member-functions`, `cppcoreguidelines-virtual-class-destructor` and the noexcept-move checks, and clang warns `-Wdeprecated-copy-with-user-provided-dtor`. A full scan reports 476 findings. In most of them, a class declares a destructor (or copy operations) but leaves the rest of copy/move implicit. That has two costs. Readers can't tell whether a class may be copied or moved. And resource-owning classes keep an implicit copy that would double-free if anyone ever used it. This diff resolves 453 of the 476. Approach, picked per class from least to most invasive: 1. An empty user-provided destructor becomes `= default`. This covers most interface classes, such as `virtual ~X() {}` -> `= default`. If the class is still copied, that would only rename the deprecated-copy warning, so the destructor is removed instead when nothing needs it (`Customizable`, `TableFactory` and `InternalKeyComparator` inherit a virtual one; `IteratorWrapperBase` isn't polymorphic), and `MemTableRep::KeyComparator` gets defaulted copy and move. 2. Classes whose copy is already deleted get explicitly deleted move operations too. Move was already unavailable, so this only documents it. 3. Resource-owning classes that are never copied get deleted copy and move operations. Examples: the C API wrapper structs, which run a `destructor_` callback (edited in `tools/c_api_gen/c_base.cc` and regenerated into `db/c.cc`); `ShardedCache`, which frees its shards; iterator wrappers; and RAII handles. 4. Value types that are copied or moved get the missing operations as `= default`, which keeps their current behavior. 5. Exception specifications of existing functions are unchanged, except for `WriteBatch`'s move assignment (below): no other `noexcept` is added or removed. Only the move operations this diff newly declares (`Comparator`, `FileSampledStats` and the thread-local `Entry`) are `noexcept`; they copy only scalars or atomics. `WriteBatch`'s copy and move assignment destroyed `*this` and placement-new'ed a new `WriteBatch` over it. That is undefined behavior when `*this` is a subclass, and if the copy constructor threw, `*this` was left destroyed and was later destroyed again. Move assignment now assigns member-wise and ends in the same state as the move constructor, which leaves `has_key_with_ts_`, the timestamp-size tracking and the other fields it doesn't copy at their defaults. `rep_` is swapped through a temporary, because a plain string move-assignment can keep the old buffer (e.g. a large batch reset with `batch = WriteBatch()` would keep its memory). Nothing in it can throw, so it is now `noexcept`. Copy assignment copies into a temporary and moves it in, so a throwing copy leaves `*this` unchanged. Bugs this exposed and fixed: - `RestoreAfterCopyOrCreateWorkItem` (backup engine): the hand-written move constructor and assignment moved only some fields, so `from_file` and `to_file` were empty after `push_back(std::move(...))`. A restore that hit a checksum mismatch then reported "While restoring -> : ..." without file names. It now uses the compiler-generated moves, which are still `noexcept`. - `ExternalSstFileIngestionJob` lived in a `std::vector`, and its destructor frees raw-owned pointers, so the implicit copy used on vector reallocation would double-free. Its file batches also point into its own inline file storage, so neither a copy nor a move is safe after `Prepare()`. Copy and move are now deleted, and `DBImpl` holds jobs through `std::unique_ptr`. Compatibility: no public class loses an operation users can call. Public interfaces mostly only get `virtual ~X() = default;`. `WriteBatch`'s move assignment becomes `noexcept`, which is not part of the mangled name, so the ABI doesn't change. `SubcompactionJobInfo` and `CompactionJobInfo` intentionally stay C++20 aggregates, so users' aggregate and designated initializers keep compiling. `Configurable`, `CompareInterface`, `WriteBatchBase`, `ObjectLibrary::Entry` and `MemTableRep::KeyComparator` declare defaulted copy and move, and `Customizable` no longer declares a destructor. As a result, `Configurable` and `Customizable` subclasses that declare no destructor or copy operations of their own now move their registered-options list instead of copying it. In-tree, these are the mutable-options adapters, the table factories, the memory allocators and test classes; all are held through smart pointers or used in place, and `Clone()` copies, so none is moved by value. Not changed (23): `IODebugContext` (custom deep copy plus a `std::shared_mutex`), `DBWithColumnFamilies` in db_bench (stored in a `std::vector` with non-movable members), `Snapshot`'s protected virtual destructor (intended), `ManagedSnapshot`, the `SubcompactionJobInfo` and `CompactionJobInfo` aggregates, and the 16 noexcept-move-constructor and noexcept-swap findings on existing functions, including `autovector`'s move assignment (adding `noexcept` would change their exception specifications). Copying `ManagedSnapshot` already double-releases the snapshot. Fixing that needs a real move implementation and is a behavior change, so it is left for a separate diff. Reviewed By: rban1 Differential Revision: D123186309 fbshipit-source-id: 6f8e57e233e12199a007407130bb9a1f6d52e8d1	2026-10-12 16:00:09 -07:00
Xingbo Wang	baa4d373a6	Commit authoritative OPTIONS file numbers in MANIFEST (#15305) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15305 Why the change is required: Column-family topology is durable in MANIFEST, while its operational configuration snapshot is stored in a separate OPTIONS file. CreateColumnFamily and DropColumnFamily previously committed MANIFEST first and wrote OPTIONS afterward. An OPTIONS I/O failure could therefore make the API report failure after the topology change was already durable. Persisted-options readers also selected the highest file number and could consume an orphan left by a failed publication. Previous behavior: RocksDB had no durable correspondence between a MANIFEST state and the OPTIONS snapshot describing it. LoadLatestOptions and GetLatestOptionsFileName treated the highest-numbered OPTIONS file as authoritative. Column-family creation and deletion returned the later OPTIONS persistence error even when their MANIFEST edit had already succeeded. Multi-column-family operations wrote a complete OPTIONS snapshot after every successful member, producing quadratic OPTIONS bytes. Ordinary DB::Open consumes caller-supplied DB and column-family options rather than loading OPTIONS. This change preserves that contract. New behavior: Add the default-disabled experimental DB option track_options_file_number_in_manifest. Its value is serialized into each OPTIONS file so the selection rule is self-describing and participates in the normal DB-options compatibility checks. When enabled, RocksDB first durably creates and renames OPTIONS-N and then commits N with one safely ignorable MANIFEST field. A normal OPTIONS rotation uses one MANIFEST commit. Column-family creation and deletion prewrite the proposed snapshot and commit its number together with a topology edit in the same VersionEdit. A failure before the OPTIONS file is durable changes no topology. A failure after the file is durable but before MANIFEST commit leaves an orphan that binding-aware readers ignore, while the previous committed snapshot remains authoritative. Each multi-CF create or drop writes one proposed snapshot, independent of the batch size. Create commits the new number with the first successful add; drop commits it with the last successful drop. Existing partial-success semantics can therefore leave a committed snapshot with extra CFs, but never one missing a live MANIFEST CF. Persisted-options APIs filter this safe superset against recovered MANIFEST membership. A snapshot missing a live CF is corruption. DB::Open with create_missing_column_families uses the same batched path. If the newest OPTIONS file has tracking disabled, it retains the legacy highest-numbered-file behavior, including after an old or disabled writer has run. If it has tracking enabled, readers use the recovered MANIFEST commit and reject newer tracked orphans. A missing usable commit falls back to the highest tracked snapshot for RepairDB and legacy-rewrite recovery. DB open logs that fallback through its configured logger; logger-less utility APIs remain silent. Writable open removes rejected tracked orphans when local obsolete-file deletion is enabled. Read-only open never mutates the DB directory. A DB with no recovered commit and tracking disabled takes the legacy fast path without parsing the newest OPTIONS file. When inspection is required, RocksDB reads only the tracking marker. Marker-read failures used for selection do not fail DB open; a recovered commit remains authoritative and ambiguous newer candidates are conservatively retained. The first topology change after enabling tracking first binds the current topology, so failure of the proposed change cannot make its orphan authoritative. MANIFEST failure during an ordinary tracked OPTIONS rotation also enters the existing manifest-write background-error path. Implementation: - Add the safely ignorable CommittedOptionsFileNumber VersionEdit field and carry it through recovery, MANIFEST snapshots, rollover, and subset checkpoints. - Persist the tracking marker through C++, C, and Java options, generated round-trip coverage, compatibility checking, and db_stress configuration. - Centralize binding-aware selection for DB open and public persisted-options APIs. Reuse RecoveryContext's directory listing, use a narrow marker scan, and recover live CF names while replaying MANIFEST. - Filter manifest-selected safe-superset snapshots to live MANIFEST CFs in LoadLatestOptions and compatibility verification without discarding their option maps. - Prevalidate and sanitize CF batches outside the DB mutex, write one complete snapshot while options_mutex_ serializes topology changes, and hold the DB mutex only for the existing individual MANIFEST commits. Batch missing-CF creation during DB open follows the same locking boundary. - Keep the committed snapshot live in cleanup and live-file APIs, and make subset checkpoints write an OPTIONS snapshot and filtered MANIFEST with identical membership in tracked and legacy modes. - Preserve the first partial multi-CF drop error while consuming legacy OPTIONS persistence Status, and sync the checkpoint MANIFEST directory after replacement. - Pass checkpoint helper callbacks by reference and group VersionEdit storage by alignment so the new paths add neither callback copies nor avoidable object padding. Safety boundaries and unsupported cases: - Tracking defaults to false. Disabled writers and never-tracked databases retain legacy write ordering and highest-numbered selection. - Older binaries safely ignore the new MANIFEST tag and continue choosing the highest OPTIONS number. Downgrade is readable but not crash-atomic: an old binary can consume a tracked orphan. That is the same class of OPTIONS/MANIFEST inconsistency as the previous behavior. - A rollback binary must know the serialized option name before the feature is enabled. Exact compatibility checks intentionally report a mismatch when the caller's tracking setting differs from the persisted snapshot. - A newer untracked OPTIONS file deliberately supersedes an older committed number, allowing an old or disabled writer to become authoritative without rewriting MANIFEST. - Reactive secondary and follower instances do not enforce or repair the binding. Ordinary DB::Open still uses caller-supplied options. - Writable open does not delete rejected tracked or ambiguous files when a compaction service owns OPTIONS-file retention. Read-only open performs no repair writes. - Direct consumers that scan OPTIONS filenames themselves are unchanged; the binding is enforced by RocksDB's persisted-options APIs. - If the committed snapshot is missing and only tracked candidates remain, selection intentionally falls back to the highest candidate so RepairDB output remains usable. Validation coverage and known limitations: Focused coverage exercises encoding compatibility, tracked and legacy selection, the never-tracked open fast path, marker-read failures, exact tracking-marker compatibility, activation and disable/re-enable transitions, batched create/drop and create-missing behavior, partial-batch safe supersets, OPTIONS/MANIFEST failure injection, background-error propagation, orphan cleanup and retention, read-only non-mutation, subset-checkpoint behavior, stale OPTIONS recovery, RepairDB option sanitization, and transient read-corruption reconstruction. The assertion-enabled full executable suite passes. The repository check-format and check-sources wrappers cannot run because they invoke Git inside this Sapling checkout; fbsource lint is used instead. Cross-platform CI is not part of local validation. Reviewed By: pdillinger Differential Revision: D123104845 fbshipit-source-id: 77d94c989243c3f3f9fedfa81fa364c166250303	2026-10-11 10:39:30 -07:00
Josh Kang	da82001d81	Mark overriding methods and destructors override (#15366) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15366 ## Summary RocksDB's `.clang-tidy` enables `modernize-use-override`, and clang reports `-Winconsistent-missing-destructor-override` when a class marks its other overrides `override` but not its destructor. A full scan finds 190 `modernize-use-override`, 100 `inconsistent-missing-destructor-override` and 2 `suggest-override` findings in existing code. Each one comes back whenever a nearby line is edited. They also leave classes where it isn't obvious which methods are overrides and which introduce new virtuals. This diff marks every overriding member function and destructor `override` and drops the redundant `virtual` on them. It also changes `final override` to plain `final`, following the check's default. Methods that introduce a new virtual keep `virtual`. Most edits are clang-tidy's own fix-its, applied mechanically and then clang-formatted. Compatibility: no signatures change. In public headers the only effect is `override` on existing overrides (mostly destructors, plus `Cache::Name()` and `Cache::GetPrintableOptions()`, whose base declarations are unconditional), so user code that subclasses these types is unaffected. Every touched class now marks all of its overrides consistently. This matters because a partially-marked class would make clang emit `-Winconsistent-missing-override` in user builds. Reviewed By: rban1 Differential Revision: D123186308 fbshipit-source-id: 1f489ea32d938e3ec0faf9abc4f7ff49e042d37a	2026-10-09 14:39:14 -07:00
Ranjan Banerjee	dec67990f0	Add db_stress coverage for WriteBufferManager flush policies (#15048) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15048 Adds a validated db_stress flag for selecting the WriteBufferManager flush policy and randomizes it in db_crashtest. Crash-test generation covers all three policies; kFlushLargestAcrossDBs automatically uses two DBs sharing one manager so the cross-DB path is exercised. Reviewed By: pdillinger Differential Revision: D114638570 fbshipit-source-id: 00f664445614e083a4c59d4e2bb0896731d4bea7	2026-10-07 16:30:03 -07:00
Josh Kang	5b77e34985	Add range deletion support to external tables (#15334) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15334 External table formats cannot currently persist range tombstones, so SST ingestion and full-mode DB flushes or compactions fail when they contain `DeleteRange` entries. This change adds an opt-in contract for storing RocksDB’s canonical range-deletion block. RocksDB retains responsibility for sequence-number handling, fragmentation, and visibility through its native range-tombstone iterators. Existing factories remain unsupported by default, and existing files without range tombstones are unaffected. Missing or invalid advertised tombstone blocks are treated as corruption. RocksDB also supplies logical entry counts and sizes when a format does not persist the properties block, while preserving format-specific properties. Reviewed By: xingbowang Differential Revision: D123475827 fbshipit-source-id: de8fec401bdc311282f5d4a48b248875c682f7c9	2026-10-05 18:39:42 -07:00
Josh Kang	c0512c4c34	Remove deprecated write_global_seqno ingestion support (#15287) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15287 `IngestExternalFileOptions::write_global_seqno` has been deprecated since RocksDB 8.0 and will soon be removed from the API. It requires ingestion to mutate SST files, sync them, regenerate checksums, and expose file-offset plumbing solely for that mutation. Remove the option and its C, Java, and `ldb` APIs. Assigned sequence numbers now live only in the MANIFEST, leaving newly ingested SST bytes and whole-file checksums unchanged. ## Backward Compatibility New readers continue to support existing SSTs containing a nonzero global sequence number written by legacy ingestion. Because that old write did not update the properties-block checksum, checksum verification retries against a temporary copy with only the global-seqno value restored to its original zero placeholder. The parsed sequence number and file contents remain unchanged. This fallback only accepts mismatches explained by the legacy rewrite. Other checksum failures continue through normal corruption handling, and `verify_checksums=false` continues to bypass verification. Reviewed By: xingbowang Differential Revision: D122405856 fbshipit-source-id: 934e9bc755d58ea8d84e1c085e9769015bd8dfd7	2026-10-05 09:53:52 -07:00
Xingbo Wang	31675ca6d0	Recover from poisoned WAL handles on remote file systems (#15193) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15193 Keep `DB::Write`'s existing immediate `IOError` contract while allowing RocksDB to recover safely when a remote file system reports that a WAL handle is permanently unusable. The public API documentation makes the persisted-state boundary, reopen behavior, and safe post-resumption retry point explicit. Normal operation remains unchanged; the additional protocol is entered only after a supported file-scoped WAL error. ### Motivation A WAL write crosses an acknowledgement boundary: storage can accept the complete record and then fail before the caller receives confirmation. The resulting `IOError` therefore cannot tell RocksDB or the application whether the record is absent, partial, or complete. At the same time, the remote file handle can become permanently poisoned, so retrying that same handle can never succeed. The previous recovery path retried the poisoned handle. Simply replacing it is not sufficient: RocksDB must also prevent reuse of an ambiguous sequence range, keep every column family on one durable boundary, and prevent WAL tailers from publishing a physically complete record that successful live resumption deliberately abandoned. The new protocol quarantines the affected WALs, reserves the ambiguous sequence range, creates a replacement WAL, atomically persists acknowledged state, retires the quarantined handles, and only then resumes writes. ### Behavior changes #### 1. Recovering from a poisoned remote WAL handle - **Why required:** A remote file system can permanently poison a WAL handle after an I/O failure. Retrying that handle cannot succeed and leaves the database write-stopped. - **Previous behavior:** RocksDB retried the same WAL handle during recovery. - **New behavior:** The remote file system marks the failure as file-scoped. RocksDB creates a replacement WAL, atomically persists all state that could depend on the failed WAL, and then removes the quarantined WAL handles from active use. Physical file deletion remains part of normal cleanup. #### 2. Preserving an indeterminate WAL write outcome - **Why required:** A WAL record can reach storage before the append returns `IOError`; neither RocksDB nor the application can know the outcome from that error alone. - **Previous behavior:** Recovery could reuse a file-scoped failed write group's sequence numbers, allowing a later acknowledged write to overlap a record that might already be in the WAL. The public write API did not explain the acknowledgement ambiguity or distinguish live resumption from close/reopen recovery. - **New behavior:** `DB::Write` still returns the original `IOError` immediately and does not retry the logical batch. Normal DB reads can continue, but the failed write is not visible in that DB session. The persisted DB remains indeterminate with respect to the failed write until supported file-scoped live resumption succeeds without closing or reopening the DB. On reopen, or for any other WAL error without the durable-abandonment guarantee, applications must resolve consistency within RocksDB's documented guarantees, including `WALRecoveryMode`, through idempotency, deduplication, reconciliation, or an equivalent mechanism. After successful supported file-scoped live resumption, RocksDB has durably abandoned the failed batch and reserved its sequence range; retrying the same logical write is then recommended to preserve one linear logical history. #### 3. Keeping TransactionLogIterator consistent with live resumption - **Why required:** A physically complete error-returned record can remain readable from its WAL even though supported file-scoped recovery excludes that batch from the database. Returning it from `GetUpdatesSince()` would make replication or change-data-capture consumers diverge from the recovered DB. - **Previous behavior:** An iterator opened before recovery could return the abandoned batch. A new iterator could also return it while the physical WAL remained visible. - **New behavior:** For file-scoped failures, RocksDB records the exact sequence range and WAL number of each indeterminate write. For the lifetime of that open DB, existing and new WAL iterators stop with `Status::NotFound` before the range, whether the physical record is complete or absent. A caller can begin a new iterator after the reserved range. Iterator creation snapshots the recovery-gap generation together with its WAL-file view so a failure on a later, unseen WAL returns `TryAgain` for the old iterator instead of skipping its valid prefix. #### 4. Keeping all column families on one recovery boundary - **Why required:** A write batch can update multiple column families. Retiring its WAL after independently persisting only some column families can expose a partially applied batch after reopen. - **Previous behavior:** With `atomic_flush=false`, recovery could persist column families independently before abandoning the WAL. - **New behavior:** File-scoped WAL recovery force-flushes every live column family atomically and verifies that each one advanced beyond the failed WAL before writers are retired or writes resume. #### 5. Merging concurrent WAL failures - **Why required:** Another WAL write can fail while an earlier recovery attempt temporarily releases the DB mutex. - **Previous behavior:** A later failed WAL cutoff or ambiguous sequence could be overwritten or cleared by the earlier recovery attempt. - **New behavior:** Each error episode monotonically merges the largest failed WAL cutoff and the sequence range of the latest ambiguous write group. Recovery clears the quarantine cutoff only if no concurrent failure extended it; otherwise the newer failure remains write-stopping for the next attempt. Iterator fencing tracks the actual ambiguous write range separately, so an older failed `SyncWAL()` and a newer ambiguous write are not incorrectly paired. #### 6. Retaining file scope through `NoSpace` recovery - **Why required:** `NoSpace` can still identify one permanently unusable WAL handle. - **Previous behavior:** The `SstFileManager` recovery path entered generic recovery without carrying the failed WAL context. - **New behavior:** File-scoped `NoSpace` carries the same cutoff and sequence evidence through the `SstFileManager` path as other file-scoped WAL errors. #### 7. Scoping recovery context to one error episode - **Why required:** A completed recovery must not affect a later unrelated background error. - **Previous behavior:** WAL recovery context could remain latched after `ClearBGError` succeeded. - **New behavior:** Successful background-error clearing resets the context. A failed recovery retains it so a later retry has the same safety information. The separate iterator gap record remains for the lifetime of the open DB because later WAL readers must continue to reject the abandoned physical record. #### 8. Preserving successful `SyncWAL` metadata - **Why required:** A healthy WAL can finish syncing while a separate recovery error already exists. - **Previous behavior:** `SyncWAL` could return the existing background error before applying the successful WAL's `VersionEdit`, dropping its MANIFEST update. - **New behavior:** `SyncWAL` skips quarantined writers, applies metadata for successfully synced healthy WALs, and only then returns any existing recovery error. #### 9. Classifying only the intended remote-file-system failure - **Why required:** File-scoped recovery is valid only for an actual WAL `IOError` from a handle whose ownership is still valid. - **Previous behavior:** Poison state could be queried before ownership fencing was resolved, and file scope could be stamped onto a non-I/O status. - **New behavior:** Ownership fencing is resolved first, poison state is queried afterward, and only an actual `IOError` can be classified as file-scoped. `IOFenced` remains fatal. #### 10. Preserving `paranoid_checks=false` retry behavior - **Why required:** Non-paranoid mode historically cleared a writer's sticky error so a later operation could retry after storage recovered. - **Previous behavior:** Avoiding reclassification of a cached synthetic writer error also bypassed `WALIOStatusCheck`, so later `SyncWAL` calls kept returning the stale error. - **New behavior:** A cached error still fails the current `SyncWAL` call, but the non-paranoid path resets the writer for a later retry without setting a background error or entering file-scoped recovery. #### 11. Preserving async WAL precreation across recovery - **Why required:** A precreated WAL is an unpublished replacement and is not the active WAL. Recovery can also consume the prepared slot while background work is stopped. - **Previous behavior:** A file-scoped failure while creating or starting the unpublished replacement was attributed to the active WAL, unnecessarily quarantining it. After successful recovery consumed a prepared WAL, the next slot was not replenished, so the following rotation created its WAL synchronously. - **New behavior:** Only an active WAL `WriteBuffer()` failure attaches the active-WAL cutoff. A failed unpublished candidate is discarded while the current WAL remains active, and successful recovery schedules the next async precreation after `ClearBGError()` re-enables background work. ### Implementation - Carry `DBRecoverContext` through generic, `NoSpace`, and `SstFileManager` recovery, merging WAL cutoffs and failed write-group sequence ranges independently of error severity. Generic errors retain the range only so a later retry that establishes file scope can activate the stronger protocol; they do not persist a reservation or install an iterator gap on their own. - Document the WAL acknowledgement ambiguity, persisted-state boundary, reopen behavior, application consistency requirements, and post-resumption retry guidance on the public `DB::Write` API. - Publish a lock-free quarantine cutoff used by write and sync paths; replace even an empty failed current WAL; persist file-scoped ambiguous sequence reservations; force and verify an atomic all-column-family flush; and detach quarantined writers only after that durable boundary. - Record each file-scoped ambiguous write group's exact WAL number and sequence range in a DB-lifetime tracker shared by `WalManager` and all `WalIteratorImpl` instances. Stop an iterator before a batch that contains or crosses the range, and use a generation snapshot to preserve the existing `TryAgain` behavior when an iterator's fixed WAL-file view predates the failure. - Keep successful healthy-WAL tracking edits in `SyncWAL`, reset completed recovery context, and preserve legacy non-paranoid writer reset behavior. - Distinguish failures on an unpublished replacement WAL from failures flushing the active WAL. Log the failed replacement identity, keep the active WAL out of quarantine, and replenish the async-precreation slot after successful recovery. - Classify poisoned remote-file-system handles after ownership fencing and only for `IOError`. - Add lifecycle logging and a state-machine comment covering quarantine, sequence reservation, iterator fencing, replacement, atomic persistence, retirement, and concurrent-error handoff. - Add an opt-in `db_stress` mode that injects file-scoped faults only into WAL writes, disables WAL tracking in that mode, validates unsupported direct invocations, and sanitizes crash-test parameters to the supported recovery envelope. A small fraction of ordinary non-specialized crash-test runs select this mode automatically; existing stress behavior is unchanged when the flag is off. - Make the concurrent-error regression test hold the initial background flush until `SyncWAL` records the intended older-WAL error, preventing scheduler-dependent retirement of the test WAL before fault injection. ### Safety boundaries and unsupported cases - Automatic file-scoped WAL recovery fails closed with `NotSupported` for manual WAL flush, 2PC/transactions, two write queues, pipelined or unordered writes, WAL recycling, WAL TTL or size retention, either WAL-tracking mode, or blob direct writes. - Unsupported recovery leaves the DB write-stopped. `DB::Resume()` returns `NotSupported`; after correcting the I/O condition, operational recovery requires close and reopen. - The failed logical batch is never retried. Only supported file-scoped recovery durably reserves its sequence range and fences `GetUpdatesSince()`. Generic WAL errors retain the existing indeterminate outcome and can replay after close/reopen; applications must use idempotency or reconciliation unless successful file-scoped live resumption has provided the stronger guarantee. - Failed recovery attempts retain their context. Only a successful `ClearBGError` resets it. - Physical WAL deletion is not part of the recovery correctness boundary; normal obsolete-file cleanup removes files later. - The TransactionLogIterator ambiguity fence is kept for the lifetime of the DB instance that observed the file-scoped error and is not persisted as new MANIFEST metadata. After a later close/reopen, normal WAL recovery and obsolete-WAL retention/deletion semantics apply. - The in-memory iterator-gap tracker can grow with the number of recovered file-scoped failures during one DB lifetime. Safe pruning must account for retained WALs and existing iterator snapshots; it is left for follow-up. - Async WAL precreation remains disabled with WAL recycling. A prepared WAL is unpublished until its start records succeed; failure before installation does not make the active WAL file-scoped. - The new stress mode is test-only and defaults off, so normal `db_stress` and database operation without an error are unchanged. - Automatic stress-profile selection is limited to fully parsed, supported configurations. Explicit targeted runs reject options files and other unsupported combinations. ### Tests and known limitations - The affected recovery, WAL-manager, WAL-iterator, fault-injection, async-WAL-precreation, and secondary-close binaries build successfully. - Checked-status suites pass for all 70 `error_handler_fs_test` cases, all 7 `wal_manager_test` cases, all 11 `db_log_iter_test` cases, all 8 `fault_injection_fs_test` cases, and all 5 `DBWALTest.AsyncWalPrecreate*` cases. `DBSecondaryTest.SecondaryCloseFiles` passes 20 consecutive repetitions. - Focused regressions cover file-scoped gap fencing after an iterator snapshot, generic errors not installing a file-scoped sequence gap, and later recovery discovering that a generic error is file-scoped. - Earlier validation on this diff also passed crash-test unit coverage, remote-file-system adapter tests, repeated concurrency tests, two bounded multi-column-family stress runs, and bounded black-box crash/reopen dry runs. - The exact internal RocksDB format check and `arc f --take-formatters --enforce-lint-clean` report no formatting or lint issues. - The full crash/stress matrix and TSAN were not run. Reviewed By: pdillinger Differential Revision: D119324616 fbshipit-source-id: 05779d2bff7018f0ae69c4cbbe5b6455a45675d6	2026-09-30 04:40:00 -07:00
Josh Kang	176d7c7ea1	Add full external table support and crash testing (#15271) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15271 ## Summary Extend the experimental external-table API with a full mode that preserves RocksDB internal keys, sequence numbers, and entry types. Unlike the existing ingestion-oriented mode, full external tables can represent normal flush and compaction output and participate in live databases with puts, merges, deletions, snapshots, `Get`, `MultiGet`, iteration, multiple LSM levels, and external-file ingestion. The existing `ExternalTable*` interfaces remain aliases for the original sequence-zero, Put-only mode, preserving compatibility for current implementations. ## Implementation details - Introduce `ExternalTableMode` and mode-specific reader and factory interfaces. Full-mode builders and iterators receive complete internal keys, while the original mode continues to receive user keys. - Adapt full-mode point lookups to RocksDB’s `GetContext`, allowing readers to return multiple versions of a user key in internal-key order so RocksDB can resolve snapshots, merges, and deletions correctly. - Add a batched `MultiGet` context that preserves per-request lookup state and maps external-reader results back to RocksDB’s original request range. - Translate iterator seek boundaries and returned keys while ensuring file-wide global sequence numbers are applied exactly once. - Track sequence-number bounds, deletion and merge counts, user-collected properties, file size, and unique IDs when building tables. Unsupported entries fail and abandon the builder. - Forward checksum verification to the external reader while retaining compatibility with implementations that do not override it. - Add a standalone test implementation shared by basic and full modes. It uses a simple length-prefixed file format with a RocksDB properties block and checksum, avoiding dependencies on native table implementations. ## Crash testing Add `use_full_external_table` support to `db_stress` and a dedicated blackbox `db_crashtest.py` profile. The profile exercises randomized reads, writes, merges, ingestion, flushes, compactions, snapshots, checksum verification, and crash recovery. The profile disables unsupported combinations, including range deletions, user timestamps, sequence-number time tracking, backward scans, multiple column families, backup reconstruction, remote compaction, dynamic table-factory replacement, randomized aggregate/all-table property probes, and global-sequence-number rewriting during ingestion. Because the simple reader materializes each open table in memory, the profile limits open files and generated file sizes to bound total database memory usage. Reviewed By: xingbowang Differential Revision: D120637416 fbshipit-source-id: aa2cbd06999e4325a234210d9c45987addd37550	2026-09-29 17:57:17 -07:00
Xingbo Wang	ac30c4d738	Add stress scan witness diagnostics (#15199) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15199 ## Why PrefixScan and MultiScan verification failures are difficult to reconstruct from the terminal assertion alone. The iterator options, bounds, seeks, preparation state, and immediately preceding movements are often the evidence needed to distinguish a database defect from an invalid test assumption. ## Why the existing recovery trace is not enough The expected-state trace exists to reconstruct the expected database state after recovery. It preserves ordered write batches and deliberately filters out reads and iterator seeks, then recovery consumes a sequence-number-selected prefix. Consequently it contains none of the scan-specific evidence needed here: read options, bounds, seek targets, iterator validity, lazy-value preparation, movements, or the exact mismatch state. Scan witnesses capture that bounded human-readable context only when a verification failure needs it; they are best effort, are never replayed, and do not alter the recovery trace or its failure semantics. ## Previous behavior The scan paths emitted generic failure output but did not retain a focused bounded witness of iterator setup and state transitions. Adding instrumentation naively would also format diagnostic strings on the normal path even when disabled and could create one file per failing operation. ## New behavior An opt-in bounded ScanWitnessLog records scan configuration and recent iterator events for PrefixScan and MultiScan variants. On a qualifying failure it writes the latest complete witness for that DB, PID, thread, and scan type. The operation ordinal remains in the file contents. Repeated failures atomically replace the fixed witness rather than creating unbounded files. ## Implementation The witness captures sanitized read options, input ranges, bounds, seeks, movement, PrepareValue results, iterator status, keys, value size and hash, and wide-column state. MultiScan tracks whether the current primary iterator value was actually prepared so failure snapshots report accurate state. Disabled witnesses avoid scan-label ownership, range formatting, expected-count formatting in release builds, and numeric iterator-label conversion. Witness writes share the per-thread diagnostic I/O-disable latch with breadcrumbs, write to a fixed temporary path, close, and rename over the final file. Entry counts are capped at 65,536. A formatting-only follow-up normalized all touched C++ lines with the repository clang-format rules. ## Safety and unsupported cases The feature is stress-tool-only and raw db_stress keeps it disabled by default. It observes public iterator state and does not change database contents, expected-state tracing, recovery replay, iterator decisions, or fault-injection outcomes. Diagnostic I/O remains best effort and cannot fail the workload. Files are bounded per retained PID, thread, and scan type. The witness covers the instrumented PrefixScan and MultiScan implementations only and is not a general RocksDB trace. Files are not fsynced and therefore are not a power-loss durability mechanism. ## Tests and limitations Validated with 47 tools/db_crashtest_test.py tests, exact-candidate debug and release db_stress builds, arc lint over the complete stack, oversized-entry rejection, and clean independent correctness reviews of the final integrated stack. make check-sources is unavailable because it invokes Git in this Sapling checkout; equivalent added-line hygiene checks pass. Only the newest complete witness per DB, PID, thread, and scan type is retained, so later failures intentionally replace earlier witnesses. ## Rebase update Rebased onto fbsource master dc59f986b964. The logical diff is unchanged; the rebased stack passes debug and release db_stress builds, all 47 db_crashtest Python tests, and arc lint. Reviewed By: jaykorean Differential Revision: D119160911 fbshipit-source-id: 0e9b040c3bbb482e0a3b1b11070a7c5cc519f060	2026-09-25 16:20:54 -07:00
Josh Kang	dbc2db5228	Make external table APIs more consistent with native tables (#15241) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15241 Native `TableFactory` implementations receive the RocksDB `WritableFileWriter` in `NewTableBuilder()` and `RandomAccessFileReader` in `NewTableReader()`. These wrappers provide buffering, checksums, rate limiting, statistics, listeners, tracing, and direct-I/O handling. External table implementations previously received a limited writable-file adapter and reopened readers from a path, bypassing parts of this wrapper logic. This change makes `WritableFileWriter` and `RandomAccessFileReader` implement `FSWritableFile` and `FSRandomAccessFile`, respectively, so the same RocksDB wrappers can be passed to external tables. `ExternalTableFactory::NewTableReader()` now receives the already-opened reader and file size. The filesystem interfaces expose standard append, flush, sync, fsync, file metadata, synchronous reads, asynchronous reads, and batched reads. `RandomAccessFileReader::SubmitReadAsync()` schedules its wrapped coroutine read on the configured filesystem read executor, and the DB table-cache and `SstFileReader` construction paths pass that filesystem through. `SstFileReader` also supplies its configured statistics, listeners, rate limiter, and file temperature, removing the need for a separate external-table metrics filesystem wrapper. For direct I/O, `MultiRead()` checks whether requests are already sorted. When alignment and merging are needed, it constructs a sorted index only for unsorted input, preserving caller request order and result mapping. Fully aligned requests using aligned caller buffers bypass this path and may be supplied in any order. Reviewed By: xingbowang Differential Revision: D121035539 fbshipit-source-id: 27cb897726c31cb1a4dc811c65c8d79e690d82b1	2026-09-24 11:46:13 -07:00
Jay Huh	ffd4ac31af	Make SecondaryDB multi-CF iterator reads non-blocking (#15259) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15259 Publish immutable SecondaryDB read views after open and successful catch-up so `NewIterators()` can acquire one cross-CF sequence and SuperVersion set without waiting for `DBImpl::mutex_`. Keep failed catch-ups on the previous view, reject refresh for batch iterators, and exercise concurrent catch-up plus batch scans in the CF-consistency stress profile. Reviewed By: xingbowang Differential Revision: D121318547 fbshipit-source-id: 3198f4766c90ab0bb39400963df3c329b4ecf6bf	2026-09-23 13:09:24 -07:00
Andrei Preda	507206e46b	Fix stale readahead trim prefix on SeekToFirst (#15216) ... Summary: `BlockBasedTableIterator::SeekImpl()` sets `seek_key_prefix_for_readahead_trimming_` only when given a seek key, and `ResetBlockCacheLookupVar()` does not clear it. After a `Seek()`, a `SeekToFirst()` on the same iterator trims readahead against the previous seek key's prefix. `DBIter::SeekToFirst()` under `prefix_same_as_start` takes its prefix from the first key found, so the old prefix does not describe the keys being scanned. With the stale prefix, `IsNextBlockOutOfReadaheadBound()` is true at every block instead of only at the prefix boundary, and each prefetch is clamped to the current block. The same predicate sets `is_index_out_of_bound_`, which `FindBlockForward()` converts to `is_out_of_bound_` and ends the scan. Under `ReadOptions::async_io` this drops keys. The bug needs `auto_readahead_size` and a preceding `Seek()` that reads no data block. A `Seek()` that lands on a key leaves the prefetch buffer anchored above the scan, and the trimming callback is not called. The bug does not occur under `allow_mmap_reads`, where `FilePrefetchBuffer` is constructed disabled. The fix clears the prefix in `SeekToFirst()`, so prefix-based trimming is disabled for that positioning. The `auto_readahead_size` documentation in `include/rocksdb/options.h` already says prefix-based trimming requires `Seek(key)`. On buffered reads with `prefix_same_as_start`, `auto_readahead_size` and `readahead_size=8192`, a cold-cache `SeekToFirst()` scan of 2M keys in a single 222 MB file, preceded on the same iterator by a `Seek()` past the last key, took 391 ms before and 308 ms after. The same scan with no preceding `Seek()` takes 307 ms. Under direct IO on a local disk it went from 33.1 s to 11.8 s. Data blocks read are unchanged; file reads drop by 3x. Also fixes a pre-existing test bug: `SeekParallelizationTestWithPosix` asserts on `number_async_seek` without resetting that cumulative thread-local, unlike its three siblings. The new tests move `prefetch_test` from 11 shards to 13, so different tests share a process, and the assertion fails at the parent commit under that grouping. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15216 Test Plan: New regression test in `PrefetchTrimReadaheadTestParam`, in sync and `async_io` variants over the fixture's three `index_shortening` modes. It runs two scans of the same keys from an equally cold block cache, differing only in a preceding `Seek()`. Reverting the `clear()` fails 3 of 6 sync parameterizations on `READAHEAD_TRIMMED` (6 vs 40) and 3 of 6 `async_io` parameterizations on key count (40 vs 1). `make check` and `ASSERT_STATUS_CHECKED=1 make check` show no new failures. `prefetch_test` 122/122, `db_iterator_test` 249/249, `table_test` 7629/7629, `block_based_table_reader_test` 5347/5347, `db_basic_test` 300/300. New cases ran 1200 times under `COERCE_CONTEXT_SWITCH=1` without failure. Added db_stress coverage for `SeekToFirst()` after `Seek()` under `prefix_same_as_start`. With the fix reverted and io_uring available, the configuration in the second commit fails 8 of 8 seeds with "Iterator diverged from control iterator"; with the fix, the same seeds pass. Reviewed By: rban1 Differential Revision: D120228466 Pulled By: xingbowang fbshipit-source-id: a4935f03a0080cc8dd4c7a8c1438404e9b8f5c3a	2026-09-21 11:28:32 -07:00
Xingbo Wang	8108b49e0c	Add db_stress operation breadcrumbs (#15194) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15194 ## Why Intermittent db_stress and db_crashtest failures often report only the terminal assertion, after the operations that established the failing state have scrolled out of logs. The diagnostics must retain the latest operations even when the stress process is killed before it can run a failure handler. ## Why the existing recovery trace is not enough The existing expected-state trace is a correctness oracle for recovery: after an expected-state checkpoint it records ordered write batches, and recovery replays the prefix implied by the database sequence number. Reads and iterator seeks are deliberately filtered out, trace-write failures are fatal, and the trace is machine-consumed rather than intended as a concise failure narrative. It therefore cannot show which read, scan, maintenance operation, or operation phase immediately preceded an assertion. Breadcrumbs fill that observability gap with a bounded, human-readable, per-thread history. They are best effort, never replayed, and remain independent of the recovery-validation contract. ## Previous behavior db_stress did not retain a bounded per-thread operation history. db_crashtest did not automatically provision a local directory for these artifacts. The first version of this diff kept the ring in process-local memory and periodically wrote a complete temp-file snapshot, which could lose every operation since the last snapshot when the process crashed. ## New behavior An opt-in per-thread memory-mapped breadcrumb ring records recent operation begin, context, and end events directly in a local file. Committed records therefore remain in the kernel page cache when only the db_stress process exits or is killed; no periodic flush is required. db_crashtest enables breadcrumbs by default, assigns a local diagnostics directory, retains the newest eight process groups after failed child runs, and removes managed artifacts after success. ## Implementation Each ThreadState lazily creates a uniquely named, preallocated file and maps it through Env::NewMemoryMappedFileBuffer(). The file contains a fixed header and 512-byte text records. A record slot is marked I before its body is replaced and V only after the complete body is published; E denotes an unused slot. Each valid record carries a monotonically increasing sequence number so wrapped records can be ordered. Oversized context is bounded and ends with details_truncated=1. Lazy initialization avoids empty files for auxiliary ThreadState objects, and a process-wide file ID prevents collisions between ThreadState objects that share a logical thread ID. The periodic-flush flag, temp-file/rename snapshot path, and verification-failure fallback flush are removed. Enabled operation-context formatting continues to reuse a per-thread string and uses std::to_chars for integers; disabled call sites avoid formatting. Diagnostic directory setup and mapping failures are reported but do not change the workload result. Auto-directory pruning and cleanup accept only generated filename schemas, reject symlink roots, preserve unrelated files and explicit user directories, and retain at most eight PID groups. ## Safety and unsupported cases This is stress-tool-only diagnostic behavior. Raw db_stress keeps breadcrumbs disabled by default. The expected-state recovery trace and scan-witness contracts are unchanged. The mapping deliberately does not use msync or fsync: it is intended to survive a process crash while the kernel remains running, not power loss, host reset, or kernel failure. Filesystems or Env implementations without shared writable mappings disable breadcrumbs non-fatally. Replacing or truncating a live mapped file is unsupported. Records are fixed width, so unusually large operation context is explicitly truncated. Concurrent live readers are not promised a consistent snapshot; post-termination readers must ignore I and E records and order V records by sequence number. Nested operation scopes remain unsupported. ## Tests and limitations Validated with all 47 tools/db_crashtest_test.py tests, exact integrated debug and release db_stress builds, complete-stack arc lint, lower/upper/combined clang-format checks, normal ring wrap, distinct files for four worker threads, an oversized 256-column-family context, and process-crash tests. A release process killed with SIGKILL retained all 16 committed ring records without an explicit flush. A debugger-forced SIGKILL after the I store but before V publication left the interrupted slot marked I. In five CPU-pinned alternating release pairs of 50,000 operations, enabled and disabled medians were both 2.15 seconds. This remains a bounded latest history rather than a complete log and provides no host-crash or power-loss durability. ## Rebase update Rebased onto fbsource master dc59f986b964. The logical diff is unchanged; the rebased stack passes debug and release db_stress builds, all 47 db_crashtest Python tests, and arc lint. ## Review follow-up The mapped-slot invalidation now uses an acquire-release atomic exchange so later body stores cannot become visible before the I marker on weakly ordered architectures. Reviewed By: anand1976 Differential Revision: D119160910 fbshipit-source-id: ffe172af9ebc75c617196592806dc9641a539a25	2026-09-17 13:01:13 -07:00
Xingbo Wang	4c2a977aa5	Track newer snapshot versions in RocksDB metadata reads (#15086) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15086 Why: An application built on top of RocksDB could use cache population reads through explicit snapshots and needs to know when the returned value has already been superseded, without a second point read. Previous behavior: Get and MultiGet returned the snapshot-visible value and optional timestamp but could not report a newer point update or covering range deletion. New behavior: experimental C++, C, and Java metadata-read APIs report newer committed puts, deletes, merges, and covering range tombstones; ordinary primary-DB writes completed before the read are guaranteed to be reported, racing writes remain best-effort, and true has no false positives. Implementation: metadata reads widen lookup behind read-callback hooks while keeping metadata-only state off passive common object layouts; per-key state stays in KeyContext/GetContext, ordinary MultiGet row-cache replay semantics are preserved, BlobDB/TTL use shared output helpers, dead single-CF plumbing and an invalid cross-key stress assertion are removed, and the range-tombstone ordering invariant is documented without changing behavior. Safety and unsupported cases: tracking is opt-in and requires an explicit snapshot; persisted-tier reads, custom transactional visibility callbacks, secondary DBs, and unsupported subclasses return NotSupported, while read-only/compacted DBs return false because their in-process views are static. Tests and limitations: 1,265 Buck tests plus focused metadata, 100x coerced-context-switch row-cache, write-committed/write-prepared crash tests, builds, and lint passed. Current-parent release db_bench found no material passive regression: readrandom +0.65%, MultiGet batch 4 -1.40%, and batch 32 +0.48%. Reviewed By: pdillinger Differential Revision: D104182397 fbshipit-source-id: 36f7be1cd50fe2383689adaae31a6bb7d7e462dc	2026-09-11 04:44:59 -07:00
Jay Huh	91b96f41cf	Retry the OpenAndCompact secondary open when the primary replaces CURRENT (#15179) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15179 A remote compaction worker (`DB::OpenAndCompact()`) opens the *live* primary's DB directory read-only, and that open reads `CURRENT`. The primary publishes each MANIFEST rotation by writing a new `CURRENT` under a temp name and renaming it over the old `CURRENT`. POSIX keeps an already-opened `CURRENT` readable after that replacement, but file systems without read-after-unlink semantics (remote or object stores) do not: the replaced object is reported as gone, surfacing in RocksDB as `IO error: No such file or directory` (`Status::PathNotFound`). The secondary open was not retried, so this transient race failed the whole compaction. There is an equivalent, already-recognized race one step later for the MANIFEST file (`ReactiveVersionSet::MaybeSwitchManifest()` converts it into `Status::TryAgain`), which was fatal here for the same reason. `DB::OpenAndCompact()` now retries the secondary open (up to 3 attempts) when it fails with `TryAgain` / `PathNotFound` / `NotFound` -- exactly the statuses a concurrent `CURRENT`/MANIFEST replacement produces. The retry limit is configurable via `OpenAndCompactOptions::max_secondary_open_retries`. Genuine failures are still returned after the bounded retries, and no safety check is weakened or skipped. Found by a `db_stress` crash test failure. Two non-default options in that run make this reachable and fatal: * `remote_compaction_worker_threads=8` with `remote_compaction_failure_fall_back_to_local=0` -- the latter was only recently randomized to 0; before that a remote compaction failure was always silently retried locally. Now `DbStressCompactionService::Wait()` maps the failure to `kFailure`, which becomes a sticky background error on the primary, so every subsequent `Flush()` returns it and `StressTest::ProcessStatus()` aborts with `assert(false)`. * `max_manifest_file_size=8192` -- the primary rotates MANIFEST (and therefore replaces `CURRENT`) constantly, which is what makes the race window hit in practice. Full chain: primary renames a new `CURRENT` over the old one -> the file system reports the replaced object as gone for the reader that already resolved it -> `GetCurrentManifestPath()` inside the worker's secondary open fails with `IO error: No such file or directory` -> `DB::OpenAndCompact()` returns that error with no retry -> `kFailure` (no local fallback) -> primary background error -> `Flush failed: IO error: No such file or directory: ... /CURRENT ...` -> assertion failure in `db_stress`. Reviewed By: joshkang97 Differential Revision: D118480192 fbshipit-source-id: a2618ee24e582e2e713af04464ee1e4038925ccb	2026-09-09 19:32:00 -07:00
Josh Kang	37234200b5	Handle benign io_uring stderr in crash tests (#15181) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15181 A crash test failed when `use_async_db_api=1` created read-executor workers on a host unable to allocate io_uring rings. Folly logged the setup failure and threw; RocksDB caught it and successfully fell back to synchronous reads, but the harness failed because stderr was non-empty. Representative stderr: ``` WARNING: Logging before InitGoogleLogging() is written to STDERR I0903 14:05:39.470695 286445 IoUringBackend.cpp:527] io_uring_queue_init_params(512,1024) failed errno = 0:"Success" 0x7d2e06295280 retrying with capacity = 512 E0903 14:05:39.470759 286445 IoUringBackend.cpp:537] io_uring_queue_init_params(512,512) failed ret = -12:"Unknown error -12" 0x7d2e06295280 ``` Initialize glog in the Folly-enabled OSS `db_stress` entry point and centralize known stderr sanitization. Only `IoUringBackend` ring-initialization failures are moved to stdout when the async DB API is enabled; existing expected post-SIGTERM errors use the same sanitizer. All unrelated stderr remains fatal. ___ Differential Revision: D118836521 fbshipit-source-id: c9ce05896f417c7723c5cd73d293fbb1cd14ee1e	2026-09-04 12:22:30 -07:00
Josh Kang	ac0f4ae69f	Isolate coroutine statistics across async reads (#15142) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15142 Coroutine statistics use TLS, but coroutine reads can interleave on the same executor thread. Previously, when a stats-enabled request suspended, it saved its counters without disabling the executor's TLS configuration. A stats-disabled request running next could inherit those enabled settings and collect statistics unexpectedly. Stats setup also lived in individual DB implementations, so wrapper early returns and future stackable DB implementations could bypass it. Move statistics ownership to the public `CoroDB` and callback-based async read boundaries, before virtual dispatch. Each operation captures its caller's configuration, enabled requests preserve their counters across suspensions, and every suspension or completion leaves executor TLS disabled. Because the scope wraps stackable DB dispatch, current and future `CoroStackableDB` implementations automatically inherit the correct behavior without adding their own stats reset or scope. For consistency between coroutine and callback-based reads, the experimental callback API now uses TLS for both configuring and consuming statistics. `AsyncCallback::OnComplete()` no longer receives context arguments; RocksDB publishes the completed per-operation counters to TLS before invoking it. ``` Before time --------------------------------------------------------------> stats-on A: onSet(enable A) -- work -- onUnset(save A) ...... onSet(A) executor TLS: [A enabled] ------------> [still enabled] ------> [A enabled] stats-off B: run ^ inherits A's enabled TLS After time --------------------------------------------------------------> stats-on A: onSet(enable A) -- work -- onUnset(save A, disable) ... onSet(A) executor TLS: [A enabled] ------------> [disabled] ----------------> [A enabled] stats-off B: run ^ remains stats-disabled ``` Reviewed By: xingbowang Differential Revision: D117017089 fbshipit-source-id: 8a85f7ba43e3b73225995f5d0b79da3892baafb9	2026-09-04 11:19:16 -07:00
Jay Huh	bec9dcec7c	Make db_stress actually tolerate transient remote IO errors (#15170) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15170 `warm_storage_tsan_liveness_crash_test` fails with ``` error : Get() returns IO error: Failed to read 1032 bytes from file ws://ws.zdb.ldc0zdbtest1/.../000093.sst from offset 5461, message [0 code=DEADLINE_EXCEEDED isRetryable=1 ... error=WSE_BLOCK_READ_TIMEOUT classification=TRANSIENT_ERROR ...] for key: 0000...78 (30280). Verification failed :( ``` The Warm Storage error is explicitly `classification=TRANSIENT_ERROR isRetryable=1`, and the DB artifact confirms there is no RocksDB problem: `000093.sst` is intact (37760 bytes, downloaded successfully after the run) and the DB `LOG` reaches `Shutdown complete` with zero IO-error or corruption entries. This is exactly the class of failure `--tolerate_non_injected_io_errors_for_remote_dbs` was added for (D112136438), but that mechanism never actually took effect. Two independent defects: **1. The triggering non-default option: `tolerate_non_injected_io_errors_for_remote_dbs=0` on a remote DB.** `db_crashtest.py` hard-coded the parameter to `0` in `default_params` and only ever *force-disabled* it (for local DBs), leaving each remote job to opt in on its own command line. `rocks/sandcastle/test-driver.sh`'s `warm_storage_*crash*` branch never passed it, so every Warm Storage crash-test job ran with the flag off and a single transient read timeout failed the run. Fix: the parameter now defaults to `1` whenever `--env_uri` / `--fs_uri` names a remote backend, so no remote job has to remember to opt in. A caller can still pass `0` explicitly, and `finalize_and_sanitize()` still force-disables it for local DBs so genuine local IO errors (e.g. io_uring failures) are never masked. **2. `injected_error_count == 0` made the flag a no-op in `no_batched_ops_stress.cc`.** All five read-path guards were `injected_error_count == 0 || !IsErrorInjectedAndRetryable(s)`. `IsErrorInjectedAndRetryable()` covers *non-injected* remote IO errors, which by definition have no injected fault to correlate with — and the `liveness` profile forces every `*_fault_one_in` to `0`, pinning `injected_error_count` to `0` for the whole run. So the left operand short-circuited to true and the error was reported as a verification failure regardless of the flag. Fix: extract `StressTest::IsTolerableNonInjectedRemoteIOError()` and add `StressTest::IsRetryableOperationError(injected_error_count, s)`, which gates the injected-error branch on `injected_error_count > 0` but lets a tolerated non-injected remote IO error through unconditionally. The five call sites now use the helper. With the tolerate flag off (all local runs), the new guard reduces algebraically to the old expression, so local behaviour is unchanged. The genuine consistency-check gate at `no_batched_ops_stress.cc:1146` (`do_consistency_check && injected_error_count == 0`) is intentionally left alone. Fixes T286575055. [Session trajectory link](https://www.internalfb.com/intern/devai/devmate/inspector/?id=9646c050-6791-4ac3-9abb-3ee110dcb54e) Reviewed By: pdillinger Differential Revision: D117886823 fbshipit-source-id: d7591472db8ee6e03c9d9f018480409b6d802838	2026-09-03 10:43:54 -07:00
Jay Huh	897162a6bc	Add session-scoped temporary directory for remote compaction (#15169) ... Summary: Remote compaction writes output SSTs into caller-selected job directories. If the primary crashes before installation or falls back to local compaction, those directories can outlive the DB session and accumulate under the DB directory. This change defines `kSessionTmpDirName` as `session_tmp`, the canonical name for a DB-local temporary directory whose contents have the lifetime of a DB session. The immutable boolean option `use_session_tmp_dir_for_remote_compaction` opts remote compaction into this directory. When enabled, an `OpenAndCompact()` directory name is resolved as `<dbname>/session_tmp/<job>`. During DB open, RocksDB creates the session temporary directory, completes recovery, and then synchronously deletes contents left by the previous DB session before scheduling compactions. `DestroyDB()` removes it as part of database deletion, and normal completion, failure, and cancellation paths clean the resolved per-job directory. The option defaults to false. Disabled callers retain the existing `<dbname>/<job>` layout and no session-directory cleanup. For rollout compatibility, legacy callers that already pass a DB-rooted output path are accepted unchanged only while the option is false. Directory creation and invalid-path errors are delegated to the configured filesystem rather than prevalidated by RocksDB. After D118301531 landed, per-job directory creation and resumption cleanup are kept together in `InitializeCompactionWorkspace()`. `OpenAndCompact()` resolves the client-provided name and expects DB open to have initialized the session temporary directory when the feature is enabled. The boolean is wired through C++ and generated C APIs, option serialization and reconstruction, `OpenAndCompact()`, ldb, db_bench, and db_stress. Tests cover path resolution, cross-session deletion, ownership boundaries, disabled behavior, `DestroyDB()`, configured compaction output, resumption fallback, and option round trips. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15169 Reviewed By: pdillinger Differential Revision: D118227112 fbshipit-source-id: 01aa834b0b3f75638102130a86d95d537a2797bd	2026-09-02 21:46:24 -07:00
Anand Ananthabhotla	80a5a6e4b2	Revert PR 15106 (Ignore OPTIONS files for manifest recovery advance) (#15158) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15158 Reverts `https://github.com/facebook/rocksdb/pull/15106`, which ignored `OPTIONS-*` and temp files when deciding whether recovery must advance `next_file_number_`. Crash-test failures show backup/checkpoint failing to copy the current `OPTIONS-*` file after options cleanup, because `DeleteObsoleteOptionsFiles()` ignores `next_file_number_` and `options_file_number_` when choosing which files to keep. The failing commands use `optimize_manifest_for_recovery=1`, so restore the previous conservative file-number reservation behavior while we investigate a narrower fix. Reviewed By: xingbowang Differential Revision: D117807052 fbshipit-source-id: 89f8656f045be4ac36d2ec851980573ed83370fe	2026-08-27 21:08:29 -07:00
Omkar Gawde	d699ffbd5c	Support partial atomic range replacement (#15154) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15154 **What**: Let `IngestExternalFileArg::atomic_replace_range` replace a half-open `[start, limit)` range that only partially overlaps existing SSTs, by committing a generated range tombstone alongside the replacement files. **How**: - Tombstone: `NumFilesToPrepare` reserves one extra file number and `PrepareAtomicReplaceRangeTombstone` writes the SST only when the pinned `Version` holds a straddling file (`HasPartialOverlap`), so the no-straddle case pays no extra write; run rescans and returns `TryAgain` if a straddler appeared since. On activation it is rotated to the front of `files_to_ingest_` so one `VersionEdit` lands `old < tombstone < data` seqnos, straddlers are retained, contained files metadata-deleted, and `generated_for_ingestion` keeps `move_files` from unlinking it. - Half-open plumbing: `RangesOverlapWithMemtables`, `RangeOverlapWithCompaction`, `OverlapProximalLevelOutputRange`, and `IsRangeOverlapped` take a required limit-exclusivity argument, and `largest_internal_key` now comes from `RangeTombstone::SerializeEndKey()`. `OverlapWithLevelIterator` stays inclusive: over-counting can only push the tombstone one level shallower, which is safe. - Eligibility: `SupportsAtomicReplaceRangeTombstone` requires universal compaction, a delete-range-capable table factory, `allow_global_seqno`, `!allow_db_generated_files`, `!snapshot_consistency`, and no UDT. `fail_if_not_bottommost_level=true` rejects partial overlap because the retained file forces the tombstone and replacements above Lmax. The `atomic_replace_range` doc comment and a new `unreleased_history/public_api_changes/` note state the half-open contract. - `ResetTableReader` passes `kNoFileChecksumFuncName` in `FileOptions` for external files, making "no checksum metadata" explicit rather than empty. - Stress: new `ingest_external_file_atomic_replace_one_in` gflag. `PickAtomicReplaceRange` picks a run of overwriteable keys strictly inside an existing SST so every replacement straddles, writes even-indexed keys and marks the interleaved odd-indexed keys deleted so the tombstone must clear the holes, and rolls back expected values on `TryAgain`. `db_stress_tool.cc` and `db_crashtest.py` gate the flag on the same eligibility set and force `acquire_snapshot_one_in=0`. **Why**: Atomic replacement previously required range bounds to align with SST bounds, and the public doc admitted the upper bound "may be interpreted as inclusive or exclusive". The tombstone overlay removes the alignment restriction and pins the contract to half-open without slowing the Nimble path or the no-straddle case. Reviewed By: pdillinger Differential Revision: D117155307 fbshipit-source-id: 65f45463e8082a7db7a9d44c8dc50debe95e36e7	2026-08-26 21:52:40 -07:00
Peter Dillinger	e4a7136d3e	Preferred-phase scheduling for periodic (time-based) compaction ... Summary: `periodic_compaction_seconds` softly guarantees every SST is rewritten at least once per interval. Across a fleet of similarly-configured DBs, many events line those intervals up in lockstep -- a shared enable, a coordinated restart (e.g. DC power-outage recovery), or just similar write history -- so the periodic compactions all come due together, a recurring "thundering herd" that is especially painful with remote compaction (SEV S698635). The obvious fixes trade one problem for another. Spreading the herd out means some files get compacted off their natural schedule, which is wasted work; and a clumsy spread just relocates the herd or spawns a new one. So the real goal is narrow: **de-herd while keeping "badness" -- extra and/or late compaction -- small, and without inducing fresh herding at any step.** This change adds an EXPERIMENTAL, opt-in mechanism that gives each `(DB, column family)` a stable, well-spread *preferred phase* within the interval and steers periodic compactions toward it. It is randomized and *uncoordinated* (DBs never talk to each other), which both keeps it simple and, as it turns out, de-herds far more cheaply than adding jitter to the interval. It is controlled by a single mutable DBOption and is **off by default**: - `periodic_compaction_phase_recovery_percent` (default `0` = disabled / kill switch; recommended `33`; clamped to `[0, 100]`) -- how aggressively each trigger converges toward the phase, as a percent of the remaining gap, only ever pulling the trigger *earlier*. `0` reproduces the classic scheduler exactly; `100` jumps to the phase in one cycle; lower values approach gradually with much less extra work. The preferred phase itself needs no tuning: it is derived from the DB ID (stable across re-open and physical cloning/migration, but distinct across DBs) and each column family is offset from the DB's base phase, so the CFs of one DB don't fire together. (An earlier draft exposed a `compaction_schedule_seed` string option for choosing the phase source; it was over-engineered for the one behavior anyone needed, and as a mutable string it also broke OPTIONS-file round-trip verification in crash testing. It has been removed in favor of always using the DB ID.) ## The two kinds of badness "Badness" here is the fraction of compaction work that is ideally unnecessary: over-compaction (rewriting a file earlier than the interval required) plus a penalty for lateness (going overdue past the interval). Steady state is 0; both "never compact" and "compact twice as often as needed" approach 1. De-herding inherently spends a little badness to spread load out; every design choice below exists to bound one of those two costs *without* re-clustering the fleet. The scenario names cited in the design bullets below (`upgrade_dynamic_r33`, `naive_upgrade_r25`, `burst_r*`, `turndown_r33`, `turnup_r33`, ...) all come from the out-of-tree simulator; see its results page (the "Simulator and visualization" section below, with an interactive plot per scenario) for the evidence behind each claim. ## Design - **A stable per-(DB, CF) preferred phase = one fixed attractor per DB.** Each DB gets a base phase from its DB ID; each CF is offset from it. Because the target is fixed (not re-rolled each cycle) and unique per DB, the fleet converges to a uniform spread and then compacts *on time* -- badness decays toward ~0. This is the crucial contrast with naive per-cycle jitter, which de-herds but never stops jittering and so pays a permanent over-compaction cost (see `naive_upgrade_r25`: flat ~12-14%, only diffusive spreading) vs. `upgrade_dynamic_r33`, whose badness decays to ~0 for the same anti-herding. - **Converge gradually, and only ever earlier.** Each trigger closes a `recovery_percent` *fraction* of the gap to the phase, reaching it over a few cycles rather than in one burst, and never later than `periodic_compaction_seconds` in steady state. A lower rate does less extra work, which matters when the phase keeps moving (write-driven / bursty workloads), where a high rate wastefully "chases" it -- the burst sweep shows `burst_r25 < burst_r33 < burst_r50 < burst_r100` in sustained badness. `33` is the default balance: quick spreading, low extra work. - **Fractional recovery (not "snap") is latency-tolerant and doesn't re-herd.** Closing a fraction of the gap gives a single stable convergence point, so real-world scheduling/queueing latency -- the delay between a file becoming due and its compaction actually running -- merely shifts the whole fleet uniformly instead of re-clustering it. A tried "snap onto the phase when in range" variant converged a bit faster with no latency but re-herded around the snap threshold under latency; it was rejected. Graceful degradation beat peak speed. - **Spread a suddenly-due cohort instead of bursting it (bounded lateness).** Enabling the feature, turning `periodic_compaction_seconds` down, or catching up after downtime can leave a whole cohort past due at the same instant. Firing it all at once would just be the herd again, so such a cohort is spread over the first quarter-interval (~N/4) after the phasing anchor on an *absolute*, epoch-aligned per-(DB, CF) grid. This trades a bounded lateness (<= ~N/4 past the deadline, so the interval is a *soft* upper bound during these transients) for anti-herding, and the absolute grid keeps a rapidly restarting DB from being perpetually re-deferred and starved. `turndown_r33` shows the past-due cohort smeared over N/4 rather than bursting; `turnup_r33` confirms a turn-up creates no past-due cohort (deadlines only grow) and so no burst. For immediate enforcement at such a transition, trigger a manual compaction. - **Off by default, with a kill switch.** Inert unless `periodic_compaction_phase_recovery_percent > 0`; at `0` behavior is identical to the classic scheduler. ## Implementation notes - Phase math is integer-only (`Hash64` + `FastRange64`); floating point is avoided for its pitfalls. The per-CF offset is a golden-ratio (Weyl/Kronecker) additive sequence -- low-discrepancy for any number of CFs. - Logic lives in `db/periodic_compaction_phaser.{h,cc}` (`PeriodicCompactionPhaser`), which owns the recovery percent and the phasing anchor and exposes the stateless trigger-time math; `VersionSet` holds one and caches the per-CF result on each `Version`. The anchor is (re)set to now on DB open, on a recovery-percent change, and on any `periodic_compaction_seconds` change -- the three events that can create a past-due cohort. - Plumbed through the C API, `db_bench`, `db_stress`, and `db_crashtest`. ## Simulator and visualization (out-of-tree, not part of this change) A standalone simulator drives 40 real RocksDB instances through a real periodic-task scheduler, records every compaction (periodic vs write-driven), and scores the badness metric above across ~17 scenarios: dynamic/restart enable, interval turn-down/turn-up via `SetOptions`, on-cadence writes, random write bursts with a recovery-rate sweep, and the naive-jitter baseline. It is the basis for the numbers cited above. Interactive HTML (**NOTE** fb-internal URL sanitization might require you to copy-paste the URL yourself): 5759c6a62d/pc_phase_sim.html Reviewed By: jaykorean Differential Revision: D117487278 fbshipit-source-id: a62439548b7dd6fe7dfdbbe21b69531d8b57a1e8	2026-08-26 20:14:26 -07:00
Peter Dillinger	8f492b2f47	Store common user-key prefix once per block (format_version 8) (#15117) ... Summary: Building on D116105558, this change finishes and publishes `format_version = 8` as production-ready. The headline feature is a tweaked block encoding that stores the common user-key prefix shared by all keys in a block **once**, in the block's leading bytes `[0, restarts[0])`, instead of repeating it at every restart point. This shrinks prefix-heavy blocks (and their block-cache footprint) and, for bytewise-ordered keys, speeds up `Seek` by comparing prefix-stripped suffixes. It is applied to **data blocks** and **every index tier** (leaf/partition index, the partitioned-index top-level, and the partitioned-filter top-level). Motivation: MyRocks/ZippyDB/timeseries-style keys carry a long shared prefix (table/index id, partition, etc.). Repeating it at each restart wastes space and cache, and every Seek re-compares those bytes. Profiles show index-block seek CPU is comparable to data-block seek CPU (partitioned index does two index seeks per data seek), so index blocks are a first-class target, not just data blocks. ### Format (self-describing, no new footer bit) - The common prefix occupies the block's leading bytes; a non-zero `restarts[0]` (the offset of the first entry, i.e. the prefix length) self-signals its presence. `restarts[0]` is 0 in every block that does not use the feature (all versions, all block types), so a single fv8 reader transparently handles any mix of stripped and unstripped blocks -- no footer bit is consumed. - Previous versions would misinterpret this encoding by quietly ignoring the common prefix, which (along with the lack of extensibility in data-block metadata) is why it is gated behind format_version 8. - Only restart-point entries change on disk; non-restart entries are byte identical to fv7, so the writer produces the exact same bytes regardless of implementation. ### Public option New `BlockBasedTableOptions::optimize_key_common_prefix` (`enum class OptimizeKeyCommonPrefix`), default `kIfFastSeek`: - `kDisabled` -- never use it. - `kIfFastSeek` (default) -- use it only where it also enables the Seek speedup: format_version >= 8 with a built-in (reverse-)bytewise comparator. - `kEnabled` -- use it for all comparators at fv8; (reverse-)bytewise get space + fast seek, other comparators get the space savings only (the reader reconstructs full keys and compares with the real comparator). Expressed as a criteria so it need not be toggled per CF based on the comparator (which can vary by CF). It additionally requires `use_delta_encoding` (the feature builds on delta encoding) and `ts_sz == 0`; when `use_delta_encoding` is false -- the setting that also preserves zero-copy key pinning -- it is a no-op. Default format_version is unchanged (still 7); the feature is opt-in via fv8. ### Scope: every index tier The reader handles all index-like blocks uniformly (a non-zero `restarts[0]` self-signals a stripped block regardless of block type), so the writer strips the common prefix from every tier: - **Data blocks** and **leaf / partition index blocks**: enabled. - **Partitioned-index top-level block**: enabled. Under the default `index_shortening = kShortenSeparatorsAndSuccessor` the file's last separator would otherwise be a short successor sharing no leading bytes, collapsing the single-block top-level's common prefix to 0. When common-prefix is on the builder suppresses that one successor (keeps the full last key -- i.e. `kShortenSeparators` behavior for that one entry), so the top-level strips uniformly under every shortening mode; the full last key is also a tighter file upper bound, so it is not worse. - **Partitioned-filter top-level index**: enabled (same uniform treatment). Its space/CPU effect is small at default partitioning (the top-level filter index is tiny) but grows with many small partitions -- a filter miss short-circuits at, and is dominated by, the top-level filter-index seek (see Performance). Included for a uniform on-disk surface across tiers. Bloom filter *contents* still hash full user keys, so prefix/whole-key filtering is unaffected. ### Implementation - Writer (`block_builder.{h,cc}`): keys are encoded incrementally in the fv7 layout while the running common prefix is tracked cheaply (reusing the delta `shared`); at `Finish()` only the restart-point entries are rewritten in place with the prefix stripped, and the prefix is written once at the block start. The rewrite is generalized to the value-delta (fv4) index layout used by index blocks (no `value_size` header; bulk-copy the inline BlockHandle and non-restart entries verbatim). - Reader (`block.{h,cc}`, `dbformat.h`): `Block` decodes the prefix from `restarts[0]`; `BlockIter::ParseNextKey` prepends it on a restart point (`shared == 0`). The suffix-seek helpers (strip target once, compare shortened suffixes) live in the templated `BlockIter` base and are shared by data and index iterators (binary search, templated on `DecodeKey`/`DecodeKeyV4`); for non-bytewise comparators (data) and the hash `PrefixSeek` refine step, the reader reconstructs the probed restart key and compares full keys. - Gating (`block_based_table_builder.cc`, `index_builder.{h,cc}`, `partitioned_filter_block.{h,cc}`): the criteria is threaded to the data-block builder and to every index tier -- leaf/partition index, the partitioned-index top-level (`use_common_prefix_top`), and the partitioned-filter top-level -- together with the last-separator successor suppression (`ShortenedIndexBuilder::GetSeparatorWithSeq`) that lets the top-level strip under the default shortening mode. The index base gate also requires `super_block_alignment_size == 0` (block alignment can trigger `skip_delta_encoding`, producing non-restart `shared == 0` entries the reader could not distinguish from stripped restart keys). - Memory: the per-block/per-iterator prefix is stored as a `uint32_t` length (the prefix is always at offset 0) rather than a full `Slice`, and `Block` / `BlockBuilder` members were repacked so the feature adds little or no struct size. - Java: a new `OptimizeKeyCommonPrefix` enum and a `BlockBasedTableConfig` getter/setter, plumbed through JNI in both directions (the `newTableFactoryHandle` writer and the options-reconstruction constructor). ### Whole-file (table-level) prefix: investigated and rejected A natural extension is to store the whole-file common prefix once per *file* (when an `SstPartitioner` guarantees every key in the file shares a fixed prefix) instead of once per *block* -- e.g. the MyRocks index/table id, or a future 128-bit global id. It was prototyped end-to-end (partitioner-provided prefix captured at file open, stored in a `user_key_common_prefix` table property, and stripped from every data/index block; the reader re-prepends it) and then dropped. The `user_key_common_prefix` table property reserved in the previous diff is dropped in this one. The rationale, in case it is revisited: - Marginal benefit over per-block. Per-block already strips the full shared prefix from every key; the only thing whole-file removes on top is the single per-block copy of that prefix, i.e. `~= prefix_len / block_size` of the uncompressed (block-cache) footprint, with no meaningful read-CPU change (per-block already delivers strip-once + suffix compares). Measured at prefix_len=12, 4 KB blocks this was ~-0.3% incremental space; at the 8-16 KB data blocks typical of the space-constrained target (MyRocks/UDB) it is ~0.1-0.2% even with a 16-byte global id -- a fraction of a percent, seen only by partitioner users. Small block-size workloads are becoming more niche, and even where they exist, they are also CPU-sensitive (see next paragraph). - Real, if small, CPU tax on the *per-block* path. Reconstructing a key from two non-contiguous parts (file prefix + block residual) cost ~0.5-1.0% additional retired instructions on the per-block read path (deterministic across gcc and clang), wall-clock-neutral in cache-resident tests but present even when no whole-file prefix is configured. Attempts to remove it did not pan out: a compile-time specialization of the reconstruction *increased* instructions (+2.9%) by defeating inlining; a per-block combined-prefix buffer trades the CPU cost for a memory cost on every (default) block; and a per-iterator buffer was the original larger regression. Only invasive iterator-level monomorphization would truly zero it, which is not justified on this hot path. - Decision. A fraction-of-a-percent, partitioner-only space win does not justify a permanent per-block-read-path CPU tax plus additional on-disk format surface (a table property that changes whole-file interpretation). Dropped. The generic `SstPartitioner::ShouldPartitionByPrefix` hook is kept (cheap, and useful independently), and the reserved `user_key_common_prefix` table property is removed since no writer will emit it. ### Performance (db_bench, release/native, cache-warm, best-of-3, pinned) Three arms: (A) fv7 on the base/main branch, (B) fv7 with this change (should be neutral / byte-identical writing), (C) fv8 with this change and `optimize_key_common_prefix=enabled` (the optimization is active). Partitioned index + filters; `compression_type=none`, so SST bytes are the uncompressed block-cache footprint. `int` = db_bench default keys (variation-early, near worst case); `pfxN` = an N-byte shared prefix + varying suffix (MyRocks-like). Setup: `num=3,000,000`, `reads=1,500,000`, single thread, `metadata_block_size=4096`, cache holds the working set, pinned to one core. | shape (key/val) | arm | SST bytes | readrandom us/op | seekrandom us/op | |---|---|---:|---:|---:| | int (32/8) | A fv7 main | 138,983,658 | 3.484 | 4.995 | | | B fv7 ours | 138,983,146 | 3.518 | 5.024 | | | C fv8 ours | 137,681,420 | 3.490 | 4.986 | | pfx24 (32/8) | A fv7 main | 71,697,531 | 3.417 | 5.561 | | | B fv7 ours | 71,698,043 | 3.448 | 5.579 | | | C fv8 ours | 65,578,236 | 3.284 | 5.386 | | pfx56 (64/16) | A fv7 main | 102,942,266 | 3.624 | 5.830 | | | B fv7 ours | 102,942,266 | 3.678 | 5.835 | | | C fv8 ours | 90,554,642 | 3.444 | 5.626 | fv8 vs fv7-main (C vs A) -- space / readrandom / seekrandom: - int (32/8): **-0.9%** / ~neutral / ~neutral - pfx24 (32/8): **-8.5%** / **-3.9%** / **-3.1%** - pfx56 (64/16): **-12.0%** / **-5.0%** / **-3.5%** fv7-ours vs fv7-main (B vs A): SST bytes byte-identical (within 512 B; the fv8 paths are gated off), read/seek us/op within run-to-run noise (~1%, best-of-3 on a shared host). Net: fv8 is a space **and** read/seek-CPU win on prefix-heavy data and neutral on variation-early (`int`) data, so `kIfFastSeek` is safe on by default at fv8. ### Space by tier (compression=none, num=3M, mdb=4096; fv7 -> fv8) Where the win comes from -- the index tiers strip the most: | shape | data | leaf/partition index | top-level index | filter | |---|---:|---:|---:|---:| | int | -0.8% | -29% | -45% | -0.2% | | pfx24 | -8.3% | -69% | -89% | -0.8% | | pfx56 | -11.1% | -83% | -96% | -1.6% | Data bytes dominate the absolute total, but every index tier is stripped: leaf/partition index -29% to -83% and the partitioned-index top-level -45% to -96%. Filter *blocks* barely move (-0.2% to -1.6%) because filter bits hash full user keys; only the small top-level filter index is stripped. ### Point-miss lookups (readmissing) A partitioned-filter miss short-circuits at the filter (no data-block read), so it is dominated by the top-level filter-index seek plus the partition bloom. At a typical `metadata_block_size=4096` (few partitions, tiny top-level filter index) fv8 is neutral on misses (int +0.9%, pfx24 -1.0%, pfx56 -0.7% us/op -- all noise). The top-level-filter stripping pays only with many small partitions: at `metadata_block_size=256` it is pfx24 1.166->1.138 (**-2.4%**) and pfx56 1.273->1.188 (**-6.7%**) us/op. ### Backward / forward compatibility - Default `format_version` remains 7; the feature is inert unless a CF opts into fv8. Older RocksDB rejects fv8 files at footer decode (fail-safe). - Within fv8, the stripped-block set a writer can emit is fixed and every fv8 reader can read it (the `restarts[0]` signal plus uniform reader handling), so the on-disk surface is self-consistent across builds/downgrade. ### Follow-ups (not in this change) - Suffix-aware interpolation / hash index seek (currently correct via the reconstruction fallback, not perf-tuned). Pull Request resolved: https://github.com/facebook/rocksdb/pull/15117 Test Plan: - New focused unit tests (`table_test`), run at the full `format_version` matrix including the now-supported fv8: `GeneralTableTest.OptimizeKeyCommonPrefix` (data-block space+read+Seek correctness: existing and missing keys straddling the prefix, before/after-all, SeekForPrev), `OptimizeKeyCommonPrefixNonBytewise` (reconstruction path), and `IndexBlockCommonPrefixEquivalence` across `{kBinarySearch, kBinarySearchWithFirstKey, kHashSearch, kTwoLevelIndexSearch}` x `{partitioned filters}` x `{bytewise, reverse-bytewise}` (so both top-level tiers -- partitioned-index and partitioned-filter -- are exercised), asserting identical reads and a shrunken fv8 index. - The randomized `ParameterizedHarnessTest` now runs at fv8 (reverse-bytewise, restart intervals `{1,16,1024}`, special/empty keys fuzzed against the feature). - `db_crashtest.py` randomizes `format_version` including 8 and `optimize_key_common_prefix` in {0,1,2}. - Removed the reserved-property carve-out: dropped the `ReservedUserKeyCommonPrefix` reader test and the C API getter for the property; C API regenerated via `tools/c_api_gen/` (`make regen-c-api`), and the intentional public-C-API removal is recorded in `tools/c_api_gen/api_compatibility_allowlist.json`. - Java: `BlockBasedTableConfigTest.optimizeKeyCommonPrefix` (default `kIfFastSeek`, plus set/get of every value). - `table_test` `FooterTests` updated for the now-published fv8 footer, whose metaindex-gap encoding stores the metaindex block size in 16 bits (the test caps that size for fv >= 8). - Manual verification: the db_bench runs in Performance above (three-arm space + read/seek CPU, per-tier byte breakdown via `sst_dump`, and `readmissing`); and `readrandom`/`seekrandom` with `--block_protection_bytes_per_key=8` find 100% of keys at both fv7 and fv8 (reconstructed keys pass per-key checksum). Reviewed By: xingbowang Differential Revision: D116365016 Pulled By: pdillinger fbshipit-source-id: 66f4b5c60ea4fe0ff48d34c98d191ba8ebdf0665	2026-08-20 17:32:41 -07:00
Peter Dillinger	e8554f1b11	Faster and safer MANIFEST recovery for remote compaction (OpenAndCompact) (#15133) ... Summary: Faster and safer MANIFEST recovery for remote compaction (OpenAndCompact) `DB::OpenAndCompact` (remote/distributed compaction) opens the target DB as a secondary and reconstructs the LSM state from the MANIFEST before running the requested compaction. Previously, point-in-time recovery would stat/open the SST (and blob) files referenced by the MANIFEST to classify them found/missing and materialize candidate versions, and eagerly load their table handlers. That is (a) a lot of unnecessary file operations and open latency, especially on remote/distributed storage, and (b) fragile: when a file referenced by the MANIFEST but not an input to the compaction was momentarily unavailable to the worker -- e.g. an obsolete file the primary had already deleted -- recovery could fail the job (surfacing as an incomplete "no valid result") or roll back to an earlier version, even though the compaction only needs its own input files, which the primary keeps referenced for the job's duration. This was seen as a flaky failure of CompactionServiceTest.BasicCompactions on aarch64, and reproduces on x86 under COERCE_CONTEXT_SWITCH. Efficiency + reliability: recovery for the OpenAndCompact path now trusts the MANIFEST. It reconstructs the current version from FileMetaData without stat-ing/opening files to classify them and without eagerly opening candidate-version table handlers, so it does far fewer file operations and never fails or rolls back over a file the compaction does not need. The compaction's input files are opened (and unique-id verified) on demand as it reads them. Correctness hardening: trusting the MANIFEST means the worker's own (possibly eventually-consistent) view of CURRENT/MANIFEST drives the reconstructed LSM shape, which feeds correctness-critical decisions (Compaction::IsBottommostLevel / KeyNotExistsBeyondOutputLevel). A worker that read a stale CURRENT (older MANIFEST), a truncated MANIFEST, or a MANIFEST with a corrupt/torn tail could reconstruct against an older LSM shape and silently drop keys that should be kept. To prevent this, the primary records the MANIFEST position (file number and size) it scheduled the compaction from and passes it in CompactionServiceInput as a floor. After recovery the worker compares the size of the maximal *valid* prefix it reconstructed -- which stops at any corrupt or torn record and at a partial atomic group at EOF, so tolerated tail garbage cannot inflate it -- against that floor, and refuses to compact against an older view (an older file number, or the same file number whose valid prefix is shorter), returning Status::Incomplete so the CompactionService can fall back to a local compaction. Accept-equal-or-later is safe by monotonicity: while the inputs stay locked (being_compacted), a later view can only have moved data further down, never resurrecting a shadowed value, so only an older view is unsafe. Single kill switch: both behaviors are bundled behind one signal and one option. Presence of the floor on the request (min_manifest_file_number != 0) couples on both trust-manifest recovery and the floor check in the worker; absence reverts the worker to the original point-in-time recovery with no floor check. A single mutable DBOption, remote_compaction_manifest_floor (default on), gates whether the primary populates the floor, so it is the one escape hatch that disables the entire new recovery path on a running DB via SetDBOptions if it ever misbehaves. The new CompactionServiceInput fields are wire-compatible in both directions (0 = not provided), so an old worker or an old primary simply behaves as if the switch were off. Follow-up: recovery is not yet strictly "open only the compaction's inputs". A limited number of non-input files are still opened by the bounded file-property sampling in Version::PrepareAppend -> UpdateAccumulatedStats (tolerating any that are missing), which today populates input FileMetaData::num_entries for compaction input-record-count verification. A natural follow-up is to record the needed per-file info (e.g. num_entries) in the MANIFEST so the worker can skip these auxiliary opens entirely; alternatively, source input num_entries from the worker's own read of its input table properties. Relatedly, the floor comparand is the recovered "maximal valid prefix" size, which today equals the MANIFEST file size but is defined to stay correct if a future MANIFEST format writes trailing padding/footer past the last record. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15133 Test Plan: Added tests in compaction_service_test.cc: - RemoteCompactionMissingLowerFileTrustsManifest: a non-input file referenced by the MANIFEST is unavailable during a remote compaction; the job completes with correct results (fails without trust-manifest recovery). The underlying race also reproduces under COERCE_CONTEXT_SWITCH. - RemoteCompactionManifestFloorRejectsStaleView: inflate the floor so the worker's recovered MANIFEST is below it; assert the worker rejects, the job falls back to local, and a below-output deleted key stays deleted (no silent resurrection). - RemoteCompactionManifestFloorSendsPosition: a non-zero floor is attached and the worker accepts its equal-or-later view. - RemoteCompactionManifestFloorKillSwitchOff: with the option off, no floor is sent, the worker uses the original recovery, and a transiently-missing non-input file makes the job fail/roll back (the escape hatch reverts behavior). - RemoteCompactionManifestFloorMutable: SetDBOptions flips the option; the floor stops being attached and the worker skips the check. options_settable_test covers the new mutable DBOption. Crash test: remote compaction is already exercised (db_crashtest randomly enables remote_compaction_worker_threads, driving DB::OpenAndCompact), so with remote_compaction_manifest_floor defaulting on, the new recovery path is covered by the crash test by default -- no new crash-test plumbing is needed. The kill switch is intentionally left un-randomized (always on): turning it off re-enables the original, occasionally-broken recovery, which would only add noisy failures for no benefit. UPDATE: crash test is not well covered because of `"remote_compaction_failure_fall_back_to_local": 1` which is masking other existing failures. Addressing this is a high-priority follow-up item. (Not practical to fix in this diff.) Reviewed By: jaykorean Differential Revision: D116738823 Pulled By: pdillinger fbshipit-source-id: c02e0a437d62fc526dbbfcf70edf43434dfc7c1f	2026-08-20 16:07:48 -07:00
Anand Ananthabhotla	4b41ccd2a7	Fix full_history_ts_low race for long-running snapshots in db_stress (#15129) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15129 When `persist_user_defined_timestamps=false` and `long_running_snapshots=1`, `AssertSame` in `MaybeReleaseSnapshots` reuses the timestamp saved at snapshot acquisition time. That timestamp can become stale because concurrent flushes advance `full_history_ts_low`, and long-running snapshots can be held much longer than the 10-second headroom added by `GetReadTimestamp()`. The snapshot validation is intended to check that a held snapshot still exposes the same sequence-number view. It should not require the acquisition-time read timestamp to remain valid after history below `full_history_ts_low` has been collapsed. The fix keeps the timestamped read API contract intact. Column families with user-defined timestamps still require `ReadOptions.timestamp`, even when timestamps are memtable-only. For `persist_user_defined_timestamps=false`, `AssertSame` now uses a fresh validation timestamp so the read remains above `full_history_ts_low` while the snapshot sequence number continues to pin data visibility. For persisted timestamps, it continues reusing the original timestamp to validate the exact timestamped snapshot view. Reviewed By: joshkang97 Differential Revision: D116110124 fbshipit-source-id: 9cf6cae6e7123f8453b7065565e9e2e0eb34321a	2026-08-20 12:32:59 -07:00
Xingbo Wang	d8536d3274	Enable continuous verification for secondary stress tests (#15114) ... Summary: PR https://github.com/facebook/rocksdb/issues/15066 fixed the bug that has caused this combination fails in the past. Re-enable this combination in the stress test. db_crashtest.py currently disables continuous_verification_interval whenever test_secondary=1. This change removes that quarantine and enables continuous verification for secondary DB stress tests. The disable predates auto_refresh_iterator_with_snapshot. When auto-refresh was later added, its stress-test integration mechanically added snapshot setup to several iterator paths, including the secondary- only NonBatchedOpsStressTest::ContinuouslyVerifyDb() path: snapshot = std::make_unique<ManagedSnapshot>(db_); read_opts.snapshot = snapshot->snapshot(); This creates a snapshot from the primary db_ and passes it to an iterator created from secondary_db_. That was never supported: a ReadOptions snapshot must belong to the DB being read, and secondary DB iterators explicitly reject non-null snapshots. The incompatibility went unnoticed because the existing sanitizer made this combination unreachable in recurring crash tests. It was therefore a stress-test integration bug, not evidence that snapshot- preserving auto-refresh was supported by secondary DBs. This change: - Stops forcing continuous_verification_interval=0 for secondary tests. - Avoids attaching a primary-owned snapshot to secondary iterators. - Continues exercising auto_refresh_iterator_with_snapshot through primary iterator paths. Secondary iterators still support ordinary Iterator::Refresh(), which advances them to the latest caught-up secondary state. Snapshot-preserving automatic refresh remains unsupported. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15114 Test Plan: - python3 tools/db_crashtest_test.py - make check-sources - AUTO_CLEAN=1 make -j192 db_stress - Targeted blackbox crash test with: - test_secondary=1 - continuous_verification_interval=1000 - auto_refresh_iterator_with_snapshot=1 Reviewed By: jaykorean Differential Revision: D116381630 Pulled By: xingbowang fbshipit-source-id: a162ab357a537c2e5047c1eda85d0c2138b56ed1	2026-08-19 11:34:01 -07:00
Anand Ananthabhotla	49faa51e11	Fix crash test Restore() failure with empty trace when replay_write_ops == 0 (#14840) ... Summary: `ExpectedState::Restore()` always attempted to open and replay the post-`saved_seqno_` trace. In the crash-test failure, recovery had already returned the DB exactly to `saved_seqno_`, so `replay_write_ops == 0` and the trace was legitimately empty. `FileTraceReader` reported `Status::Incomplete()` on that empty trace, which incorrectly failed restore. Skip the trace replay work when `replay_write_ops == 0`, but still run the normal state promotion, cleanup, and `saved_seqno_` reset path so later saves continue to work. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14840 Reviewed By: xingbowang Differential Revision: D107968797 fbshipit-source-id: b4024e1ff329aa3bd47f0307e67d9a4f224b66d4	2026-08-18 21:09:30 -07:00
Anand Ananthabhotla	7d18ffe316	Ignore OPTIONS files for manifest recovery advance (#15106) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15106 When `optimize_manifest_for_recovery` is enabled, `MaybeUpdateNextFileNumber` uses on-disk files to decide whether recovery must append a `SetNextFile` edit. `OPTIONS-*` and temp options files are written as sidecar option snapshots during open, after recovery, so their presence alone should not force MANIFEST growth on a later clean reopen. Exclude `kOptionsFile` and `kTempFile` from the trigger that requires a MANIFEST file-number advance. If any real file requires an advance, the code still advances past the largest observed file number, including OPTIONS files. Also tighten db_stress strict MANIFEST verification so fatal zero-growth checks only run when all fault injection is disabled. Open write/sync fault injection can legitimately make `reuse_manifest_on_open` fall back to a fresh MANIFEST. Reviewed By: xingbowang Differential Revision: D116092705 fbshipit-source-id: 65b57b0b7f57b2cef42efc4df353fb2218688d34	2026-08-18 17:53:22 -07:00
Peter Dillinger	495c21dc5c	Unify lazy wide-column blob read verify policy; tool coverage (#15115) ... Summary: Follow-ups to the lazy wide-column read API (DB::GetEntityLazy / MultiGetEntityLazy). Three related, low-risk changes under one theme; no public API change. 1. Internal read-API cleanup (retires TODO(lazy-blob-resolution-cleanup)). The lazy byte-range work had left a whole/range/force-verify method fan-out on the blob read path: VersionBlobFetcherBase::FetchBlobRange plus a separate FetchBlobForceVerify, and SameFileBlobReader::GetSameFileBlob paired with GetSameFileBlobRange, with the (verify_checksums, force_verify) decision duplicated across branches. This folds them into a single 3-valued BlobVerifyPolicy (kVerifyIfPresent / kVerifyIfNoAmplification / kSkip, added to db/blob/blob_constants.h) derived once at the resolver boundary and threaded down. FetchBlobRange and GetSameFileBlob now take a range descriptor (range_length == kWholeBlobLength means the whole value) plus the policy; FetchBlobForceVerify and the *Range twins are removed. The public LazyColumnReadRequest::force_verify bool is unchanged. The widely-called lower-layer whole-value primitives (BlobFileReader / BlobSource / Version GetBlob) are intentionally left as delegated primitives to bound blast radius; folding those is a later cleanup. 2. Embedded block-cache-tier quirk fix (retires TODO(lazy-blob-resolution-quirks)). ValidateEmbeddedBlobIndex rejected read_tier == kBlockCacheTier before the blob-cache probe, so a same-file (embedded) column already in the blob cache returned Incomplete under kBlockCacheTier -- unlike the separate-file path, which probes the cache first. The up-front rejection moves to the two no-cache paths (ResolveEmbeddedBlob / ResolveEmbeddedBlobPinned); the cache-probing paths now serve a cached record under kBlockCacheTier, matching separate-file behavior. This is a shared lower-layer change: the non-lazy Get / GetEntity and iterator embedded read paths (get_context / MaybeResolveEmbeddedValue -> ResolveEmbeddedBlobCached) get the same corrected behavior, not just the lazy resolver. 3. Coverage. db_bench gains a readrandomentitylazy benchmark (GetEntityLazy + MultiResolve of --lazy_entity_read_length bytes per column) for measuring byte-range (partial) blob read savings; it requires open_files == -1. The crash test now also fuzzes LazyColumnReadRequest::force_verify in the lazy-vs-eager differential (force_verify affects only verification, not the returned bytes, so the comparison is unchanged). This also sets up the next planned step -- cross-key coalescing for MultiGetEntityLazy / LazyWideColumnsBatch::MultiResolve. The internal BlobReadRequest already carries a {offset, len} range descriptor, and with the verify decision now a single BlobVerifyPolicy rather than a bool plus a separate force-verify method, a batched resolve can group a vector of range+policy requests per (CF, Version, blob file) and coalesce them onto the existing MultiGetBlob machinery -- without re-introducing whole/range/force-verify method twins in the multi-read path. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15115 Test Plan: - New unit test DBLazyEntityTest.EmbeddedBlockCacheTierServedFromCachedValue: a cached embedded column serves whole- and sub-range reads under kBlockCacheTier instead of returning Incomplete. Existing force_verify / BlockCacheTier tests are unchanged (public behavior preserved). - New unit test DBBlobIndexTest.EmbeddedBlobWideColumnGetEntityBlockCacheTierServesCached: covers the non-lazy GetEntity path -- a cached embedded column is served under kBlockCacheTier; the existing ...GetEntityBlockCacheTier test still asserts a genuine miss yields Incomplete. - Crash test: extended the existing lazy differential to fuzz force_verify (reuses --lazy_entity_read_one_in). - db_bench: readrandomentitylazy over 2000 entities x 2 uncompressed embedded blob columns (4000 B each), reading 100 B per column: | metric | value | |---|---| | rocksdb.blobdb.lazy.partial.read.count | 4000 | | rocksdb.blobdb.lazy.read.bytes | 400000 | | rocksdb.blobdb.lazy.partial.bytes.saved | 15600000 | i.e. 100 B read per column instead of the 4000 B whole-column value. Reviewed By: xingbowang Differential Revision: D116334969 Pulled By: pdillinger fbshipit-source-id: cebe90456b23be0f5be60cdc4bff85c8a37b5442	2026-08-17 17:06:32 -07:00
Anand Ananthabhotla	575b6df1b8	Force immediate creation of a new MANIFEST if the current one cannot be reused (#15103) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15103 The crash artifact showed `MANIFEST-000005` was physically well-formed but contained an incomplete atomic group that had been tolerated at EOF. `reuse_manifest_on_open` then appended ordinary records after that tail, turning the tolerated crash tail into mid-file `corrupted atomic group` corruption seen by OpenAndCompact and backup restore. Record whether MANIFEST recovery ended with a pending atomic group. When that marker is set, skip `ReopenManifestForAppend()` so the next manifest write creates a fresh MANIFEST instead of appending after the partial group. The existing physical-size guard remains as a separate torn-tail defense. This replaces the earlier keep-size preallocation hypothesis with the artifact-backed root cause. Reviewed By: xingbowang Differential Revision: D116062618 fbshipit-source-id: fb55ce2891a6fd50fa2c670dc720e91bad27e94d	2026-08-17 16:18:06 -07:00
Peter Dillinger	021374c62c	Start format_version=8 development toward new features (#15104) ... Summary: format_version background: SST files are shared across RocksDB versions. format_version is a critical tool in ensuring prior (and later) versioned readers do not misinterpret a file created by some version. Opt-in features can sometimes rely on previously-rejected corners of the schema space, but not always. This change: lands much of the format_version=8 (fv8) format surface as an internal-only draft -- readable and testable but not user-writable -- so upcoming features can build on a stable base. fv8 is NOT published: kLatestBbtFormatVersion stays 7, and reading or writing fv8 is gated behind the existing TEST-only TEST_AllowUnsupportedFormatVersion() hook. Publishing fv8 (raising the ceiling) is deferred to a later change. The pieces of fv8 added here, and what each unlocks: - Index value-delta escape. Existing behavior: index blocks (format_version >= 4) store each block handle as a delta against the previous entry's handle, and signal "this value is a full handle instead" by reusing the key sentinel `shared == 0`. Because the two are coupled, an index entry whose handle is not contiguous with the previous one must be forced to a restart, costing a full key as well as a full value. fv8 decouples them with an in-value escape codepoint: varint 0 means "a full BlockHandle follows," and genuine size deltas are remapped to zigzag(delta)+1 so that 0 stays free. A non-contiguous entry can then remain an ordinary key-delta-encoded, on-cadence entry. The escape applies to every index-shaped block, which includes the top-level index of index partitions and the index of filter partitions, not just the index over data blocks. Unlocks: the upcoming optimize_key_common_prefix index feature (removes its need to special-case super_block_alignment) and value-delta encoding for embedded-blob SSTs (previously disabled outright because interleaved blob records break handle contiguity). Guarded strictly on fv8; format_version <= 7 encoding and decoding are byte-for-byte unchanged. Note that restart points must not delta-encode, so they still have full values without a sentinel. - Data block footer bit 30 is reserved as an "extended metadata present" escape. Decode rejects a set bit as unsupported/corrupt at all versions. Unlocks: adding block-level metadata in the future through the one shared footer decoder, without another format_version bump or plumbing format_version into every block parser. (Older reader versions relied on this bit being zero for some integrity/consistency checking; format_version bump now makes it safe to use.) - Footer metaindex gap. For fv8, the top 16 bits of the footer's 4-byte metaindex-size field record the gap (in bytes) between the end of the metaindex block and the start of the footer; the low 16 bits hold the metaindex block size. Unlocks: a space very near the end of the file to save full file checksum state, for a "files carry their own checksums" feature that doesn't require SST parsing to extract the checksum states (to run them to completion on the trivial file tail). - Reserved "user_key_common_prefix" table property. The reader returns NotSupported when it is present and non-empty. Unlocks: a future whole-file optimization that stores the common prefix of all user keys once instead of in every restart key; until implemented, fv8 readers refuse such a file rather than mis-read it. - Because of the problem of inferring such a prefix at the start of constructing an SST file, we expect it will rely on SstPartitioner. Here we've added SstPartitioner::ShouldPartitionByPrefix() (not yet used) that will enable this optimization, and avoid the overhead of a virtual callback to SstPartitioner on every key written to an SST file. Supporting cleanups: the Footer's "unset" format_version sentinel is named (kInvalidFormatVersion) and the FormatVersionUses*() predicates assert against it, as each is a `version >= N` test that an unpopulated Footer would silently satisfy. Index block reader entry points (Block::NewIndexIterator() and friends) drop their defaulted trailing arguments so each caller states which codec it expects, and BlockBuilder::Add() takes delta_value by const reference rather than a pointer whose null case no caller used. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15104 Test Plan: New unit tests, all exercising fv8 through TEST_AllowUnsupportedFormatVersion(): - block_test: value-delta escape codec round-trip (delta 0, negative, large, and the escape; asserts format_version <= 7 bytes match the legacy encoding and a legacy 0x00 still decodes as delta 0), and rejection of footer bit 30 at all versions. - db_flush_test: fv7-vs-fv8 read equivalence for super_block_alignment, including the separate_key_value_in_data_block intersection, confirming padding events (hence escape entries) occur and fv8 index values are delta encoded. - sst_file_reader_test: fv7-vs-fv8 read equivalence for embedded-blob SSTs, confirming fv8 (unlike fv7) delta-encodes their index values and actually emits escape entries (counted through a sync point). - block_based_table_reader_test: a builder+reader test that injects a footer metaindex gap via a sync point (verifies the gap bytes are written and the reader locates the metaindex via the recorded gap, and that too large a gap surfaces as a builder error), and that a non-empty reserved user_key_common_prefix property makes the reader return NotSupported. - table_test: two-level index with partitioned filters and index_block_restart_interval > 1, the configuration in which both top-level indexes delta-encode their values; verifies scan and point lookup round trips at each tested format_version including fv8. - (Not strictly fv8) Basic SstPartitionerFixedPrefix tests in slice_test (ShouldPartition, ShouldPartitionByPrefix, their mutual consistency, and CanDoTrivialMove), which had no non-DB coverage before. fv8 is also added to the table_test harness (GenerateArgList) and the shared footer version set (test::kFooterFormatVersionsToTest), so the existing table-level and DB-level property/checksum suites now also run at fv8. And added to crash test (minor revisions to db_stress). ## Performance details Release db_bench (PORTABLE=0, -march=native) A/B/C on the read/write fast path, comparing (a) parent, (b) this commit @ fv7, (c) this commit @ fv8. All DBs on tmpfs, 20M keys, block_size=2048, compression=none. Note index_block_restart_ interval=16 (not the default 1) is required to actually exercise the fv7<->fv8 index value-delta codec -- with restart interval 1 every index entry is a full- handle restart and the changed path never runs (confirmed delta-value=1 via sst_dump). Reads single-thread + core-pinned, warm cache, interleaved/rotated rounds, paired deltas. Reads (readrandom + seekrandom, non-partitioned single ~1.9MB index block and partitioned index+filters): fv7->fv8 in the *same* binary is within noise on all four workloads (largest -1.0%, CIs cross zero). before->after (fv7 path) is also within noise; perf shows a deterministic +0.4-0.6% instructions/op from the added branches (the format_version>=8 value_delta_escape guard + refactored data-block footer decode), which is below the throughput noise floor. Writes (fillseq + compact total CPU): fv7->fv8 null. compact (which rewrites all index blocks) is null before->after. fillseq showed +3.5% CPU before->after, but perf attributes it entirely to cycles/IPC with *identical* instructions/op (+0.01-0.04%) -- a code-layout effect on the untouched memtable-insert loop, not this change. Sanity: any single throughput/CPU delta <~1% is within the build-to-build noise floor, measured directly by comparing two byte-identical binaries (swings up to ~0.9% reads; the pre-native/PORTABLE=1 build showed a spurious -1.85% that vanished under -march=native). Net: no measurable regression from fv8 draft; fv7 encoding/decoding byte-for-byte unchanged, only the noted sub-1% instruction bump on the shared read path. Reviewed By: xingbowang Differential Revision: D116105558 Pulled By: pdillinger fbshipit-source-id: ead776cdd396d497d47872c0956ea8c651c0d13f	2026-08-17 12:08:59 -07:00
Xingbo Wang	58d835d951	Add db_stress liveness profile and fix lost flush request (#14830) ... Summary: Add a db_crashtest liveness mode that runs db_stress with a watchdog for foreground progress. The profile disables fault injection and uses longer runs so failures are more likely to represent real forward-progress bugs instead of expected injected I/O errors. The watchdog tracks completed operations, active operation type/start time, and per-operation completion counts to distinguish stuck writes from long admin operations. Bug: The new mode exposed a latent flush scheduling bug. A flush request can be queued with an effective max memtable id, then a newer immutable memtable can be added before the background flush picks work. PickMemtablesToFlush() could pick only the older memtables and still clear flush_requested_. A later queued flush for the newer memtable would then be skipped because the column family no longer looked flush-pending, leaving the immutable memtable permanently unflushed. In stress this surfaced as IngestExternalFile() waiting in WaitForFlushMemTables(), with writes eventually blocked behind the same unflushed memtable. Fix: Keep flush_requested_ set until all unstarted immutable memtables have been picked. Add a direct MemTableList regression for the partial-pick case and a DB-level regression covering the ingest/get-live-files ordering that reproduced the lost request. ## Testing - CI - Local liveness mode 32 full 20-minute runs passed, 0 failures Pull Request resolved: https://github.com/facebook/rocksdb/pull/14830 Reviewed By: joshkang97 Differential Revision: D107895052 Pulled By: xingbowang fbshipit-source-id: ca67c5f9376899eefdc9af7bad4af06200b52446	2026-08-11 18:05:08 -07:00
Michael C Huang	276495843d	Add blob file writer buffer size option (#15061) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15061 Adds mutable ColumnFamilyOptions field `blob_file_writable_file_max_buffer_size` to cap blob-file WritableFileWriter buffers independently from DBOptions::writable_file_max_buffer_size. The default value is 0, which preserves existing behavior by inheriting the DB-wide writer buffer cap. Non-zero values apply to both flush/compaction blob writers and direct-write blob writers. Direct-write updates are propagated through DBImpl::SetOptions to newly opened blob files without requiring a DB restart. Also wires option parsing, dumping, db_bench, db_stress, crashtest, and release-note coverage. Reviewed By: xingbowang Differential Revision: D114979265 fbshipit-source-id: 3f764b797628eee0ce379110c1a1f8a16beb3fa2	2026-08-11 17:38:34 -07:00
Peter Dillinger	fcf382c95c	Integrated and ratio-aware compression auto-skip (#15079) ... Summary: Auto-skip compression (stop attempting compression on data that isn't compressing well, to save CPU) was previously an experimental CompressionManager wrapper (CreateAutoSkipCompressionManager) whose predictor lived in each per-thread working area. Besides being fragmented across parallel workers and reset on a tumbling window, it decided based on how OFTEN blocks failed to compress, ignoring how MUCH they compress -- so on mixed data it could give up substantial compression to save a little CPU (see performance results). This makes auto-skip a first-class, optional, ratio-aware feature of the block-based table builder, controlled by two new CompressionOptions fields: - auto_skip (default false): the on/off switch. There is no separate threshold knob; the decision reuses max_compressed_bytes_per_kb (the existing per-block "minimum compression worth keeping" bar) as the bar for whether attempting compression on the stream is worthwhile. - auto_skip_min_sample_every: sampling interval that bounds the local impact around a transition back to compressible data. 0 selects an internal default (currently 64 skipped data blocks between forced samples). The emit thread makes the skip/attempt decision once per data block (serial and parallel paths alike; data blocks only), so a skipped block is never dispatched to a compression worker. It is driven by a continuously updated, shared estimate q_hat of the aggregate stored/uncompressed ratio the builder would achieve by attempting -- each block counted at its achieved ratio, or 1.0 when stored raw: attempt while q_hat is at or below the bar, otherwise skip. Because the estimate is payoff-weighted, a stream whose blocks each save little -- or where only a few blocks compress well -- does not clear the bar and is skipped, while a stream that compresses substantially is attempted. The compression given up per KB is bounded by (1024 - max_compressed_bytes_per_kb). q_hat uses an asymmetric exponentially weighted moving average (EWMA), slightly faster toward better compression than toward worse; the fast side is deliberately gentle so a single marginally-compressible sample does not flip a whole file back to attempting. It is kept on its own cache line to avoid false sharing. Under parallel compression q_hat is updated with relaxed atomics, and the emit thread can decide from a value that lags the learning it drives by the in-flight depth -- tolerable concessions of accuracy and responsiveness for a coarse heuristic. The first data block of each file is always attempted (the sample countdown starts at 0), and while skipping the builder keeps sampling (one block after a randomized gap) so a return to compressible data is detected. q_hat is carried across the files a flush/compaction thread emits via thread_local state keyed by preferred compression type, so a sustained region does not re-learn on every file. The old CompressionManager-based AutoSkip is removed (CostAware is retained); db_stress, db_bench, and db_crashtest are migrated to the new option. db_bench is extended so it can generate the relevant regimes: --compressible_data_fraction and --compressible_data_run_length produce a mix of compressible/incompressible blocks -- i.i.d. per value (run length 1) or in alternating regions (large run length). The default (fraction 1.0) reproduces the historical single-pool behavior. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15079 Test Plan: New/updated unit tests in `util/compression_test.cc` (`DBAutoSkipTest`): - skips sustained incompressible data; - never skips compressible data; - disabled by default never bypasses a block; - 4-thread parallel compression round-trips all data across alternating compressible/incompressible regions. **Performance** (`db_bench fillseq`, release, zstd level 3; 100k values of 16 KiB each, roughly one value per data block; WAL and auto-compaction off; median of 5 runs on a quiet host). Auto-skip trades one cost for another, so each cell reports two costs, both lower-is-better: CPU cost as process CPU time (user+sys) per GB of uncompressed input (s/GB; ~1.64 GB uncompressed here), and storage cost as the resulting DB size. Auto-skip does not appear to change parallelizability, so wall-clock s/GB tracks CPU s/GB (the two agree here); CPU s/GB is reported. Percentages are relative to the no-auto-skip baseline: negative CPU = cheaper, positive size = more storage (shown only where storage grows). | Workload | baseline (off) | (a) old wrapper | (b) draft | (c) this version | |---|---|---|---|---| | Incompressible | 2.33 s/GB, 1568 MB | 0.96 (-59%), 1568 MB | 0.82 (-65%), 1568 MB | 0.82 (-65%), 1568 MB | | Compressible | 3.55 s/GB, 710 MB | 3.54 (~0%), 710 MB | 3.56 (~0%), 710 MB | 3.55 (~0%), 710 MB | | Marginal-payoff mix | 3.80 s/GB, 1376 MB | 1.54 (-59%), 1523 MB (+11%) | 3.81 (~0%), 1376 MB | 1.43 (-62%), 1531 MB (+11%) | | High-payoff mix | 2.20 s/GB, 929 MB | 1.14 (-48%), 1417 MB (+53%) | 2.16 (~0%), 929 MB | 2.17 (~0%), 929 MB | A strategy wins by cutting CPU cost without paying much storage cost. On incompressible data all three cut CPU ~60% at no storage cost; on compressible data none skip (within run-to-run noise). The two mixes (50% incompressible, 50% compressible per block) separate the strategies: - Marginal-payoff mix: the compressible half barely clears the per-block bar, so the stream is not worth compressing -- skipping is right. (c) and (a) cut CPU ~60% for ~11% more storage. That extra storage is compression (c) intentionally declines because its aggregate benefit is below the configured worthwhile threshold (max_compressed_bytes_per_kb, ~12.5% per KB); it stays within the (1024 - max_compressed_bytes_per_kb) bound. The ratio-unaware draft (b) cannot see the low payoff, keeps compressing, and saves no CPU. - High-payoff mix: the compressible half compresses hard, so compressing is right. (c) and (b) keep full compression at baseline storage; the old wrapper (a) cuts CPU 48% but inflates storage +53% -- far beyond that bound -- giving up worthwhile compression. Only (c) makes the right call on both mixes: it never gives up worthwhile compression (unlike (a)) and does not waste CPU on low-payoff data (unlike (b)). Reviewed By: xingbowang Differential Revision: D115444567 Pulled By: pdillinger fbshipit-source-id: 05e122d25de95ddcd0edeb0dd07c9d25bead7293	2026-08-11 14:29:51 -07:00
Xingbo Wang	66d7035708	Do not test GetEntityLazy on read-only primary handle (#15084) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15084 MaybeOpenReadOnlyOnPrimary is intended to exercise opening and closing a read-only DB on the primary live directory, then verify that read-only close does not delete files still live in the primary. D115278311 diagnosed T283967366 as a race between GetEntityLazy on that read-only handle and the primary deleting SST files. After checking the call path, the ENOENT is from GetEntityLazy's initial point lookup opening an SST through TableCache, not from deferred lazy column resolution. Other read APIs are not exercised on this read-only-on-primary handle, so adding GetEntityLazy there made this old open/close harness test also perform a live-directory point read. Remove the read-only-handle GetEntityLazy exercise and keep GetEntityLazy coverage in the normal stress read paths, consistent with the other read operations. Reviewed By: pdillinger Differential Revision: D115569968 fbshipit-source-id: fcef30d09903525c2a7897b9c52bc9ef951bed6a	2026-08-11 13:30:57 -07:00
Xingbo Wang	f8c601b91c	Fix io_activity assertion in stress test after MultiGetEntityLazy (#15076) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15076 Fix the `io_activity == Env::IOActivity::kUnknown || io_activity == options.io_activity` assertion failure in the crash test. The crash occurs because `TestMultiGetEntity` in `no_batched_ops_stress.cc` changes the thread operation to `OP_GETENTITY` during its consistency-check phase (the `check_results` lambda calls `SetThreadOperation(OP_GETENTITY)` before calling `GetEntity` for comparison). When `MaybeTestMultiGetEntityLazy` is called afterward, it invokes `MultiGetEntityLazy` which correctly sets `read_options.io_activity = kMultiGetEntity`, but the thread operation is still stale at `OP_GETENTITY`. The debug assertion in `DbStressRandomAccessFileWrapper::Read` then sees `TEST_GetExpectedIOActivity(OP_GETENTITY) = kGetEntity != kMultiGetEntity` and aborts. The fix restores `SetThreadOperation(OP_MULTIGETENTITY)` before each call to `MaybeTestMultiGetEntityLazy` in `TestMultiGetEntity`. Triggered by commit cb73fc845fe0 which added the `MaybeTestMultiGetEntityLazy` calls. The `--lazy_entity_read_one_in=100` flag (non-default) enables this code path. Reviewed By: pdillinger Differential Revision: D115021723 fbshipit-source-id: e3eed600db4986cab27d7b1924282b1dfb8b0a7a	2026-08-11 08:12:46 -07:00
Peter Dillinger	8e1569b351	Lazy wide-column byte-range (partial) blob reads (#15067) ... Summary: Builds on the lazy wide-column read API (DB::GetEntityLazy / DB::MultiGetEntityLazy) to serve true byte-range reads: resolving a sub-range of an uncompressed blob column now reads only the requested bytes from storage instead of fetching and slicing the whole column. This covers blobs in separate blob files and embedded (same-file) blobs in the SST, so a caller that knows a large value's layout can pull just the parts it needs. A partial read skips whole-record checksum verification (a strict sub-range cannot cover the checksum) and does not populate the blob cache (the cache entry holds the whole record). A compressed column, a whole-column read, a cache hit, or a read with LazyColumnReadRequest::force_verify still resolves (and verifies) the whole column; force_verify forces verification even when ReadOptions::verify_checksums is off. Mechanics: new range primitives mirror the whole-value read paths -- BlobFileReader::GetBlobRange, BlobSource::GetBlobRange, Version::GetBlobRange, and VersionBlobFetcherBase::FetchBlobRange for separate-file blobs; ReadSimpleGen2BlobRange, BlobSource::GetSimpleGen2BlobRange, and SameFileBlobReader::GetSameFileBlobRange (implemented by BlockBasedTable) for embedded blobs. ReadPathBlobResolver::ResolveColumnRange owns the partial-vs-whole decision and routes same-file references through the current SST's reader; the whole-column path also gained VersionBlobFetcherBase::FetchBlobForceVerify to honor force_verify. Observability: deferred resolve reads are attributed to a new Env::IOActivity::kLazyResolve (distinct from the kGetEntity / kMultiGetEntity of the initial entity read), and lazy resolution's storage reads are counted by new statistics rocksdb.blobdb.lazy.read.count / .read.bytes (all lazy reads) and rocksdb.blobdb.lazy.partial.read.count / .partial.bytes.saved (the byte-range subset and the bytes it avoided). The whole-column read sites gate recording on kLazyResolve so non-lazy reads are excluded. Also fixes a latent bug in LazyWideColumnsBatch::MultiResolve where an empty/default-constructed batch (null rep_) would accept a foreign standalone column (null owning_batch_rep_) through a nullptr == nullptr comparison instead of returning InvalidArgument. Follow-ups (in a local plan): unifying the blob-read API surface -- folding the whole/range method twins and the verify-policy flags (verify_checksums x force_verify) into one internal BlobReadRequest primitive (a range descriptor plus a 3-valued verify policy), which retires FetchBlobForceVerify and the whole/range method pairs; this is deferred for feature velocity and is not public-API affecting (the public force_verify bool stays). Marked in code with TODO(lazy-blob-resolution-cleanup) in db/blob/blob_fetcher.h and db/wide/read_path_blob_resolver.cc. Also cross-key coalescing (Phase 3) and async execution (Phase 4). Pull Request resolved: https://github.com/facebook/rocksdb/pull/15067 Test Plan: Extended db/wide/db_lazy_entity_test.cc: separate-file and embedded partial reads return the exact sub-span and read only the requested bytes; a partial read does not fill the blob cache and a cache hit slices with no I/O; a compressed column resolves whole and slices; force_verify does a full verified read and is detected via a tampered record even with verify_checksums off; the lazy statistics distinguish whole vs partial reads and are not touched by non-lazy reads; deferred reads carry kLazyResolve (verified with a FileSystem wrapper); and a regression test for the empty-batch ownership check. Added an IOActivityToString case in env/env_test.cc and registered the new tickers in the statistics name map. Crash test: extended the db_stress lazy-vs-eager differential (db_stress_tool/db_stress_test_base.cc) to fuzz per-column byte ranges. Each resolved column now randomly picks a whole-column read or a partial (offset, length) sub-range -- occasionally with an offset past the end or a length beyond the remainder to exercise clamping -- for both GetEntityLazy and MultiGetEntityLazy; when an eager reference is available the resolved bytes are compared against the same clamped sub-range of it. No new flags (reuses --lazy_entity_read_one_in); force_verify fuzzing remains a follow-up. Reviewed By: xingbowang Differential Revision: D115130388 Pulled By: pdillinger fbshipit-source-id: 454368dea8fae069d9de649c2c51611469d3c809	2026-08-07 12:08:03 -07:00
Darkhan Imangaliyev	5fe225357a	Extend parallel checkpointing and db_stress coverage for column-family-subset checkpoints (#15059) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15059 **What**: 1. Extends the parallel `CheckpointEngine` so it can also checkpoint a subset of column families. 2. Adds `db_stress` and `db_crashtest.py` coverage for subset checkpoints, over both the serial and the parallel API. **How**: - **Engine API.** `CreateCheckpointOptions` gains a `std::vector<ColumnFamilyHandle*> column_families` field (empty, the default, still means all column families). This goes on the existing per-operation options truct rather than adding a second pure virtual to `CheckpointEngineBase`: it avoids overload-resolution hazards with the defaulted `sequence_number_ptr`, and it is the pattern the engine API already uses. `CreateCheckpointOptions` has no users outside the engine API, so adding a field breaks nothing. - **Shared validation.** The handle checks that were inline in `CheckpointImpl::CreateCheckpoint` -- reject null handles, handles belonging to another DB, and handles for already-dropped families; coalesce ids; always add the default CF -- move to `static CheckpointImpl::ResolveIncludeColumnFamilyIds()`, now called by both APIs. Rejecting a foreign handle matters because `handle->GetID()` alone would silently coerce a foreign id onto whichever family in this DB happens to share it. Validation runs before any staging directory is created, so a rejected request leaves nothing behind. - **Almost no new plumbing.** `CheckpointImpl::CreateCheckpointImpl()` already accepted `include_cf_ids` and is shared by the serial and parallel movers, and the MANIFEST `kColumnFamilyDrop` rewrite runs after `mover->Finish()`, so it was already mover-agnostic. The engine change is just resolving the ids and forwarding them. - **db_stress.** New `subset_cf_checkpoint_one_in` knob (mirrors `parallel_checkpoint_one_in`): each checkpoint operation covers a random proper subset with probability 1/N. It is a no-op when the DB has only the default column family, and it is independent of `parallel_checkpoint_one_in`, so subset checkpoints get exercised through both the serial API and the engine. Subset selection includes each non-default family with probability 1/2 and then force-excludes one, so every subset checkpoint exercises the MANIFEST rewrite instead of degenerating into a whole-DB checkpoint; the default handle is passed in only half the time, covering the "default is always included whether or not you pass it" contract on both sides. - **db_stress validation.** `DB::ListColumnFamilies()` on the checkpoint must return exactly the requested set -- the direct check on the MANIFEST post-processing, catching both an excluded family that was not recorded as dropped and a requested family that went missing. The checkpoint is then opened read-only with exactly those descriptors, proving the reduced MANIFEST and reduced file set are self-consistent under the default `paranoid_checks`. The pre-existing per-key comparison against `SharedState::Exists()` still runs, restricted to the families actually present. - **Incompatibility.** `clear_column_family_one_in > 0` is rejected together with the new knob, the same way D113103899 did for `abort_and_resume_cf_compactions_one_in`: a subset checkpoint hands live handles from `column_families_` to the API and validates against `column_family_names_`, and neither is stable while families are dropped and recreated underneath. `db_crashtest.py` already pins `clear_column_family_one_in` to 0. - **Test cleanup.** The subset tests shared a lot of setup and verification, so `PopulateFourCfsForSubsetCheckpoint()`, `VerifyFourCfsIntact()` and `VerifyDefaultAndTwoSubsetCheckpoint()` were extracted into the test file's anonymous namespace and are reused by the pre-existing `CheckpointSubsetOfCF` as well as the four new engine tests. This extends `StressTest::TestCheckpoint` with a mode rather than adding a separate operation, which is how `parallel_checkpoint_one_in` was integrated. Reviewed By: rban1 Differential Revision: D114832267 fbshipit-source-id: f3f843819614477497cc1466910547922cafc51d	2026-08-06 16:17:04 -07:00
Peter Dillinger	fd7399004f	Add lazy wide-column read API (GetEntityLazy / MultiGetEntityLazy) (#15012) ... Summary: Adds an experimental **lazy wide-column read API** that lets callers enumerate a wide-column entity and read back only the columns (and, in a later phase, the byte ranges) they actually need, instead of eagerly materializing every blob-backed column like `GetEntity()` / `MultiGetEntity()`. New public entry points (`include/rocksdb/db.h`): - `DB::GetEntityLazy(read_options, cf, key, LazyWideColumns* result)` - `DB::MultiGetEntityLazy(read_options, cf, num_keys, keys, LazyWideColumnsBatch* result, statuses, sorted_input)` Both leave a wide-column entity's **inline** columns materialized up front but its **blob-backed** columns as *unresolved references*, so blobs for columns that are never pulled are never read. ### Result types (`include/rocksdb/lazy_wide_columns.h`) - **`LazyWideColumn`** — the lazy analogue of `WideColumn`: `name()`, `index()`, `inline_value()` (an `OptSlice`; empty ⇒ unresolved reference), `is_reference()`, and `logical_size()` (`std::optional<uint64_t>`; known for inline and uncompressed references, empty for compressed ones). All no-I/O. - **`LazyWideColumns`** — a move-only result that is **indexable and iterable like a vector** of `LazyWideColumn` (`size()`, `operator[]`, range `begin/end`). Unlike `PinnableWideColumns`, it may be used after the producing call returns: it **pins the SuperVersion** (like an iterator) so referenced blob files / SST readers stay valid for deferred reads. - **`LazyWideColumnsBatch`** — the `MultiGetEntityLazy` result; vector-like over its per-key `LazyWideColumns`. ### Resolution - **`LazyColumnReadRequest { const LazyWideColumn* column; uint64_t offset; size_t length; bool force_verify; PinnableSlice* result; Status* status; }`** — a read names a **typed column pointer**, not an untyped index. Because a column identifies its owning result (and, for a batch, its owning entity), `MultiResolve` routes each read correctly and **rejects a null or foreign column** with `InvalidArgument` on that read's per-read status — cross-result misuse can't be silently mis-indexed. - **`LazyWideColumns::MultiResolve` / `LazyWideColumnsBatch::MultiResolve`** take the same request type (pointer+count and `std::vector` sugar overloads); the batch form resolves reads spanning any entities of the batch in one call. - **`ResolveColumn(column, result)` / `ResolveColumnRange(column, offset, length, result)`** — per-column sugar over a one-entry `MultiResolve`. - A column's blob is fetched **at most once** over the result's lifetime (bytes are cached in the result), so repeated reads of the same column reuse that fetch. Resolution honors the `ReadOptions` captured at `GetEntityLazy()` time (e.g. `read_tier`, `verify_checksums`); the resolve methods intentionally take no per-resolve `ReadOptions`. ### Where it works - Primary, **read-only**, and **secondary** DB instances (each `GetImpl` override forwards the lazy signal and transfers a SuperVersion pin via the shared `DBImpl::TransferSuperVersionPin` helper). - Forwarded through `StackableDB` wrappers (e.g. `TransactionDB`) rather than falling through to the `NotSupported` base default. - Requires `max_open_files == -1` (immortal table readers) so same-file ("embedded") references stay resolvable lazily; otherwise the call returns `InvalidArgument`. ## Implementation notes - Read path: `GetContext` gains a lazy mode that leaves blob references unresolved and captures the SST's `SameFileBlobReader`; `Version::Get` forwards the new (defaulted) `lazy_columns_same_file_reader` out-param. - `ReadPathBlobResolver` performs on-demand, cached blob resolution bound to the pinned `Version` + decoded blob references. - Internal identity is by heap-stable `Rep` pointers, so column/result/batch pointers stay valid across moves with no move-time fix-ups. ## Scope / current limitations - Resolves **whole columns**; byte ranges are served by slicing the whole column for now (so `length`/`offset`/`force_verify` don't yet save I/O or skip checksums). Single column family. Synchronous. `kBlockCacheTier` on the originating call yields `Incomplete` on a blob-cache miss. - User-defined timestamps: entities aren't writable under UDT, so `GetEntityLazy` is at parity with `GetEntity` (same read-timestamp validation; plain values return a single default column). - Planned follow-ups: partial (byte-range) blob reads that skip checksum/cache- fill, cross-key coalescing, and asynchronous execution. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15012 Test Plan: - **`db/wide/db_lazy_entity_test.cc` (15 tests, all passing):** enumeration with no blob I/O; single range and whole-column resolution; `MultiResolve` of many blobs and mixed ranges/columns (incl. the `std::vector` overload); cross-key batch resolution; result-outlives-call / pin survives move; unpulled columns never read; `max_open_files == -1` requirement; `kBlockCacheTier` ⇒ `Incomplete`; foreign/null column ⇒ `InvalidArgument` (single-entity and batch); UDT parity with `GetEntity`; `StackableDB` forwarding; and read-only and secondary instances. - **db_stress:** `GetEntityLazy` / `MultiGetEntityLazy` run as a differential against eager `GetEntity` / `MultiGetEntity` (gated by `--lazy_entity_read_one_in`, which only takes effect under `--open_files=-1`), including coverage on read-only (`--open_read_only_one_in`) and secondary (`--test_secondary`) instances. Smoke-run (`EXIT=0`, "Verification successful") with integrated blob files + `--use_put_entity_one_in`. - `make format-auto` clean; builds `ASSERT_STATUS_CHECKED`-clean. Reviewed By: xingbowang Differential Revision: D113792365 Pulled By: pdillinger fbshipit-source-id: cb73fc845fe0e20ed352d697e552e7b47b2eb9cf	2026-08-05 22:21:40 -07:00
Josh Kang	d1fbdffdae	Log failed Status values in db_stress assertions (#15054) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15054 `db_stress` often terminates on raw `assert(status.ok())` checks, which discard the underlying error and make crash-test failures difficult to diagnose. Log the expression and full status before terminating. A message variant preserves operation-specific context and variadic formatting. In debug builds, assert after logging to retain assertion diagnostics and stack traces. In optimized builds, where assertions are disabled, exit with status 1. Reviewed By: jaykorean Differential Revision: D114756034 fbshipit-source-id: 5a0d4409bac69820b6560c8200cc401b7bfc65d6	2026-08-05 19:51:14 -07:00
Josh Kang	dc4006480b	Tolerate InvalidArgument from full_history_ts_low race when persist_user_defined_timestamps=false (#15052) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15052 When `persist_user_defined_timestamps=false`, `full_history_ts_low` is automatically advanced by flushes to just above the newest timestamp in the flushed memtable. This creates the following race: ``` Reader Concurrent writer / flush ------ ------------------------- T0: Sample read timestamp: 892779959679761 T1: Write with a newer timestamp: 892779963289806 T2: Flush the memtable. T3: Advance full_history_ts_low: 892779963289807 T4: Validate the read: 892779959679761 < 892779963289807 Result: InvalidArgument ``` This is expected RocksDB behavior because history below `full_history_ts_low` is no longer guaranteed to be complete. However, the stress test intends these operations to read the latest value. This change adds ten seconds of headroom to read timestamps when user-defined timestamps are not persisted. This makes it substantially less likely that a concurrent flush overtakes a read timestamp. Unlike tolerating every `InvalidArgument`, this preserves reporting for genuine timestamp errors. Persisted read timestamps and write timestamps remain unchanged. The triggering non-default options are `persist_user_defined_timestamps=false` and `user_timestamp_size=8`. Reviewed By: xingbowang Differential Revision: D114565978 fbshipit-source-id: 6939df9163fd97fefec2fa185bc2ca72231ab04f	2026-08-05 17:06:48 -07:00
Omkar Gawde	dbf9328c71	Add per-column-family compaction abort and resume (#14982) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14982 **What**: Add reference-counted `DB::AbortCompactions(cf)` / `DB::ResumeCompactions(cf)` to abort and resume compactions for a single column family without stopping unrelated column families. They compose with the DB-wide `AbortAllCompactions()` / `ResumeAllCompactions()` and are no-ops once DB shutdown has begun. **How**: - **Per-CF abort flag.** `ColumnFamilyData` gains `compaction_aborted_`, wrapped in `CacheAlignedWrapper` so the lock-free reads done by compaction workers do not false-share with the DB-mutex-written fields packed around it. A new `IsCompactionAborted(cfd)` helper checks it alongside the DB-wide counter at every entry point (`CompactRange()`, `RunManualCompaction()`, `CompactFilesImpl()`), and `CompactionJob::ProcessKeyValue()` checks both on entry and in its periodic cron check. - **Parking.** Automatic work for an aborted CF is preserved rather than dropped. `PickCompactionFromQueue()` now returns `std::variant<CompactionQueuePicked, CompactionQueueThrottled, CompactionQueueParked>` and moves an aborted CF into `parked_compaction_cfds_` only once it reaches the front of the queue, so CFs behind it keep their place and the scheduler credit is not restored (which would cause a hot re-pick loop). `EnqueuePendingCompaction()` parks newly needed work for an aborted CF and now early-returns for dropped CFs. `RestoreParkedCompaction()` re-enqueues on the final resume; a CF dropped while parked is handled in `DropColumnFamily()`, and one dropped before its aborted queue entry is reached is handled by a fast path in `PickCompactionFromQueue()`. - **Knowing when it is safe to return.** `ColumnFamilyData::num_in_flight_compactions_` counts actually-picked work, and `Defer` balances `BeginInFlightCompaction()` / `EndInFlightCompaction()` on every `BackgroundCompaction()` and `CompactFiles()` exit path. That covers the `NotifyOnCompactionCompleted()` window that `CompactionPicker::compactions_in_progress_` does not, since the picker set is cleared during `Install`'s manifest write. `MustWaitForCompaction()` combines the counter with the picker set (filtered to skip `kExternalSstIngestion` / `kRefitLevel` reservations, which are not real jobs) because only the picker set covers the LOW-to-BOTTOM priority handoff, where the counter briefly dips to 0. - **Waking the waiter.** `AbortCompactions()` cancels matching `manual_compaction_dequeue_` entries and then waits on `bg_cv_`; `per_cf_compaction_abort_waiters_` forces the compaction completion paths to signal, because the picker's `UnregisterCompaction()` does not. `CloseHelper()` drains `parked_compaction_cfds_` and releases their references. - **API surface.** The two methods are new pure virtuals on `DB`, so `StackableDB` forwards them and the `db_test.cc` DB wrapper stubs them. `db.h` documents how the per-CF API composes with DB-wide abort, `DisableManualCompaction()`, `PauseBackgroundWork()`, shutdown, ingestion reservations, level refit and remote compaction, plus the parked-work caveat on `WaitForCompact()`. Comments in `listener.h`, `statistics.h` and `status.h` now name both abort APIs. - **Coverage.** 10 new cases in `db_compaction_abort_test.cc` (CF scoping, counter composition, parked-work restore, CF drop both while parked and before parking, trivial-move rejection, waiting for the completion callback, post-close no-op). `db_stress` gains `abort_and_resume_cf_compactions_one_in` and `TestAbortAndResumeCfCompactions()`, rejected in combination with `clear_column_family_one_in` because that would drop the column family out from under an active abort. Also a `db_crashtest.py` knob and one `new_features` release note. - **Known gap.** A `TODO` records the pre-existing post-pick stale compaction-score problem: aborting after `PickCompaction()` releases the inputs without recomputing the score, so the resume can observe a stale sub-trigger score and re-enqueue nothing. `ResumeAllCompactions()` has the same gap and carries a pointer to the same TODO; fixing it is left to a separate change covering both APIs. **Why**: Atomic-replace bulk loads only conflict with their target column family, but the DB-wide abort suppresses compaction for every column family colocated in the same RocksDB instance. Scoping the abort keeps unrelated column families compacting for the duration of a bulk load. Reviewed By: pdillinger Differential Revision: D113103899 fbshipit-source-id: 9bef924132676ed118a37aee86da6ee8bcefef09	2026-08-04 13:36:03 -07:00
Ranjan Banerjee	b1b50fe102	Parallel checkpointing stress test (#15037) ... Summary: Adds db_stress / crash-test coverage for the parallel `CheckpointEngine` so it is exercised alongside existing features (atomic flush, sync fault injection, cf_consistency, backups). - Opens one shared `CheckpointEngine` per db_stress process, configured by `--checkpoint_engine_max_background_operations` and `--checkpoint_engine_use_link_file_when_available`. - `--parallel_checkpoint_one_in=N`: each checkpoint independently uses the parallel engine with probability 1/N, otherwise the legacy serial `Checkpoint` API, so serial and parallel paths run concurrently across threads. - `db_crashtest.py` randomizes all three per run. This coverage surfaced the missing destination fsync in the parallel copy path fixed by the parent diff. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15037 Reviewed By: xingbowang Differential Revision: D114414578 Pulled By: rban1 fbshipit-source-id: 48e7920a62b9b663a1c6054fc60b5765667bcbe5	2026-08-03 11:11:46 -07:00
Josh Kang	00e73b99a2	Centralize db_stress validation errors (#15034) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15034 `db_stress` startup validation had accumulated multiple error-reporting paths, including direct `fprintf`/`exit(1)` pairs. This made invalid configurations inconsistent and bypassed normal unwinding. Centralize validation failures through `ReturnValidationError`, including diagnostics with runtime values, while leaving operational I/O failures on their existing paths. The variadic stream interface keeps the existing messages without `printf` casts. Reviewed By: jaykorean Differential Revision: D114384751 fbshipit-source-id: f396f8e7d099ef87c72aaf19455b777f2125f4df	2026-08-03 10:33:36 -07:00
Peter Dillinger	afc922bf5f	Make ReadOptions::table_filter a caller-owned pointer for cheaper copies (#15005) ... Summary: ReadOptions is copied frequently on read paths, and its only non-trivially-copyable member, table_filter, is a std::function that is almost always empty yet still gets deep-copied on every ReadOptions copy (and can heap-allocate). This changes table_filter to a pointer to a caller-owned std::function instead of storing one by value: when non-nullptr, the caller is responsible for keeping the pointed-to function alive for the duration of the read (like the other pointer fields such as iterate_upper_bound). With this change every ReadOptions member is trivially copyable, so copying ReadOptions no longer touches a std::function at all. That trivial-copyability is locked in with a static_assert in options_test.cc (kept out of the public header). This is inspired by D113109635, which targeted the same ReadOptions-copy hotspot by relocating table_filter to the end of the struct so the trivial members memcpy in one contiguous block. This is intended to be a superior solution: rather than only defragmenting the copy, it removes the std::function copy entirely. This is a breaking API change, tolerated because table_filter is a relatively obscure option. A nullptr, or a pointer to an empty std::function, means no filtering (preserving the previous empty-function behavior). A shared internal helper HasTableFilter(const ReadOptions&) in options_helper.h centralizes the nullptr + empty-function check used on the read paths. The C API and Java bindings now own the std::function and hand ReadOptions a pointer to it; their public signatures are unchanged, and the C API rocksdb_readoptions_t clears the owned function when the callback is null. Making ReadOptions trivially copyable also let -Werror=unused-but-set-variable fire on a pre-existing dead local in db_bench's MultiScan: a ReadOptions that set async_io/readahead_size but was never passed to NewMultiScan (the warning had been suppressed while ReadOptions had a non-trivial member). That local was in fact inert for multiscan -- BlockBasedTableIterator::Prepare overrides ReadOptions::async_io with MultiScanArgs::use_async_io for the coalesced prefetch, so multiscan async I/O is driven by --multiscan_use_async_io, not ReadOptions::async_io. Removed the dead local; MultiScan now uses read_options_ directly, consistent with MultiScanRandom. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15005 Test Plan: Existing table_filter coverage was adapted to the new pointer API and still exercises the behavior end to end: db_iterator_test.TableFilter (callback invoked once per table; returning false hides a table) and its range-conversion rejection case; db_bloom_filter_test SstQueryFilter/FixedWidthSegments range queries; and trie_index_db_test's range-query iterator (which returns iterators outliving the filter, exercising the caller-owned lifetime). db/c_test covers the C API set/clear/has_table_filter path. options_test asserts std::is_trivially_copyable_v<ReadOptions>. Manually verified the db_bench MultiScan async behavior with a ~200MB DB and --use_direct_reads: with ReadOptions::async_io held true, async reads tracked only --multiscan_use_async_io (rocksdb.read.async.micros count 329511 with use_async_io=true vs 0 with false), confirming ReadOptions::async_io does not drive multiscan async and the removed local had no effect. Reviewed By: anand1976 Differential Revision: D113613525 Pulled By: pdillinger fbshipit-source-id: 38d73b607500a5c7ca2b89dd8f4187ef43940aa4	2026-07-31 10:20:25 -07:00
Josh Kang	c64f682946	Fix IoUringBackend crash in ReadIOExecutor worker threads (#15020) ... Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15020 Each read executor worker creates its own `IoUringBackend`, and Folly's I/O thread pool creates its worker threads and EventBases lazily. A successful calling-thread probe or `DB::Open()` therefore does not validate every worker. A later read can start a new worker; previously, an `IoUringBackend` initialization exception on that worker could escape startup and terminate the process. The previous 512-submit setup also allowed Folly's capacity retry to reduce the completion queue to 512 while retaining a 1024-entry submission queue, which the kernel rejects with `EINVAL`. Use a 256-submit limit so the 512-entry retry remains valid. Configure the shared read executor during `DB::Open()` through `DBOptions::read_io_executor_threads`, defaulting to one thread. The backend factory catches per-worker io_uring initialization failures and uses Folly's default backend. Each worker's thread-local availability marker starts false and is set true only after `IoUringBackend` construction succeeds. When construction fails, the marker remains false and `SubmitReadAsync()` performs the read synchronously. The executor's maximum thread capacity can be raised or lowered; `db_stress` requests eight threads for async DB APIs. To expose this degradation, `FSRandomAccessFile::SubmitReadAsync()` now reports whether the asynchronous path was used. `RandomAccessFileReader` increments the `FILE_SUBMIT_ASYNC_READ_FALLBACK` ticker for every submission that falls back to synchronous I/O, making the fallback visible through the DB's `Statistics`. Reviewed By: xingbowang Differential Revision: D114031018 fbshipit-source-id: 1b7f6c1cea693e5dd743ea1ada0fca1cf757b3f9	2026-07-30 11:18:08 -07:00