mirror of
https://github.com/facebook/rocksdb.git
synced 2026-10-23 08:09:02 +03:00
main
1022 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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 |
||
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 |
||
Jay Huh
|
956c7042da |
Check async I/O support on the MultiGet thread and fall back to sync (#15337)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15337 ## Why RocksDB crash tests repeatedly fail with `MultiGetEntity error: Not implemented: ReadAsync: failed to init io_uring` when synchronous `MultiGet` requests async I/O on a thread that cannot create an io_uring. The same `NotSupported` status can reach users if capability detection succeeds but an individual asynchronous submission later fails. ## Previous behavior `Version` cached `FSSupportedOps::kAsyncIO` when it was constructed. Posix io_uring instances are thread-local, so that result described the construction thread rather than the thread later executing MultiGet. A cached positive result could route a different thread into `ReadAsync`, where io_uring creation failed and the error reached the caller. A cached negative result could also unnecessarily disable async I/O on a capable read thread. ## New behavior and implementation Synchronous `Version::MultiGet` now checks async-I/O support on the thread executing the read. Both the cross-level `optimize_multiget_for_io` path and the within-level overlap path enter their coroutine-based I/O only when that thread supports it. Posix `SupportedOps` eagerly creates and retains that thread-local io_uring, so subsequent submissions reuse the verified ring. The capability remains advisory: if `ReadAsync` nevertheless returns `NotSupported`, `AsyncFileReader` completes the `FSReadRequest` synchronously, including requests that use filesystem-owned buffers. Poll errors are applied only to requests that were actually submitted asynchronously, so synchronous fallbacks in a mixed batch retain their successful status. Regression coverage forces io_uring capability failure only on a dedicated read thread and exercises both synchronous MultiGet branches. Separate tests cover all-request fallback and a batch mixing submitted and synchronously fallen-back requests under an injected Poll error. A one-line bug-fix release note records the user-visible correction. ## Safety and compatibility This changes no API, ABI, or persisted format. The support check is only performed when synchronous `MultiGet` receives `ReadOptions::async_io`; the Posix steady-state cost is a virtual call and thread-local ring lookup per such MultiGet. Direct I/O continues to use the existing synchronous `MultiRead` route before `AsyncFileReader`, and native coroutine APIs are unchanged. Reviewed By: pdillinger Differential Revision: D117802488 fbshipit-source-id: 0f2991a523607d894436cd53123e7a1b2fe93167 |
||
|
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 |
||
|
Xingbo Wang
|
1334e4dd52 |
Add offline OPTIONS binding repair (#15310)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15310 Why the change is required: The committed OPTIONS number in MANIFEST gives persisted-options readers a precise source of truth. If that file is missing or corrupt, or an operator must deliberately replace it with a known-good snapshot, persisted-options loading can fail before a normal writable open can publish a replacement. RocksDB therefore needs a supported offline recovery path that obeys the binding and global file-number rules. Previous behavior: ldb update_manifest could rewrite file-temperature metadata but could not repair an OPTIONS binding. Operators could not safely bind a selected snapshot while accounting for the current MANIFEST boundary, every file-number allocation root, recoverable WAL output, and crash durability. Blindly selecting or reusing an existing OPTIONS number could bind the wrong snapshot or collide with another RocksDB file. New behavior: ldb update_manifest --rebind_options_file --options_file=OPTIONS-<number> performs an offline repair from an explicitly selected trusted snapshot whose tracking marker is enabled. Optional --file_number_paths arguments identify external allocation roots that are not serialized in OPTIONS. The command locks the DB and validates that the selected OPTIONS snapshot contains every live MANIFEST column family. Extra OPTIONS CFs are accepted as a safe superset and are filtered by binding-aware persisted-options APIs; a snapshot missing any live CF is rejected. The command raw-copies the selected bytes to a newly allocated canonical OPTIONS number, syncs that file and its directory entry, and commits the new number with one MANIFEST record. It rewrites the current MANIFEST only through the last completely decoded record, appends the commit and next-file boundary to a temporary copy, syncs it, atomically renames it over the original, and syncs the MANIFEST directory. The crash contract has one publication point. Before MANIFEST replacement, the previous commit remains authoritative and the copied tracked OPTIONS file is only an ignorable orphan. After replacement, the new snapshot is authoritative. Implementation: - Parse the selected snapshot for validation but copy it byte-for-byte so unknown or newer option fields are preserved. - Replay MANIFEST to recover live CF membership, the committed number, next file number, and last valid logical-record boundary; discard a torn physical tail during the bounded-memory rewrite. - Scan the DB root, WAL/archive directories, serialized roots, and operator-supplied roots for allocated numbers. - Parse recoverable WALs and reserve conservative headroom for recovery outputs, missing-column-family creation, persistent statistics, asynchronous WAL creation, MANIFEST rollover, and subsequent writable-open allocations. - Reject file-number overflow, missing allocation roots, corrupt or unreadable WALs, invalid input names, missing-live-CF snapshots, and simultaneous temperature rewriting. - Preserve the existing update_temperatures behavior and result text. Safety boundaries and unsupported cases: - This is an explicit offline mutation. It never auto-selects a snapshot and refuses to run while another process holds the DB lock. - The selected input must be a canonical OPTIONS-<number> basename in the DB directory, must parse successfully, must enable MANIFEST tracking, and must contain all live MANIFEST CFs. Extra CFs are allowed but missing live CFs are not. - The command does not repair snapshot contents or alter topology. - OPTIONS does not serialize all DB/CF data roots. Operators must provide every external root through --file_number_paths; the command fails closed when a supplied or inferred root cannot be scanned. - Conservative allocation headroom can reject a repair near kFileNumberMask even when a particular future open would allocate fewer files. - Normal DB open and core publication do not depend on this maintenance command. Validation coverage and known limitations: Coverage includes parsing and incompatible modes, the DB lock, tracking and membership validation, safe-superset acceptance, missing-live-CF rejection, byte-preserving copy, torn MANIFEST tails, one-record commit publication, file-number exhaustion and writable-open headroom, external and missing roots, WAL recovery bounds, asynchronous WAL and persistent-stat allocations, MANIFEST rollover, writable reopen, and later CF creation. The end-to-end test confirms safe-superset rebinding and rejection of a higher tracked snapshot that omits a live CF, verifies the final committed MANIFEST number, and runs the real ldb scan --try_load_options subprocess. Cross-platform CI is not part of local validation. Reviewed By: pdillinger Differential Revision: D123104846 fbshipit-source-id: 8cf7c54c1c7a360411034469614eed8aaffdcf8e |
||
|
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 |
||
|
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 |
||
Ranjan Banerjee
|
0561153b73 |
Add configurable largest-memtable flush policies to WriteBufferManager (#15047)
Summary: Motivation A WriteBufferManager can be shared by many RocksDB instances and column families. Under the existing policy, the DB whose writer first observes memory pressure flushes its own oldest mutable memtable. That DB might own only a small fraction of the reclaimable memory, so workloads with many DBs or CFs can generate many small flushes while the largest memory consumer remains untouched. Scanning every registered DB while holding process-wide or DB mutexes would improve selection accuracy but would serialize unrelated write paths and is not viable for deployments with thousands of DBs. Previous behavior kFlushOldest was the only behavior: the write-group leader that noticed WBM pressure synchronously selected the oldest eligible mutable memtable in its own DB. The WBM had no per-DB view of reclaimable memory and could not direct reclamation to another DB sharing the same budget. New behavior This change adds configurable WriteBufferFlushPolicy modes: - kFlushOldest preserves the existing behavior and remains the default. - kFlushLargest selects the largest eligible flushable column family in the writing DB. - kFlushLargestAcrossDBs selects the DB advertising the most reclaimable memory across all DBs sharing the WBM, then flushes its largest eligible CF. Atomic-flush DBs advertise and flush the aggregate eligible memory across their CFs. High-level implementation Each writable DB registers a lightweight FlushInitiator with the shared WBM. Per-DB accounting publishes two approximate values using atomics: total mutable memory and the largest flushable CF contribution, including eligible immutable memtables waiting for flush. Allocator reporting is batched, so the allocation hot path avoids registry scans and DB mutexes. A dedicated WBM coordinator thread maintains an approximate ranked snapshot of registered DBs. Refreshes are driven by accounting changes, threshold crossings, and periodic pressure checks. Ranking and O(number of DBs) work happen on that coordinator thread; writers only update counters and signal the coordinator. Registration lifetime is protected independently so DB close can safely drain an in-flight handoff. Threading and flush handoff The coordinator reserves and pins one ranked candidate at a time, without holding a registry lock while invoking the DB. The target DB uses TryLock on its DB mutex and rejects the handoff if it is closing, paused, stopped, stalled, already handling a WBM job, or no longer has flushable memory. Rejected candidates do not block consideration of the next DB. An accepted handoff queues one LOW-priority WBM job. The LOW worker joins the target DB write thread and revalidates eligibility before switching the memtable. The resulting ordinary flush is then scheduled through RocksDB's existing flush machinery on the HIGH-priority flush pool. Keeping the write-thread join off the requesting writer avoids making one DB's write path wait on another DB's mutex or write queue. Only one manager-wide remote handoff is active at a time. Completion or cancellation clears the handoff and requests a rerank. If memory reaches the hard limit while remote coordination is delayed, a rate-limited local fallback lets a writer seal a local memtable to preserve forward progress. Write stalls, shutdown, WaitForCompact, policy changes, and DB deregistration explicitly drain or reject outstanding handoffs. Compatibility The legacy constructor and kFlushOldest behavior remain unchanged unless a new policy is explicitly selected. There are no on-disk format changes. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15047 Reviewed By: xingbowang Differential Revision: D112396106 fbshipit-source-id: 3049adc7034dd4e00d74d7d86e1d19004292d299 |
||
Peter Dillinger
|
1c490a4a4d |
Fix "ancester" spelling; deprecate TablePropertiesNames (#15335)
Summary: Follow-up to https://github.com/facebook/rocksdb/issues/15304, which renamed `TableProperties::creation_time` to `oldest_ancestor_time` but left the long-standing "ancester" misspelling everywhere else, so the same concept was spelled two ways (e.g. `TableProperties::oldest_ancestor_time` vs. `FileMetaData::oldest_ancester_time` and `SstFileMetaData::oldest_ancester_time`). This change makes the spelling consistent and addresses remaining review feedback on https://github.com/facebook/rocksdb/issues/15304. Change: * All internal identifiers and comments now use "ancestor": `FileMetaData::oldest_ancestor_time`, `CompactionServiceOutputFile::oldest_ancestor_time`, `kUnknownOldestAncestorTime`, `TryGetOldestAncestorTime()`, `Compaction::MinInputFileOldestAncestorTime()`, the `kOldestAncestorTime` manifest tag enum, and related locals and test sync point names. * Public API: `SstFileMetaData::oldest_ancestor_time` is the new name; `oldest_ancester_time` remains as a deprecated alias sharing storage via an anonymous union (same approach as `TableProperties::creation_time`, so size, layout, and copy semantics are unchanged). * Public API: `TablePropertiesNames::kOldestAncestorTime` is the new name; `kCreationTime` remains as a deprecated `static const std::string&` alias. A static reference member has no per-object cost and binds at constant initialization, so it adds no static-init-order exposure. * `TablePropertiesNames` as a whole is marked DEPRECATED for public use. An fbsource-wide BigGrep found no references outside RocksDB and its vendored copies, and it is only meaningful to table format internals; users should read the corresponding `TableProperties` fields. Fix: * The `table_file_creation` event log entry had the `oldest_key_time` and `newest_key_time` values swapped since https://github.com/facebook/rocksdb/issues/13083 (v10.0). Now each label is paired with its own value. Also: tools/c_api_gen/auto_simple_bindings.py resets the pending anonymous union field list after use and skips unnamed members inside the union (review suggestion); generated C API output is unchanged. Compatibility: names that are persisted or externally parsed intentionally keep the old spelling: the on-disk property name `rocksdb.creation.time`, the `"oldest_ancester_time"` key in remote compaction serialization, and the `oldest_ancester_time` / `OldestAncesterTime` labels in `VersionEdit` DebugString/DebugJSON (ldb manifest_dump output). The manifest tag value is unchanged. HISTORY.md is left as historical record. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15335 Test Plan: DBTablePropertiesTest.CreationTimeAliasesOldestAncestorTime now also verifies `TablePropertiesNames::kCreationTime` and `kOldestAncestorTime` refer to the same object, and new DBTablePropertiesTest.SstFileMetaDataOldestAncesterTimeAlias verifies the two `SstFileMetaData` names share an address and that writes through the old name are visible through the new one. Remote compaction serialization through the unchanged `"oldest_ancester_time"` key is covered by compaction_service_test; manifest encode/decode through the renamed field by version_edit_test. `make check-c-api-gen` confirms the generated C API is up to date and backward compatible. Reviewed By: jaykorean Differential Revision: D123559299 Pulled By: pdillinger fbshipit-source-id: 480106e8d539e654f31db210ef6e1de02325d604 |
||
|
Peter Dillinger
|
c83a84e98c |
Rename TableProperties::creation_time to oldest_ancestor_time (#15304)
Summary: The `creation_time` table property has long stored the file's oldest ancestor time (the oldest key time among the file's ancestors, falling back to flush/compaction time), not the file's creation time, which is `file_creation_time`. A TODO in table_properties.h acknowledged the misleading name. This change starts the API migration: `TableProperties::oldest_ancestor_time` is the new name, and `creation_time` remains as a deprecated alias sharing storage via an anonymous union, so existing external reads and writes continue to compile and behave identically. An anonymous union is used rather than a `uint64_t&` alias member. A reference member would delete TableProperties copy/move assignment, make the implicit copy constructor bind the copy's alias to the source object (dangling), permit writes through a const TableProperties, add 8 bytes, and break the uint64_t-range walk in TEST_SetRandomTableProperties. The union leaves size, layout, copying, constness, and offsetof unchanged. Reading the other union member is formally UB in ISO C++, but both members are uint64_t and this is supported in practice by GCC, Clang, and MSVC. All in-repo uses of the old name now use `oldest_ancestor_time`. Names that are externally observable or persisted are intentionally unchanged: * On-disk property name `rocksdb.creation.time` (`TablePropertiesNames::kCreationTime`) * The `"creation_time"` key in TableProperties serialization (used by remote compaction) * The event-log JSON key and `TableProperties::ToString()` label * Java `TableProperties.getCreationTime()` and C `rocksdb_table_properties_creation_time()` C API: the auto-binding generator (tools/c_api_gen/auto_simple_bindings.py) only considered direct FieldDecls and failed on the anonymous union's implicit unnamed field. It now treats anonymous union members as direct fields. Regenerating adds a new C getter `rocksdb_table_properties_oldest_ancestor_time()` alongside the existing `rocksdb_table_properties_creation_time()`. Follow-up: remove the deprecated `creation_time` alias in a future release. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15304 Test Plan: New unit test DBTablePropertiesTest.CreationTimeAliasesOldestAncestorTime verifies the two names share an address and that a write through the old name is visible through the new one. options_settable_test and compaction_service_test cover TableProperties serialization round trips through the unchanged "creation_time" key, now mapped to the renamed field. Reviewed By: jaykorean Differential Revision: D123079083 Pulled By: pdillinger fbshipit-source-id: 18b07d91f303a8f6de58da0c730f80d8c371505c |
||
|
Xingbo Wang
|
19b6f85266 |
Support async file open for SecondaryDB (#15294)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15294 Why the change is required: `open_files_async` reduces startup latency by moving SST validation and table-reader opening to background work, but `DB::OpenAsSecondary()` still performed that work synchronously. Previous behavior: A secondary using `open_files_async=true` replayed its MANIFEST through `ReactiveVersionSet`, synchronously verified and opened every current SST, and only then returned. The option therefore provided no startup benefit for secondary instances. New behavior: `DB::OpenAsSecondary()` can now return after installing its initial read view and start initial SST validation and table-reader pinning through the existing asynchronous opener. With `max_open_files=-1`, the opener processes all current SSTs in the background; a finite table cache can stop the eager pass at capacity and validate remaining files on demand. Later `TryCatchUpWithPrimary()` rounds remain synchronous. Async-open failures notify listeners and are latched according to the existing severity and `paranoid_checks` policy. If an error is not latched, a later read retries the file access and reports a persistent failure. A latched error requires fixing the underlying problem and reopening the secondary. Read-only instances do not start write-oriented recovery. Implementation: Initial reactive MANIFEST replay can defer SST verification and handler loading. The deferral is immutable constructor state on `ReactiveVersionSet`, is cleared before catch-up mode, and is restricted to normal secondary recovery. `VersionBuilder::GetAndClearIntermediateFiles()` now moves out and clears its own historical add-then-delete bookkeeping so callers cannot accidentally retain stale state. Secondary open schedules the background opener after publishing the read view. Its context pins the captured Versions and column families, including retained secondary handles whose column family was subsequently dropped by the primary. Existing table-reader opening performs SST size and unique-ID validation, avoiding a separate serial metadata pass. Closing the DB or canceling background work safely cancels a queued opener or waits for a running opener. Read-only async-open errors use an ErrorHandler path that notifies listeners without automatic recovery. Secondary async open explicitly rejects FIFO compaction. Safety, compatibility, and limitations: The default `open_files_async=false` path and existing primary async-open behavior are unchanged. WAL and blob recovery, as well as later secondary catch-up, remain synchronous. Follower DB and `OpenAndCompact()` do not use this deferral. There are no file-format, ABI, or public API signature changes. Secondary async open requires `skip_stats_update_on_db_open=true` and does not support FIFO compaction. Applications must keep files accessible by coordinating deletion with the primary or by using an Env/FileSystem that delays physical deletion until every instance releases a file. With asynchronous opening, `max_open_files=-1` cannot guarantee every SST is pinned before `OpenAsSecondary()` returns; the table-files-only `max_open_files=-1` workaround applies only when asynchronous opening is disabled. No completion API is added. Validation covers initial deferral and pinning, synchronous catch-up, table-reader detection of physical-size mismatch, FIFO rejection, listener-adjusted errors and synchronous read retry, retained handles after primary column-family deletion, close and explicit cancellation with no HIGH-priority worker, stopping new table opens during shutdown, primary async open, follower isolation, and remote compaction. Linux Buck builds and focused tests passed. Cross-platform CI is pending, and `make check-sources` cannot run successfully in this Sapling checkout because its script requires Git. Reviewed By: pdillinger Differential Revision: D122417547 fbshipit-source-id: 91d408469c35c4ea21165c5e7db75068e65c2389 |
||
|
Xingbo Wang
|
11516577ab |
Pass blob file checksums through blob file opens (#15268)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15268 Why this is required: remote file systems need an immutable blob-file checksum and checksum-function name when RocksDB opens a finalized blob file. This lets the file system distinguish file generations and safely validate or key cached extents. Previous behavior: integrated BlobDB opened normal read-path blob files with the column family's ordinary FileOptions. Although finalized checksum metadata was stored in BlobFileMetaData and BlobFileAddition, it was not propagated through point reads, range reads, MultiGet, retry opens, or recovery verification. Delayed reads of direct-write files could also cache a finalized file reader opened without metadata. New behavior: every finalized integrated BlobDB file open receives its checksum value and checksum-function name through FileOptions. Active direct-write files continue to use empty checksum fields until finalization. Delayed reads pinned to a pre-flush SuperVersion use the finalized metadata once the file has been sealed. Implementation: add internal BlobFileOpenInfo transport across Version, BlobSource, and BlobFileCache. Populate it from version metadata and recovery additions. At the actual cache miss/open, copy the base FileOptions and set both checksum fields. Direct-write sealing registers owned checksum metadata with BlobFileCache, atomically evicts footer-less readers, and prevents a racing pre-seal refresh from repopulating the cache without metadata. Metadata is released after write-path ownership and old-memtable protection end. Safety boundaries: this does not change public APIs, persisted formats, or blob cache keys. Cache hits do not copy checksum strings. Files with no generated checksum, including active direct-write files, retain empty checksum fields. Existing footer-skip, corruption, and reader-refresh behavior remains in place. Integration in downstream file systems is outside this change. Tests and known limitations: filesystem-capture tests cover regular blob reads and delayed direct-write reads. After rebasing onto the latest master, blob_file_cache_test, blob_source_test, db_blob_basic_test, and db_blob_direct_write_test passed both normally and with ASSERT_STATUS_CHECKED=1 (95 tests in each configuration). Fast arc lint passed. make format-auto and make check-sources could not operate because their scripts assume a Git checkout while this checkout uses Sapling; all files in the patch were formatted directly with clang-format. Reviewed By: chaitanyadatye, anand1976 Differential Revision: D121630172 fbshipit-source-id: 601d1097e4a3caa60f0d9ba1794e78e81855980c |
||
Omkar Gawde
|
b4e068e1ed |
Reuse MANIFEST batch traversal after apply errors (#15288)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15288 Follow-up to D122242630 (https://github.com/facebook/rocksdb/pull/15286), addressing its review. Previous behavior: Before D122242630, when `VersionSet::ProcessManifestWrites` failed to build a batch's new `Version`, it returned without removing the batch from `manifest_writers_` (the two `FIXME? manifest_writers_ still has requested updates` comments). The writers are destroyed when their `LogAndApply` call returns, so later MANIFEST updates, including flushes and compactions, could hang behind the dangling entries or corrupt memory. It needs a fatal LSM consistency-check failure, or a `MemoryLimit` from charging `CacheEntryRole::kFileMetadata` (off by default) to a full block cache with `strict_capacity_limit`. It was found by reading the code. D122242630 fixed that, but found the end of a failed batch with a second scan that copied the grouping loop's stop conditions, which can drift apart, and added no release note. New behavior: A failed batch completes the same writers as with D122242630, but only the grouping loop decides where it ends: `ProcessManifestWrites` records the first `LogAndApplyHelper` failure in `batch_apply_status`, later iterations skip version construction but still run the loop's end-of-batch checks, and the batch then completes through `finish_batch`. This also labels the inner-loop `break`s and adds a release note. Safety, compatibility, and limitations: No API, option, or format change. The success path and the `SaveTo` failure path are unchanged. Reviewed By: pdillinger Differential Revision: D122417768 fbshipit-source-id: c6d474ace32446d6b413b1a1b3cf2fa49a02447d |
||
|
Omkar Gawde
|
5fa16200aa |
Release MANIFEST writers after batch-build failures (#15286)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15286 **What**: `LogAndApply` completes every MANIFEST writer in a batch when version construction fails before persistence. **How**: `VersionSet::ProcessManifestWrites` centralizes queue completion in `finish_batch`, invokes it for `VersionBuilder::Apply` and `SaveTo` failures, and extends assembly failures through the normal group boundary. Completion propagates the failure through callbacks, signals writers owned by other callers, and wakes the next leader. **Why**: Stack-backed writers otherwise remain queued after their caller returns, leaving dangling pointers, blocked writes, and flush or compaction state that never receives its failure callback. Reviewed By: xingbowang Differential Revision: D122242630 fbshipit-source-id: bab4ac8f665269ba57dd86579299c1d5935fa907 |
||
Anand Ananthabhotla
|
dd54c5abc9 |
Log DB session id for file deletion and manifest creation (#15229)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15229 Add `db_session_id` to obsolete file deletion logs and structured table/blob deletion event logs. Also include `db_session_id` when logging new MANIFEST creation so split-brain DBImpl sessions can be correlated without relying on process-local pointers. Reviewed By: joshkang97 Differential Revision: D120413139 fbshipit-source-id: c175b70d9b1f93e3242ee9e3191c274dc51ef3bf |
||
|
Peter Dillinger
|
34d5074bfa |
Cross-key coalescing for lazy wide-column blob reads (#15178)
Summary: Follow-up (Phase 3) to the experimental lazy wide-column read API (DB::GetEntityLazy / DB::MultiGetEntityLazy). A MultiGetEntityLazy batch now resolves blob references across keys with coalesced I/O instead of key-by-key. - MultiGetEntityLazy acquires one SuperVersion and one consistent (implicit) sequence number for the whole batch via MultiCFSnapshot (as multi-CF iterators do) and transfers a single shared per-column-family pin into the LazyWideColumnsBatch, replacing the former per-key SuperVersion + explicit-snapshot loop. Single column family for now; a cross-CF ColumnFamily** overload remains a follow-up (the batch already models a per-CF pin map). - LazyWideColumnsBatch::MultiResolve groups the batch's reads by (Version, blob file) for separate-file references and by SST for embedded references, and issues one coalesced read per group -- for whole-column and byte-range reads alike -- caching whole values in each entity's resolver and pinning partial reads directly. - Adds the coalesced blob-read primitives this needs (none existed before): range-aware BlobFileReader::MultiGetBlobRange / BlobSource::MultiGetBlobRange / Version::MultiGetBlobLazy for separate-file references, and an embedded (SimpleGen2) batch read path (blob_gen2_format batch readers + BlobSource::MultiGetSimpleGen2Blob[Range] + BlockBasedTable::MultiGetSameFileBlob). Whole-value separate-file coalescing reuses the existing MultiGetBlob. The batch readers sort their requests by file offset before issuing the underlying MultiRead (which requires ascending offsets), and gen2 checksum verification is factored into one helper shared by the scalar and batch readers. - The hot non-lazy Get / MultiGet / iterator / compaction paths are untouched: the new multis are lazy-only, and the GetImpl change is gated on a new opt-in GetImplOptions field (an injected shared SuperVersion + sequence number) used only by the batched lazy read. - Blob read failures on these paths (short reads, invalid offsets, out-of-range requests, compression mismatches) now carry file/offset/size and expected-vs-actual context, mirroring the block-checksum diagnostics. - db_bench gains a multireadrandomentitylazy benchmark for measuring the coalesced batch path. No public API change, so this is a performance follow-up to the still-recent lazy API rather than a new feature. EXCEPT that `LazyColumnReadRequest::status == nullptr is now explicitly disallowed`. Left for later follow-up: - When a whole fetch is planned for an (entity, column), bind ordinary range requests for that column to the same fetch and slice them after adoption instead of issuing separate range I/O. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15178 Test Plan: New unit tests in db/wide/db_lazy_entity_test.cc assert cross-key coalescing via a blob-file / SST MultiRead counter added to the test FileSystem wrapper: - N whole-column separate-file reads across keys in one blob file collapse to a single coalesced MultiRead (and the fetched values are cached, so a repeat read does no further blob I/O); - N byte-range separate-file reads collapse to one MultiRead, save the un-read bytes (rocksdb.blobdb.lazy.partial.bytes.saved), and never fill the blob cache; - embedded (same-file) reads across keys in one SST collapse to one MultiRead over that SST; - an out-of-order embedded batch (reads issued in descending record offset) still resolves correctly in one coalesced read, covering the batch readers' internal offset sort; - a mixed batch (inline + whole + byte-range across keys) returns the correct bytes for each read; - a batched MultiGetEntityLazy + one MultiResolve matches resolving each key via a separate GetEntityLazy. The existing db_stress lazy-vs-eager differential (MaybeTestMultiGetEntityLazy) exercises the new batched read and cross-key MultiResolve unchanged (same public API). Reviewed By: xingbowang Differential Revision: D118660682 Pulled By: pdillinger fbshipit-source-id: 6a832175e730917eddeedab8e0b9cb2b8d6ae7b4 |
||
|
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 |
||
|
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):
|
||
|
Peter Dillinger
|
f7b791b213 |
Fix bottommost-files infinite compaction loop with preserve/preclude (#15146)
Summary: Leveled compaction combined with preserve_internal_time_seconds or preclude_last_level_data_seconds is a known-broken combination, but it is easy to enable by accident -- for example a fleet-wide config rollout that turns on the preserve time for CFs that happen to be leveled. Rather than degrading gracefully, it triggers an infinite kBottommostFiles compaction loop: a bottommost file gets marked, rewritten with no progress, and immediately re-marked, continuously rewriting the file and growing the MANIFEST so the file set never quiesces. This change makes such a misconfiguration a harmless no-op instead. Before this change, the marking logic (ComputeBottommostFilesMarkedForCompaction) marked a bottommost file whenever its largest seqno was below the oldest snapshot, without checking whether a compaction could actually zero that seqno. Recently written seqnos stay within the seqno->time "preserve" window and cannot be zeroed, so the rewrite never made progress. The same futile marking also occurred with user-defined timestamps when full_history_ts_low is unset. The underlying issue is that the "can this bottommost seqno be zeroed?" question was answered independently by the marking logic and by the compaction output path (CompactionIterator::PrepareOutput), and the two had drifted apart. With this change, both paths answer that question through one shared predicate (BottommostSeqnoCanBeZeroed), and the preserve-window boundary that marking needs is plumbed into VersionStorageInfo from DBImpl, mirroring the existing oldest-snapshot plumbing. Snapshot visibility stays a per-site check. On CFs without preserve/preclude the predicate is a no-op. Confirmed that caller invariants ensure that relocating the `ikey_.sequence <= preserve_seqno_after_` check in CompactionIterator::PrepareOutput() will not trip the ROCKSDB_LOG_FATAL. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15146 Test Plan: - compaction_picker_test: new CompactionPickerTest BottommostFileNotMarkedWithinPreserveWindow / BottommostFileMarkedBelowPreserveWindow cover the preserve-window boundary; new CompactionPickerU64TsTest BottommostNotMarkedWithEmptyFullHistoryTsLowAndUnknownMaxTs covers the UDT edge case. - tiered_compaction_test: new PrecludeBottommostLoopTest LeveledPreserveTimeNoBottommostLoop is an end-to-end regression for the loop and the DBImpl plumbing: a bottommost file with a nonzero largest seqno inside the preserve window, made a marking candidate via a snapshot release, must not be marked. It reproduces the loop (hangs) when the plumbing is neutralized and passes with the fix. Reviewed By: xingbowang Differential Revision: D117299979 Pulled By: pdillinger fbshipit-source-id: 51e8e2d4e2116984d16d3c7ef54a9635d5aded74 |
||
|
Xingbo Wang
|
e6a2ee0bd2 |
Fix subset checkpoints with incomplete MANIFEST tails (#15123)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15123 Subset checkpoint post-processing appends column-family drop records to a copied MANIFEST. Recovery can tolerate an incomplete atomic group at the tail while retaining the reader high-water size, so appending at that size can produce a checkpoint that fails recovery. Track the valid logical append boundary together with its MANIFEST file number, refresh it after recovery and successful writes, reset it across reactive MANIFEST switches, and trim only subset-checkpoint copies before appending drop records. Whole-DB checkpoints and the source MANIFEST remain unchanged. Reviewed By: anand1976 Differential Revision: D116449394 fbshipit-source-id: a162dd6a2558f0f9ec40d878ee83956a89df698b |
||
|
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 |
||
|
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 |
||
Andrei Preda
|
b616b203b8 |
Drop flushed WAL data from the secondary's active memtable (#15066)
Summary: Fix https://github.com/facebook/rocksdb/issues/15051: a secondary serves stale reads because entries replayed from a WAL the primary has since flushed are never evicted from the active memtable. During TryCatchUpWithPrimary(), seal the active memtable once the installed Version provably covers everything it holds, so the following RemoveOldMemTables() call drops it. Every memtable-dropping decision takes that same watermark, so a file set the secondary cannot read never turns the stale read into a vanished key. ## Why and How A secondary replays the primary's WALs into its active memtable and stops replaying a WAL once the primary flushes it. Entries already replayed stayed behind, and since reads consult the memtable first, they shadowed the newer flushed values indefinitely. Iterators go stale too once bottommost compaction rewrites the flushed entry's sequence number to 0; until then Get() and an iterator disagree. Seal the active memtable during catch-up, gated on the installed Version: each point-in-time Version now carries the column family's log number it reflects, exposed via the new ReactiveVersionSet::GetInstalledVersionLogNumber(). Sealing requires cf_id_to_current_log_[cfd->GetID()] < that number, which guarantees the primary flushed everything the memtable holds (unflushed WAL data is kept) and that the flushed data is readable through the installed Version. The gate asks the Version directly because proxies fail: the log number alone advances even when no Version was installed, and "no missing files" misses atomic_flush groups parked by a sibling's missing file and corrupt-but-present files with verify_sst_unique_id_in_manifest=false. In those cases the memtable holds the only readable copy, so dropping it would turn a stale read into a vanished key; the gate fails closed and keeps it. Reconcile every initialized column family, not just cfds_changed: a round that failed after advancing the log number leaves a stale memtable that later rounds see no new MANIFEST record for. It can also leave a collectible immutable memtable, so the loop consults the new O(1) MemTableList::HasOldMemTablesToRemove() before skipping a column family. The predicate keeps the skip precise: reconciling any column family with a non-empty immutable list would be correct but wasteful, installing a fresh SuperVersion every catch-up call for memtables legitimately waiting on a primary flush and costing readers a mutex acquisition each time. With it, no extra super versions are installed in the steady state. That predicate and the RemoveOldMemTables() cutoff take the installed Version's log number as well, not cfd->GetLogNumber(): that advances as soon as the flush record is read, so an immutable memtable sealed on a WAL switch could otherwise be freed while the flushed file superseding it is unreadable. A sealed memtable is stamped with cf_id_to_current_log_[cfd->GetID()] + 1, the log number following its contents. cf_id_to_current_log_ is keyed by column family id rather than ColumnFamilyData*, because ids are never reused while a dropped column family's pointer can be recycled, and it is recorded before WriteBatchInternal::InsertInto() rather than after: a batch stops at its first failure with earlier entries already inserted, so recording afterwards could leave the map naming an older WAL than the memtable holds. While the watermark lags and uncollectible memtables accumulate, the secondary warns, since it has no write path to enforce max_write_buffer_number. The seal sequence, shared with RecoverLogFiles() and the follower's TryCatchUpWithLeader(), is extracted into DBImplSecondary::SealActiveMemtable(). The follower is refactored onto it with no behaviour change: it keeps cfd->GetLogNumber() as its cutoff because that catch-up path never replays WALs, so its memtables hold nothing that only a not-yet-installed Version could make readable. ## Testing New tests in db_secondary_test cover: the stale read via both Get() and an iterator, before and after bottommost compaction; unflushed WAL-only data surviving sealing, including two column families sharing a WAL; no data loss with a missing, atomic_flush-sibling-missing, or corrupt flushed file; an immutable memtable sealed on a WAL switch surviving an unreadable flushed file; a batch rejected partway through replay leaving the recorded WAL no older than the memtable's contents; and reconciliation after a failed round (injected via FaultInjectionTestFS at a sync point). Each mechanism was individually neutralized and confirmed to fail its test. Verified with build_tools/rocksptest.sh over db_secondary_test, db_follower_test, memtable_list_test, version_set_test and version_edit_test (all pass, also under ASSERT_STATUS_CHECKED=1), plus db_basic_test, corruption_test, repair_test and db_flush_test; the secondary tests also ran 20x under COERCE_CONTEXT_SWITCH=1 with no flakes. make format-auto and make check-sources are clean. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15066 Reviewed By: jaykorean, anand1976 Differential Revision: D115317523 Pulled By: xingbowang fbshipit-source-id: a1d4615eb951d75e69ed242731336485c0dfa137 |
||
|
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 |
||
|
Josh Kang
|
14ecf7f13c |
Route single-blob reads through BlobFetcher (#15046)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15046 `BlobFetcher` is the policy boundary for single-blob reads, but `Version::Get()` and primary direct-write postprocessing still called lower-level backends directly. This duplicated backend selection and could bypass future fetcher policy or decorators. Route those paths through their existing `VersionBlobFetcher` and remove the redundant Slice-based `Version::GetBlob()` overload. This is behavior-preserving: each fetcher delegates to the same Version or direct-write backend with the same `ReadOptions`, cache, and lifetime context. Reviewed By: xingbowang Differential Revision: D114616978 fbshipit-source-id: 0d94cc389138f27791de636e2cca585e51cb142a |
||
Darkhan Imangaliyev
|
f9df618e4d |
Api to checkpoint only selected set of column families (#15019)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15019 RocksDB's Checkpoint API currently copies all column families. For use cases that only need a subset, this is wasteful. This diff adds a new `CreateCheckpoint(dir, column_families, ...)` overload that: - Validates input handles (rejects null/foreign handles, coalesces duplicates, always includes default CF). - Filters GetLiveFilesStorageInfo to capture only the requested CFs' table/blob files. - Post-processes the copied MANIFEST by appending kColumnFamilyDrop records for excluded CFs, so the checkpoint opens without referencing missing files. - An empty column_families vector falls through to the existing whole-DB path. Reviewed By: xingbowang, pdillinger Differential Revision: D113963616 fbshipit-source-id: d1becbbb9528dd13c2dc314165880b76123757f0 |
||
|
Josh Kang
|
2c77c32ac9 |
Add Get and MultiGet sync/async plumbing (#14934)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14934 First diff in the async read stack: pure plumbing, no behavior change. Both the point `Get` path and the batched `MultiGet` path gain synchronous/coroutine twins generated by `DEFINE_SYNC_AND_ASYNC`; `DB::GetAsync` and `DB::MultiGetAsync` become the public callback-based async entry points with synchronous default implementations. For `Get`, the existing `DBImpl::Get` / `GetImpl` bodies are relocated into the sync/async wrapper. For `MultiGet`, the same wiring continues through the DBImpl, version, and table-cache layers. The generated coroutine variants still run the existing synchronous logic. This keeps the refactor mechanical while preserving dispatch semantics: the coroutine side remains virtual at the same `GetImpl` boundary as the synchronous path, read-only and secondary DBs use generated sync/coroutine overrides instead of falling back to base `DBImpl`, and `StackableDB` leaves async `Get` / `MultiGet` to the base wrappers so wrapper-specific `Get` / `MultiGet` behavior is preserved. For ABI compatibility and avoiding MACRO blocks in public headers, the asynchronous APIs are callback based instead of coroutine based. Internally it will be developed using coroutines, and applications can also develop their own coroutine wrappers. This is meant to make the later changes to actually enable coroutine-backed async queries much easier to review. Benchmarks: Compared `db_bench` with and without this diff using: `db_bench --benchmarks=fillseq,readrandom --num=1000000 --reads=5000000 --key_size=16 --value_size=100 --seed=12345 --db=<tmp>` | Benchmark | Without diff avg | With diff avg | Delta | | `fillseq` | 4.076 us/op | 4.165 us/op | +2.2% | | `readrandom` | 3.939 us/op | 3.947 us/op | +0.2% | No meaningful local `db_bench` regression was observed. Reviewed By: xingbowang Differential Revision: D109346179 fbshipit-source-id: 0dee1239f49099b67c11c63426204466e963f903 |
||
Andrew Chang
|
2b628ccfe5 |
Preserve fast_sst_open metadata across MANIFEST rotation (#14987)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14987 The `fast_sst_open` optimization speeds up DB open times by persisting file metadata information in the MANIFEST. When MANIFESTs are rotated however, this metadata is currently getting dropped, since it is not fed through `VersionEdit::AddFile`. There were also a few additional missing `AddFile` call sites. Fix: Add an optional `file_open_metadata` to `AddFile`. Feed this through wherever we call `AddFile`. Reviewed By: anand1976 Differential Revision: D113265655 fbshipit-source-id: c8371d66f6555bb9c372eea095bfa30a5f56a88a |
||
|
Peter Dillinger
|
81ea677072 |
PinnableWideColumns support for >1 backing buffer to avoid re-serialization (#14943)
Summary:
Wide-column read paths that materialize blob-referenced columns used to re-serialize the entire entity into a fresh contiguous buffer, copying the (potentially large) resolved column values byte-for-byte. This was forced by PinnableWideColumns holding a single backing buffer with all column Slices pointing into it.
PinnableWideColumns now holds its columns as zero-copy Slices into one or more stable backing buffers, kept in a std::forward_list<PinnableSlice> whose nodes never relocate on move, move-assignment, or splice. Resolved blob columns can be spliced in as additional backing buffers instead of re-serializing the entity.
Using address-stable nodes also cleans up the old Slice-invalidation handling. The previous single-buffer representation had to cope with a self-pinned PinnableSlice whose small (SSO) std::string relocates when the PinnableSlice is moved: on every move it detected the relocated backing pointer and re-derived the column Slices by re-parsing. With forward_list nodes that never move, the column Slices stay valid across move / move-assignment / splice, so that handling is gone and the move operations are now the defaulted member-wise moves.
The multi-buffer plumbing is internal only: a new PinnableWideColumnsHelper (a friend) performs the splice and the little bit of private-member access the read paths need, so there is no new public API surface and the previous `friend class DBImpl` is gone. The public view is unchanged: columns() still returns a single, in-order set.
Read paths converted to resolve without re-serializing:
- GetContext::SaveWideColumnEntityToColumns (eager GetEntity SST reads, separate-blob-file references)
- DBImpl::ResolveDirectWriteWideColumns (direct-write blob references)
- DBImpl::MaybeResolveMemtableBlobValue (blob-backed memtable reads)
Merge output still serializes (the MergeOperator API consumes/produces a serialized entity). Same-file ("embedded") blob columns are resolved -- and still re-serialized -- earlier in the block-based table reader (MaybeResolveEmbeddedValue / ResolveEmbeddedValueForGet) before the entity reaches GetContext, so the GetEntity path above only avoids re-serialization for separate-blob-file references. Compaction's separate entity machinery is unchanged.
API/accounting: deprecated PinnableWideColumns::serialized_size() (comment-only, per repo convention) in favor of the new payload_size() (sum of column name + value sizes; equals the value size for a plain value). Read statistics for entities (BYTES_READ, BYTES_PER_READ, per-key GetEntity/MultiGetEntity byte counts) and the ReadOptions::value_size_soft_limit threshold now measure entities by payload_size() instead of the serialized size. This drops framing bytes (entity measurements are slightly smaller), thus making plain-value and wide-column accounting consistent and reducing unnecessary exposure to implementation details.
Serialization cleanup enabled by the above:
- Consolidated Deserialize/DeserializeV2 into a single Deserialize(const Slice&, WideColumns&, blob_columns*) plus a DeserializeSimple alias for the no-blob-references case; both take const Slice&, require the input to be exactly one serialized entity, and reject any trailing data uniformly (V1 previously ignored trailing bytes). An unexpected blob reference where a fully-resolved entity is required now returns Corruption.
- ResolveEntityBlobColumns (serialized output, used only by merge) is now a thin wrapper over the new multi-buffer resolver plus Serialize, removing the duplicated fetch loop and the SerializeResolvedEntity helper.
- Consistently use `const Slice& input` rather than `Slice& input` for wide column functions expecting an input slice with no trailing data.
Design/polish from human review:
* The initial plan called for more complex handling around a vector<PinnableSlice> backing store (unique_ptr indirection and re-deriving the column Slices on move to cope with reallocation and SSO relocation); switching the backing store to a std::forward_list of address-stable nodes was a human suggestion that removed that complexity and the move-time Slice re-derivation entirely.
* Keep resolve plumbing off the public API via PinnableWideColumnsHelper and removing the DBImpl friend
* Various pieces of cleanup and polish (more left to do)
* The eager-GetEntity optimization
* The memtable inline-copy NOTE/TODO.
Intended follow-up:
* Resolve FIXME about pre-existing swallowed non-OK status in db_impl.cc
* Resolve TODO about value_size_soft_limit in db_impl.cc
* Extend this optimization to SST embedded-blob references
* Error-status/API cleanup: classify an unrecognized wide-column version as Corruption (not NotSupported) at all entry points; replace the NotSupported sentinel from GetValueOfDefaultColumn with an explicit is_blob_reference out-param (removing GetValueOfDefaultColumnResolvingBlobs); and centralize the "blob-backed value without a blob fetcher" Corruption into the resolvers (resolving the ResolveEntityForMerge FIXME) rather than repeating it at each caller
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14943
Test Plan:
Extended db/wide/wide_column_serialization_test.cc with PinnableWideColumns unit tests: set/payload_size for plain, V1, and resolved entities; move and move-assign of a multi-buffer result preserve column data pointers (the node-stability guarantee); Reset; ResolveColumns splices blob buffers in zero-copy (resolved values' data() point into the spliced-in buffers, not a re-serialized entity); serialized_size() == SerializedSizeV1(columns()); and Deserialize rejects trailing data after a serialized entity for both V1 and V2 layouts.
Built and ran (debug): wide_column_serialization_test, db_wide_basic_test, db_wide_blob_direct_write_test, db_blob_basic_test, compaction_iterator_test, db_merge_operator_test, db_iterator_test, udt_util_test, write_batch_with_index_test -- all pass. librocksdb and db_stress build. make format-auto and build_tools/check-sources.sh are clean.
Reviewed By: xingbowang
Differential Revision: D112212266
Pulled By: pdillinger
fbshipit-source-id: 13b89e2bb7d8371f4c1ff18b234a549bea9c9e77
|
||
|
Peter Dillinger
|
6cceef9497 |
Bug fixes: surface embedded-blob resolution errors (#14906)
Summary:
Experimental embedded ("same-file") blob SST support (https://github.com/facebook/rocksdb/issues/14851) overloads blob file number 0 as both kInvalidBlobFileNumber and the same-file sentinel kCurrentFileBlobIndexFileNumber. Same-file references must be resolved by EmbeddedBlobResolvingIterator before they reach any generic file-metadata or integrated-BlobDB path, all of which reject file number 0 as invalid.
When resolving a same-file blob hit an error (e.g. a db_stress-injected blob-region read fault), EmbeddedBlobResolvingIterator::value() fell back to returning the RAW, unresolved same-file value, surfacing the error only via status()/Valid(). Compaction consumed the raw value before consulting status, leaking an unresolved same-file reference into the output stream. Depending on the value type this produced two different crash-test failures, both from this one root cause:
* T277566778 -- wide-column entity variant. The raw same-file entity (kTypeWideColumnEntity) reached FileMetaData::UpdateBoundaries, whose blob-ref scan (correctly) rejected file number 0: Flush failed: Corruption: Invalid blob file number (also observed from a background compaction). The tripwire fired on the leak, so the real injected read error was masked behind a misleading corruption status.
* T277310719 -- whole-value BlobIndex variant, and more dangerous because it escapes that tripwire. ResolveKeyType() rewrites the key type kTypeBlobIndex -> kTypeValue with no I/O (so it always succeeds); on the masked error the emitted entry is therefore {kTypeValue, raw BlobIndex bytes}. UpdateBoundaries only scans kTypeBlobIndex/entity types, so it does NOT reject this record: the corruption is silently written to the compaction output and persists. A later point lookup reads those raw BlobIndex bytes back as the value, which db_stress detects as db_stress: expected_value.cc:102: Assertion `ExpectedValue::IsValueBaseValid(value_base)' failed from TestGet. In the captured crash the aborting db_stress process had injected no faults itself -- it read corruption persisted by an earlier faults-on run (blackbox reuses the DB directory), confirming the persistence route.
Fix: EmbeddedBlobResolvingIterator now resolves eagerly for callers that do not opt into unprepared values (allow_unprepared_value=false, e.g. compaction), via a new EagerEmbeddedBlobResolvingIterator (the caller is selected in BlockBasedTable::NewIterator). Eager callers resolve the value during positioning, so a resolution error (blob-region I/O or corruption) is observable through status()/Valid() BEFORE value() is consumed, and value() never exposes an unresolved same-file BlobIndex. Lazy callers (allow_unprepared_value=true, user iteration) keep value laziness but must honor PrepareValue()'s result. This keeps the "callers never see an unresolved same-file blob" invariant structural, even on the error path. Using a template for EmbeddedBlobResolvingIterator minimizes unnecessary overheads.
Also hardens and documents the integrity tripwires that caught the leak, so they are not "fixed" by weakening them -- doing so would mask real corruption and could persist unresolvable same-file references into ordinary (non-embedded) SSTs, turning a transient error into permanent data loss: FileMetaData::UpdateBoundaries (write/output path), Version::GetBlob and Version::MultiGetBlob (integrated-BlobDB read path), plus the contract note in blob_constants.h.
Adds a test-only sync point
"BlockBasedTable::MaybeResolveEmbeddedValue:InjectError" (compiled out under NDEBUG) to simulate a blob-region resolution fault deterministically.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14906
Test Plan:
New unit tests in db_blob_index_test, both of which fail without the fix and pass with it:
* DBBlobIndexTest.EmbeddedBlobResolveErrorDuringCompactionNotMasked -- entity variant. Without the fix CompactRange fails with the masked "Invalid blob file number"; with the fix it fails with the injected IOError.
* DBBlobIndexTest.EmbeddedBlobResolveErrorWholeValueDuringCompactionNotMasked -- whole-value variant. Without the fix CompactRange SUCCEEDS (silently masking the error and persisting {kTypeValue, raw BlobIndex}); with the fix it fails with the injected IOError.
Confirmed the tests exercise the real root cause by temporarily neutralizing the eager resolution (MaybeEagerlyMaterialize): both tests then fail in their respective pre-fix modes (entity -> "Invalid blob file number"; whole-value -> compaction succeeds), then pass again once restored.
End-to-end: the db_stress reproducer for T277566778 (blackbox crash test with embedded-blob ingestion and fault injection) reproduced the "Invalid blob file number" crash before the fix and no longer reproduces with it.
Reviewed By: anand1976
Differential Revision: D110361228
Pulled By: pdillinger
fbshipit-source-id: 1faaa9a9ead726f17b611ff150418fa59ed75fd4
|
||
|
Josh Kang
|
7785264e5a |
Make file opens multi-threaded for commit external sst file ingestion (#14853)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14853 `IngestExternalFileOptions` now exposes `file_opening_threads` so external file ingestion can open table readers for newly committed SSTs in parallel. The requested thread count is carried from each ingestion job into commit and then into manifest `LogAndApply`, where `VersionBuilder::LoadTableHandlers()` uses it while installing the ingested files. This is intended for workloads that ingest many external SSTs in one call, where serial table-reader opens can dominate commit latency. The ingestexternalfile benchmark now has a flag for this option, and `db_stress` now creates multiple data SSTs per ingest operation so stress runs cover multi-file ingestion, file-info ingestion, and the parallel file-opening path together. Benchmarks Ran `db_bench --benchmarks=ingestexternalfile --use_existing_db=0 --num=2200000 --compression_type=none --statistics --use_direct_reads=true --ingest_external_file_batch_size=200 --ingest_external_file_num_batches=1 --ingest_external_file_use_file_info=true --ingest_external_file_fill_cache=false` while varying `--ingest_external_file_file_opening_threads`. The generated SST files were about 256 MiB each. | `file_opening_threads` | `rocksdb.ingest.external.file.prepare.micros` | `rocksdb.ingest.external.file.run.micros` | File opens | | 1 | 16,150 us | 327,136 us | 200 | | 32 | 16,665 us | 38,349 us | 200 | At the same ~256 MiB SST size, 2,000 files is roughly 500 GiB of SST data. A straight 10x extrapolation of the 200-file ingest histograms puts prepare+run at about 3.43s with `file_opening_threads=1` and 0.55s with `file_opening_threads=32`; run alone is about 3.27s vs. 0.38s. Reviewed By: pdillinger Differential Revision: D108347952 fbshipit-source-id: 49efa48524df64cace010b881030f63f771cbb95 |
||
Steph Pontikes
|
e18d41e08c |
lazily intialize iterators (#14772)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14772 Updated the iterator creation scheme to happen lazily (on request) as oppsed to eagerly. this allows us to prune the iterator tree structure at the time of requesting iterator preparation as opposed to creation, and allows pruning to become an implementation detail. Version now skips non-overlapping SST levels and files before adding children to the iterator tree, returns direct table iterators when a level has a single matching file, and uses pruned LevelIterator instances when multiple files in one non-L0 level match. The overload no longer prepares iterators during creation; callers that need prepared multiscan execution still call Prepare explicitly after construction, and MultiScan does that itself. Benchmark: ran `db_bench` in opt mode for the base revision and this diff, with `fillseq,compact,levelstats,multiscanrandom`, `--num=1000000`, `--reads=10000000`, single thread, fixed seeds, `--multiscan_use_async_io=false`, and `--use_multiscan=true`. Both A and B had exactly one SST file and no memtable/L0 data (`L0: 0 files`, `L1: 1 file, 61 MB`). `multiscanrandom` creates `MultiScanArgs` and calls `NewMultiScan(...)`, which reaches the new `NewIterator(..., scan_opts)` pruning path in this diff. ``` seed base A pruning B delta 424242 21824.333 17693.333 -18.9% 424243 24042.014 19424.056 -19.2% 424244 22424.974 17636.910 -21.4% 424245 22404.213 18612.840 -16.9% ``` Average: base `22673.9 us/op`, pruning `18341.8 us/op`, about `19.1%` faster. Reviewed By: xingbowang Differential Revision: D104904298 fbshipit-source-id: a742106a1d5813fb795a39eeeb35f8cddc02e886 |
||
zaidoon
|
7affaee1c4 |
Add use_direct_io_for_compaction_reads option (#14743)
Summary: Adds a new `DBOption use_direct_io_for_compaction_reads` (default false). When on, compaction-input SST files are opened with `O_DIRECT` so the sequential read-once data from compaction doesn't pollute the OS page cache and evict the hot user-read working set. User reads keep going through the buffered fast path. This protects user-read tail latency on write-heavy workloads without forcing user reads onto the existing global `use_direct_reads` knob (which pays in throughput and P50 — see the bench below). The interesting bit is that just flipping the FileOptions returned by `FileSystem::OptimizeForCompactionTableRead` doesn't actually trigger `O_DIRECT` at the kernel level. The TableCache (and `FileMetaData::pinned_reader`) is already holding buffered handles opened at flush time or at `DB::Open` via `LoadTableHandlers`. When compaction asks for an iterator, it gets back the cached buffered handle and the kernel never sees the `O_DIRECT` flag. So this PR also adds a small bypass path: - `TableCache::FindTable` / `NewIterator` learn a `open_ephemeral_table_reader` mode. When set, the pinned-reader fast path and the shared cache are skipped, `GetTableReader` is called directly with the caller's FileOptions, and ownership of the freshly opened TableReader is handed back via a `unique_ptr`. The iterator takes ownership via `RegisterCleanup` and frees the reader on destruction. - `VersionSet::MakeInputIterator` and `LevelIterator` plumb the flag through both L0 and L1+ compaction-input paths. - `CompactionJob::ProcessKeyValueCompaction` turns the bypass on when `use_direct_io_for_compaction_reads` is set, the global `use_direct_reads` is off, and `OptimizeForCompactionTableRead` produced `use_direct_reads=true` in the compaction-read FileOptions. The option is opt-in: when off, nothing changes for existing users. When on, only the compaction-input opens take the bypass path; user reads keep hitting the TableCache and the buffered fast path normally. There's also a small db_bench helper in the same PR: a new `--bgwriter_num` flag that lets the writer thread in `readwhilewriting` (and the other "while writing" variants) spread its puts across `[0, bgwriter_num)` instead of `[0, num)`. Without this the readers and writer share a key range and you can't have both a hot read subset and meaningful compaction work — this lets you have both. ### Benchmark Setup: Ubuntu 24.04 (kernel 7.0.5, OrbStack Linux VM on Apple Silicon), 14 vCPUs, virtio-blk disk, btrfs. MGLRU disabled (`echo 0 > /sys/kernel/mm/lru_gen/enabled`) so the kernel uses the classic active/inactive LRU. 14 GB DB (3.5M keys × 4 KB values), no compression. Each measurement run is pinned to a 1 GB cgroup via `systemd-run --scope -p MemoryMax=1G -p MemorySwapMax=0`. Page cache is dropped between configs. db_bench is Release build. Workload: `readwhilewriting` for 120s. 4 reader threads doing random reads over a hot key subset, plus 1 writer thread spreading overwrites across the full 3.5M-key keyspace (via `--bgwriter_num=3500000`) throttled at 200 MB/s, so there's continuous compaction running while the readers go. The size of the hot reader subset relative to available page cache controls how visible the optimization is. The Cassandra blog ([Lightfoot 2026](https://lightfoot.dev/direct-i-o-for-cassandra-compaction-cutting-p99-read-latency-by-5x/)) documented the same thing: biggest wins when the hot set is big enough to actually compete for cache, smaller wins when the hot set trivially fits, neutral when the hot set is way bigger than cache. So I ran two hot-set sizes. #### Small hot set: ~30 MB (~3% of the 1 GB cgroup) — N=5 iterations, mean (CV) `--num=7500`. The hot set is small enough that the page cache holds it without much trouble even under compaction, so the wins here are real but on the modest side. | Config | Throughput (ops/s) | Read P50 (µs) | Read P99 (µs) | Read P99.9 (µs) | Read P99.99 (µs) | |---|---|---|---|---|---| | buffered (default) | 233,477 (8.2%) | 16.09 | 82.24 | 721.0 | 2,102.5 | | direct_compaction_writes_only (existing knob alone) | 287,405 (2.8%) — **+23.1%** | 13.00 (−19.2%) | **66.77 (−18.8%)** | 553.9 (−23.2%) | 1,787.6 (−15.0%) | | direct_compaction_read_only (new knob alone) | 250,669 (2.4%) — +7.4% | 14.16 (−12.0%) | 102.99 (+25.2%) | 689.8 (−4.3%) | 1,801.3 (−14.3%) | | direct_compaction_read_write (new + existing, recommended) | 277,920 (3.3%) — **+19.0%** | **12.99 (−19.3%)** | 84.23 (+2.4%) | 613.4 (−14.9%) | **1,738.2 (−17.3%)** | | use_direct_reads=true (existing global) + write-side | 249,014 (2.5%) — +6.7% | 15.95 (−0.9%) | 68.78 (−16.4%) | **450.8 (−37.5%)** | 1,814.5 (−13.7%) | CV is 2.4–3.3% on the optimized configs (8.2% on buffered), so the deltas are real. With a hot set this small, the existing `use_direct_io_for_flush_and_compaction` knob is already doing most of the work — the new flag's main extra contribution here is P99.99 (combined wins it by ~2 points vs writes-only-alone). Worth noting: the new flag *alone* (without the existing write-side flag) improves P99.99 but regresses P99 by 25% on this small-hot-set workload, because direct compaction reads lose kernel readahead and compaction-output writes are still hitting the page cache. That regression goes away once you combine with the existing write-side flag, or once the hot set is bigger (see next table). So if you're using just one knob, use the existing one. If you're using this PR's flag, pair it with `use_direct_io_for_flush_and_compaction=true`. #### Larger hot set: ~400 MB (~40% of cache) — N=5 iterations, mean (CV) `--num=100000`. This is the case the Cassandra blog calls out — hot set big enough to actually fight compaction for cache. Their analogous setup (1M hot partitions, ~33% hot/cache) reported 1.93× p99 improvement. Numbers here are the headline: | Config | Throughput (ops/s) | Read P50 (µs) | Read P99 (µs) | Read P99.9 (µs) | Read P99.99 (µs) | |---|---|---|---|---|---| | buffered (default) | 68,959 (7.7%) | 44.81 | 541.22 | 2,225.2 | 11,334.5 | | direct_compaction_writes_only (existing knob alone) | 73,973 (10.3%) — +7.3% | 42.22 (−5.8%) | 456.27 (−15.7%) | 2,016.9 (−9.4%) | 9,190.0 (−18.9%) | | direct_compaction_read_only (new knob alone) | 84,337 (2.3%) — +22.3% | 38.66 (−13.7%) | 386.97 (−28.5%) | 1,644.8 (−26.1%) | 4,837.9 (−57.3%, 2.34×) | | direct_compaction_read_write (new + existing, recommended) | **104,923 (8.4%) — +52.2%** | **34.26 (−23.5%)** | **290.97 (−46.2%)** | **1,143.4 (−48.6%)** | **3,080.3 (−72.8%, 3.68×)** | | use_direct_reads=true (existing global) + write-side | 71,598 (9.1%) — +3.8% | 51.33 (+14.5%) | 297.91 (−45.0%) | 1,663.6 (−25.2%) | 6,530.0 (−42.4%) | Combined config gets a 3.68× p99.99 win, 1.86× p99, p50 down 23%, throughput up 52%. Same shape as the Cassandra blog's 1.93× p99 result — the improvement just lands at deeper percentiles for us because RocksDB's baseline data path is roughly 40× faster than Cassandra's (their buffered p99 was 35 ms, ours is 0.54 ms), so the cache-miss tail is further out. A few things worth calling out from this table: - The new flag is doing real work on top of the existing write-side flag here, not just shifting things around. Combined throughput is +42% over `direct_compaction_writes_only` alone, and combined p99.99 is 3× better. The existing knob alone gives a fairly modest +7% throughput / -19% p99.99 in this case — there's a clear gap that the new flag fills. - The new flag *alone* (no existing write-side flag) is also a real improvement here: +22% throughput, p99.99 down 57%. The P99 regression we saw in the small-hot-set case is gone, because the cache-protection effect now dominates the lost-readahead cost. - `use_direct_reads=true` (the existing global flag) actually regresses P50 by 14.5% in this workload — taking user reads off the page cache hurts you when the hot data could have been cached. It also gets the worst throughput of any direct config. It's not an equivalent way to get these gains. ### `compaction_readahead_size` matters when this flag is on Direct I/O bypasses kernel readahead, so RocksDB's own `DBOptions::compaction_readahead_size` becomes the only prefetch the iterator has. The default of 2 MB is enough and real users will get it automatically. **But `db_bench`'s `--compaction_readahead_size` CLI default is 0**, which defeats prefetch and makes direct compaction look slower than it actually is. If you're reproducing the numbers above, pass `--compaction_readahead_size=2097152` (or larger). - Recommended production config is `use_direct_io_for_compaction_reads=true` + `use_direct_io_for_flush_and_compaction=true`. Strongest configuration at every percentile and throughput in both benches. - The new flag is the read-side counterpart to `use_direct_io_for_flush_and_compaction`, which handles compaction-write cache pollution. They address different sources of pollution and compose. The gap between "combined" and "writes-only-alone" is 17 percentage points on p99.99 in the small-hot-set bench and 54 points in the larger one, so the new flag is contributing real value, especially as the hot set grows. - The new flag alone is also a real improvement when the hot set is big enough to compete with cache (+22% throughput, 2.34× p99.99 in the larger-hot-set bench). On a very small hot set it improves p99.99 but regresses p99, so pairing with the existing write-side flag is safer. - The benefit is workload-dependent. Small hot sets get modest tail-latency wins. Hot sets sized to actually compete for cache get the big multi-percentile wins shown above. Hot sets bigger than cache (not benched here but covered in the Cassandra blog) see no change either way — every read misses regardless. ### Reproducing Any Linux host (or a Linux VM on macOS via OrbStack / Multipass / lima): ```bash sudo apt-get install -y build-essential clang cmake git pkg-config \ libgflags-dev libsnappy-dev zlib1g-dev libbz2-dev liblz4-dev libzstd-dev cmake -DCMAKE_BUILD_TYPE=Release -DPORTABLE=1 -DWITH_GFLAGS=1 -DWITH_TESTS=0 .. make -j db_bench echo 0 | sudo tee /sys/kernel/mm/lru_gen/enabled ``` Build the source DB once, unrestricted memory: ```bash ./db_bench --benchmarks=fillrandom,compact,waitforcompaction,stats \ --db=/path/to/source_db --num=3500000 --key_size=16 --value_size=4096 \ --write_buffer_size=16777216 --target_file_size_base=16777216 \ --max_background_jobs=4 --compression_type=none --cache_size=4194304 \ --max_bytes_for_level_base=67108864 --disable_wal=1 --sync=0 ``` For each config, copy `source_db -> scratch_db`, run `sync && echo 3 > /proc/sys/vm/drop_caches`, then: ```bash sudo systemd-run --scope -p MemoryMax=1G -p MemorySwapMax=0 \ ./db_bench --use_existing_db=1 \ --benchmarks=readwhilewriting,stats --db=/path/to/scratch_db \ --threads=5 --duration=120 --statistics=true --histogram=1 \ --num=7500 --bgwriter_num=3500000 \ --key_size=16 --value_size=4096 \ --write_buffer_size=16777216 --target_file_size_base=16777216 \ --max_background_jobs=4 --compression_type=none \ --cache_size=4194304 --open_files=200 \ --skip_stats_update_on_db_open=true \ --max_bytes_for_level_base=67108864 \ --benchmark_write_rate_limit=209715200 \ --compaction_readahead_size=2097152 \ --rate_limiter_bytes_per_sec=0 \ --use_direct_reads={true|false} \ --use_direct_io_for_compaction_reads={true|false} \ --use_direct_io_for_flush_and_compaction={true|false} ``` For the larger hot-set table, change `--num=7500` to `--num=100000`. The five configs in the tables: - `buffered`: all three flags false. - `direct_compaction_writes_only`: `use_direct_io_for_flush_and_compaction=true`, the other two false. This is what users have today without this PR. - `direct_compaction_read_only`: `use_direct_io_for_compaction_reads=true`, the other two false. - `direct_compaction_read_write`: `use_direct_io_for_compaction_reads=true`, `use_direct_io_for_flush_and_compaction=true`, `use_direct_reads=false`. **Recommended.** - `direct_all`: `use_direct_reads=true`, `use_direct_io_for_flush_and_compaction=true`, `use_direct_io_for_compaction_reads=false`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14743 Reviewed By: pdillinger Differential Revision: D108017601 Pulled By: xingbowang fbshipit-source-id: 4039d490d7e77b476db7a477a2f3d24738db6336 |
||
|
Peter Dillinger
|
62f05627be |
Reduce manifest rotation for foreground metadata ops (#14797)
Summary: Async WAL precreation in https://github.com/facebook/rocksdb/pull/14738 / D105020559 was motivated by slow file creation time on remote storage. MANIFEST does not need the same precreation treatment as WAL because most MANIFEST writes come from background flush and compaction work, but user-facing metadata operations can still pay MANIFEST rotation file creation latency inline. File ingestion performance is a particular concern to some Meta users. Relax the effective MANIFEST rotation limit by 25% for MANIFEST write batches containing any foreground VersionEdit, while keeping background-only flush/compaction batches on the configured or auto-tuned limit. This covers column family manipulation, external file ingestion and import, and DeleteFilesInRange(s). SetOptions remains expected to avoid MANIFEST writes; the test keeps a regression guard for that behavior. The relaxation is intentionally bounded. It reduces the chance that foreground metadata operations create a new MANIFEST inline, while still allowing foreground operations to rotate once the current MANIFEST is beyond the relaxed threshold. Heavier blocking operations like manual Flush or CompactRange already trigger additional file creation and do not get this treatment here, though that could be reconsidered later. This should reduce a potential latency hazard of manifest file size auto-tuning: more frequent MANIFEST rotations. With this change, rotation latency is shifted toward background-only MANIFEST batches when possible. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14797 Test Plan: Expanded DBEtc3Test.AutoTuneManifestSize to cover the foreground threshold behavior and the original auto-tuning behavior in separate phases: - verifies foreground-only CreateColumnFamily writes get only bounded 25% headroom by asserting the first four large-CF additions do not rotate and the fifth does; - verifies auto-tuned background thresholds still prevent excessive rotation; - verifies foreground operations stay below the relaxed threshold for CreateColumnFamily, IngestExternalFile, CreateColumnFamilyWithImport, and DeleteFilesInRanges; - verifies SetOptions still does not write to MANIFEST; - verifies a following background flush still rotates at the normal threshold; - preserves the persisted compacted manifest size close/reopen coverage. Reviewed By: xingbowang Differential Revision: D106578771 Pulled By: pdillinger fbshipit-source-id: f8e274032cd9e7f50e95b685c949242f95351498 |
||
|
Xingbo Wang
|
d7afd3bb3b |
Sync recovery SST directory before reused MANIFEST append (#14780)
Summary: - When `reuse_manifest_on_open` reuses the current MANIFEST, `DB::Open` recovery can flush WAL data into a new L0 SST and append the corresponding `VersionEdit` to that already-current MANIFEST. - If open later fails and the process crashes, the MANIFEST edit can be durable while the recovered SST directory entry is not, leaving the DB pointing at a missing SST. - Fsync the recovered SST's data directory before adding the file to the recovery edit when appending to a reused MANIFEST. - Add a regression test that injects failure after MANIFEST sync, simulates crash cleanup of files created after the last directory sync, and verifies the recovered key remains readable. ## Task - T272584339 Pull Request resolved: https://github.com/facebook/rocksdb/pull/14780 Test Plan: CI Reviewed By: anand1976 Differential Revision: D106201774 Pulled By: xingbowang fbshipit-source-id: a44a7d1263d5bc1d82b995c90eef1a825eab4182 |
||
|
Peter Dillinger
|
87c554b492 |
Persist compacted manifest size for auto-tuning across DB::Open (#14725)
Summary: last_compacted_manifest_file_size_ drives TuneMaxManifestFileSize() to compute the manifest rotation threshold, but it started at 0 on every DB::Open and was only populated after the first manifest rotation. This is really only a problem with reuse_manifest_on_open, because no fresh manifest is created on open. Add a new forward-compatible (safe-to-ignore) MANIFEST tag kLastCompactedManifestFileSize that records the approximate compacted manifest size at the end of WriteCurrentStateToManifest. During recovery, the value is loaded and used to immediately tune the rotation threshold. The record includes a rough estimate of its own overhead (~15 bytes) and must be the last record written by WriteCurrentStateToManifest for accurate estimation. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14725 Test Plan: Extended AutoTuneManifestSize in db_etc3_test to close and reopen with reuse_manifest_on_open after establishing a known auto-tuning state. Verifies that the manifest file number is preserved (no spurious rotation) and that subsequent CF additions don't trigger rotation -- proving the persisted compacted size keeps the tuned threshold correct. Verified the test fails when the recovery loading is disabled. Relax a fragile Java test that was dependent on the exact size of the manifest file. SHORT_TEST=1 ./tools/check_format_compatible.sh Reviewed By: anand1976 Differential Revision: D104464522 Pulled By: pdillinger fbshipit-source-id: 4f5d22d2e149bd40a523ee11780e5e3344803c19 |
||
|
Xingbo Wang
|
4707775ae9 |
Fix GetContext status propagation and blob-backed wide-column merge operands (#14640)
Summary: - propagate lower-level read and merge failures through `GetContext` via `read_status`, so `Get` and `GetEntity` preserve the original error instead of synthesizing `Corruption` when blob-backed reads or merge resolution fail - teach `GetMergeOperands` to resolve blob-backed default columns from wide-column entities, covering both the direct base-value path and the merge-plus-base path - add regression coverage for blob-read IO errors during `Get`/`GetEntity` merge resolution and for `GetMergeOperands` on blob-backed wide-column entities - fix the `DBFlushTest.MemPurgeCorrectLogNumberAndSSTFileCreation` test race by waiting for flush callbacks and cleaning up sync points ## Testing - `make db_blob_basic_test -j14` - `/usr/bin/perl -e 'alarm shift; exec ARGV' 60 ./db_blob_basic_test --gtest_filter='DBBlobBasicTest/DBBlobBasicIOErrorTest.GetBlob_IOError/*:DBBlobBasicTest/DBBlobBasicIOErrorTest.GetEntityMergeWithBlobBaseIOError/*'` ## Task T265824017, T265415808 Pull Request resolved: https://github.com/facebook/rocksdb/pull/14640 Reviewed By: anand1976 Differential Revision: D101690700 Pulled By: xingbowang fbshipit-source-id: 2b6fc357b37a01efa72a2d54dcff55be8992f42a |
||
|
Peter Dillinger
|
795f3bd61f |
Fix check-sources.sh non-ASCII check and remove non-ASCII from sources (#14729)
Summary: The non-ASCII character check in check-sources.sh used git grep -P (Perl regex), which requires git compiled with PCRE support. On systems without it, the command fails with exit code 128, which is != 1 (no match), so the check always reported a violation -- effectively dead. Even in CI where git has PCRE2 support, the check was silently broken: git grep -P uses PCRE2 in UTF mode by default, which interprets [\x80-\xFF] as a Unicode codepoint range (U+0080 to U+00FF). Characters like em-dash (U+2014), arrows (U+2192), and math symbols (U+2248, etc.) fall outside that range and were not detected. Only Latin-1 Supplement characters (U+0080-U+00FF) would have been caught. Replace with LC_ALL=C git grep using bash $'[\x80-\xff]' literal byte range, which works with basic regex in the C locale, and replace all non-ASCII characters in non-excluded source files: - em-dash to -- - arrow to -> - math symbols to ASCII equivalents (~=, <=, >=) - box-drawing characters to ASCII art Also exclude .github/ from the check, as scripts there can use non-ascii without disrupting RocksDB builds on non-UTF-8 systems. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14729 Test Plan: manual / CI (make check-sources passes clean) Reviewed By: hx235 Differential Revision: D104692574 Pulled By: pdillinger fbshipit-source-id: 1d884c21056dcd83558b825a04b867f1c08e3f45 |
||
|
Josh Kang
|
e07ccc3528 |
block GetCreationTimeOfOldestFile on async file open completion (#14723)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14723 ### Context `GetCreationTimeOfOldestFile()` assumed `max_open_files` = -1 meant every live SST had a pinned table reader. That is not true with `open_files_async`: recovery intentionally skips loading table files, `DB::Open()` returns, and `BGWorkAsyncFileOpen()` pins readers later. Any caller — e.g. fb_rocksdb's daily report at `FbRocksDb.cpp:1025` — invoking the API in the window between `DB::Open` returning and the background opener completing trips a debug assert. Reported in https://fb.workplace.com/groups/rocksdb/permalink/31668956482726231/. Removing the assert alone is insufficient. For legacy DBs whose manifest does not carry `file_creation_time`, `FileMetaData::TryGetFileCreationTime()` falls back to the pinned reader; with no reader, it returns `kUnknownFileCreationTime` and the function silently returns 0 (the "info unavailable" sentinel). The caller cannot distinguish "no info" from "raced with async open." ### Changes - Remove the invalid debug assert in `Version::GetCreationTimeOfOldestFile`. - Add a private helper `DBImpl::WaitForAsyncFileOpen()` that blocks on `bg_cv_` while `bg_async_file_open_state_ == kScheduled`. The synchronization machinery (`bg_async_file_open_state_` + `bg_cv_`) already exists — the destructor wait loop in `db_impl.cc:674-687` uses the same pattern. The helper is a no-op when `open_files_async = false`, and bails on `shutting_down_` so `DB::Close()` is not blocked by an in-flight caller. - Call `WaitForAsyncFileOpen()` at the top of `DBImpl::GetCreationTimeOfOldestFile()` (inside the `max_open_files == -1` branch). - Document the blocking behavior in `include/rocksdb/db.h`. - Replace the regression test with one that uses a `DBImpl::WaitForAsyncFileOpen::BeforeWait` sync point: spawn a thread that calls `GetCreationTimeOfOldestFile`, deterministically confirm it blocks inside the wait, release async open, confirm the caller wakes with the real value. ### Potential Followups (not included here) - Apply the same wait to `GetLiveFilesMetaData` and `GetColumnFamilyMetaData` — both zero out `oldest_ancester_time` / `file_creation_time` in the SST metadata they return during the async-open window (`db/version_set.cc:7877-7878`, `2090-2091`, `2172-2173`). - Address compaction-picker effects: TTL/periodic file selection (`db/version_set.cc:4039`, `4092-4094`), bottommost over-marking (`db/version_set.cc:4700-4701`), FIFO TTL/temperature pickers (`db/compaction/compaction_picker_fifo.cc:105-107`, `167-170`, `401-411`), and tiered-compaction output time inheritance (`db/compaction/compaction.cc:981`, `1000`). - Harden `FbRocksDb.cpp:1025` to check `status.ok()` instead of `status.code() != kNotSupported`. Reviewed By: mszeszko-meta Differential Revision: D104285992 fbshipit-source-id: ea46375ea1b3ba77fe6b548071aee1101ac0da77 |
||
|
Anand Ananthabhotla
|
c734b7cc60 |
Add reuse_manifest_on_open DBOption (#14704)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14704 Add an immutable DBOption `reuse_manifest_on_open` (default false). When enabled, `DB::Open` can keep using the recovered MANIFEST for the first post-open metadata update instead of rebuilding a fresh MANIFEST, which can reduce warm-open latency for DBs whose MANIFEST is expensive to regenerate. Reuse is still best-effort. If RocksDB cannot safely resume appending to the recovered MANIFEST, it falls back to the existing fresh-MANIFEST path. The option is also disabled under `best_efforts_recovery`. This diff also teaches the reopened MANIFEST writer to adopt the existing file size before appending, documents the small-`max_manifest_file_size` caveat for the reused path, and keeps the full warm-reopen composition working with `optimize_manifest_for_recovery`. Reviewed By: hx235, pdillinger Differential Revision: D103568447 fbshipit-source-id: f4f5c35ea3ef0b80a0d52d94be40c6bd11505999 |
||
|
Anand Ananthabhotla
|
02a2b3501d |
Add optimize_manifest_for_recovery DBOption (#14702)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14702 Add a mutable DBOption `optimize_manifest_for_recovery` (default false) as a temporary rollout / kill switch for warm-reopen MANIFEST optimizations. In this diff, enabling the option lets recovery skip MANIFEST updates during `DB::Open` when the recovered state is already reflected on disk, which reduces metadata appends after a clean shutdown and can lower warm-reopen latency on storage where MANIFEST appends are expensive. If the option is disabled, RocksDB follows the existing recovery path unchanged. The optimization is disabled under `best_efforts_recovery`, where recovery intentionally rewrites metadata as part of salvage, and the option is mutable so later diffs in this stack can share the same rollout knob. Reviewed By: pdillinger, hx235 Differential Revision: D103568448 fbshipit-source-id: 9ec930343e434f1bee6130bcdbd7738dddd92b6d |
||
|
Anand Ananthabhotla
|
4e2fe35bbb |
Gate file_open_metadata consumption on fast_sst_open option (#14676)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14676 When fast_sst_open is disabled, RocksDB was still passing previously-persisted file_open_metadata from the MANIFEST to NewRandomAccessFile. This could cause failures when the metadata becomes stale (e.g. expired filesystem credentials). This change gates the consumption of file_open_metadata in TableCache::GetTableReader on the fast_sst_open option. When fast_sst_open is false, previously persisted metadata is ignored and not passed to the filesystem via FileOptions::file_metadata. The fast_sst_open flag is threaded from MutableDBOptions through VersionSet -> ColumnFamilySet -> ColumnFamilyData -> TableCache at construction time, ensuring the gate is active before any table readers are opened during recovery. Dynamic changes via SetDBOptions are also propagated to all existing TableCache instances. Reviewed By: mszeszko-meta, xingbowang Differential Revision: D102735581 fbshipit-source-id: 9a2c4dc0644a2f65c36b2468605df57779e127cd |
||
Danny Chen
|
2e7cf42cda |
Add MANIFEST_VALIDATION_FAILURE_COUNT statistic (#14657)
Summary: CONTEXT: The manifest validation on close feature (verify_manifest_content_on_close) detects corruption but does not increment any statistics counter, making it harder to monitor in production. WHAT: Add a new ticker MANIFEST_VALIDATION_FAILURE_COUNT that is incremented each time content validation detects manifest corruption during DB::Close(). The counter fires per corruption detection, so it can increment up to 2 times per close (once on initial check, once after rewrite attempt). Updated all existing manifest validation tests to verify the counter value. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14657 Test Plan: - All 7 manifest validation tests pass with new stat assertions - 5x repeat with COERCE_CONTEXT_SWITCH=1 shows no flakiness - Full version_set_test suite (212 tests) passes Reviewed By: anand1976 Differential Revision: D102404260 Pulled By: dannyhchen fbshipit-source-id: 21a0aa1ad8de12a935caf5642e41ccf2a47b46d9 |
||
|
Xingbo Wang
|
af4e32945b |
Blob direct write v1: write-path blob separation with partitioned files (reduced scope) (#14535)
Summary: This PR introduces **blob direct write v1**, a reduced-scope write-path optimization where large values (>= `min_blob_size`) are written directly to blob files during `Put()` and replaced in the memtable with compact `BlobIndex` references. This avoids holding full values in memory until flush time. ### Motivation In the existing BlobDB architecture, values are written to the WAL and memtable in their full form and separated into blob files only at flush time. This means: - Large values are held in memory twice (raw in memtable + blob file at flush) - Blob I/O is serialized through a single flush thread per column family Blob direct write addresses both: values leave the write path as small `BlobIndex` references, and multiple **partitions** (configurable via `blob_direct_write_partitions`) allow concurrent blob writes with independent locks. ### Design (v1 — single-writer, WAL-disabled, reduced scope) The v1 design intentionally keeps scope narrow for correctness and reviewability: - **Single writer thread assumption**: no concurrent writes to the same partition file. One logical writer serializes the batch. - **WAL-disabled**: direct-write blob files are only registered in MANIFEST at flush time. WAL replay cannot recover unregistered blob references, so WAL is disabled for this v1. - **Flush-on-write**: each `AddRecord` call flushes to the OS immediately. - **FIFO generation batching**: each memtable switch creates one generation batch. Direct-write files for that memtable are sealed and registered atomically when the batch is flushed to MANIFEST. - **Round-robin partitions**: blob writes are distributed across `blob_direct_write_partitions` files using an atomic counter. ### New components | Component | Description | |---|---| | `BlobFilePartitionManager` | Owns N partition files per CF. Manages open/seal/register lifecycle tied to memtable generations. | | `BlobWriteBatchTransformer` | A `WriteBatch::Handler` that rewrites qualifying `Put` values as `BlobIndex` entries before the batch enters the write group. | ### Write path integration 1. `DBImpl::WriteImpl` calls `BlobWriteBatchTransformer::TransformBatch` before entering the writer group (for default write path), or before joining the batch group (for pipelined/unordered write). 2. Values >= `min_blob_size` are written to a partition file; the key is stored with a `BlobIndex` in the transformed batch. A rollback guard marks blob bytes as initial garbage if the write fails. 3. On `SwitchMemtable`, `RotateCurrentGeneration` moves active partitions into the next immutable batch. 4. `FlushMemTableToOutputFile` / `AtomicFlushMemTablesToOutputFiles` call `PrepareFlushAdditions` to seal partition files and collect `BlobFileAddition` + `BlobFileGarbage` entries registered to MANIFEST alongside the flush. 5. Shutdown paths (`CancelAllBackgroundWork`, `WaitForCompact` with `close_db=true`) force-flush all CFs with active direct-write managers to ensure blob files are registered before close. ### Read path - **Get/MultiGet**: `MaybeResolveBlobForWritePath` resolves `BlobIndex` references found in memtable or immutable memtable via `BlobFilePartitionManager::ResolveBlobDirectWriteIndex`, which first checks manifest-visible state and falls back to direct blob-file reads via `BlobFileCache`. - **Iterator**: `DBIter::BlobReader` is extended with a `BlobFilePartitionManager*` to resolve direct-write blob indexes during iteration. The unified `ResolveBlobDirectWriteIndex` path handles both manifest-visible and not-yet-flushed files. ### New options | Option | Default | Description | |---|---|---| | `enable_blob_direct_write` | `false` | Enable write-path blob separation for this CF. Requires `enable_blob_files = true`. Not dynamically changeable. | | `blob_direct_write_partitions` | `1` | Number of parallel partition files per CF. Not dynamically changeable. | ### Feature incompatibilities (reduced v1 scope) The following features are *not supported* when `enable_blob_direct_write = true`, and are enforced both in `db_stress_tool` validation and `db_crashtest.py` sanitization: **Write model constraints:** - `threads` must be 1 (single writer assumption) - `allow_concurrent_memtable_write` = 0 - `enable_pipelined_write` = 0 (transformation done before batch group, but pipelined path supported with pre-transform) - `two_write_queues` = 0 - `unordered_write` = 0 (transformation done before batch group, but unordered path supported with pre-transform) **WAL and recovery:** - `disable_wal` = 1 (required — WAL replay of unregistered blob files is out of v1 scope) - `best_efforts_recovery` = 0 - `reopen` = 0 (no crash-restart with WAL replay) - All WAL-related stress features disabled: `manual_wal_flush_one_in`, `sync_wal_one_in`, `lock_wal_one_in`, `get_sorted_wal_files_one_in`, `get_current_wal_file_one_in`, `track_and_verify_wals`, `rate_limit_auto_wal_flush`, `recycle_log_file_num` **Blob GC and dynamic options:** - `use_blob_db` = 0 (stacked BlobDB not supported) - `allow_setting_blob_options_dynamically` = 0 - `enable_blob_garbage_collection` = 0 - `blob_compaction_readahead_size` = 0 - `blob_file_starting_level` = 0 **Unsupported value types and APIs:** - Merge (`use_merge`, `use_full_merge_v1`) — merge values pass through untransformed - Entity APIs (`use_put_entity_one_in`, `use_get_entity`, `use_multi_get_entity`, `use_attribute_group`) - `use_timed_put_one_in` - User-defined timestamps (`user_timestamp_size`, `persist_user_defined_timestamps`, `create_timestamped_snapshot_one_in`) - Transactions (`use_txn`, `use_optimistic_txn`, `test_multi_ops_txns`, `commit_bypass_memtable_one_in`) — though `WriteCommittedTxn::CommitInternal` falls back from bypass-memtable to normal path when BDW is active - `IngestWriteBatchWithIndex` returns `NotSupported` - `inplace_update_support` = 0 **Fault injection:** - All write/read/metadata fault injection disabled (`sync_fault_injection`, `write_fault_one_in`, `metadata_write_fault_one_in`, `read_fault_one_in`, `metadata_read_fault_one_in`, `open_*_fault_one_in`) **Infrastructure/snapshot APIs:** - `remote_compaction_worker_threads` = 0 - `test_secondary` = 0 - `backup_one_in` = 0 - `checkpoint_one_in` = 0 - `get_live_files_apis_one_in` = 0 - `ingest_external_file_one_in` = 0 - `ingest_wbwi_one_in` = 0 ### Tests - `db/blob/db_blob_basic_test.cc`: ~660 lines of new direct-write unit tests covering basic put/get, multi-partition, flush/compaction, recovery, and error injection. - `db/blob/blob_file_cache_test.cc`: ~96 lines of new tests for direct-write blob file cache behavior. - `db/write_batch_test.cc`: ~96 lines of tests for WriteBatch with blob index entries. - `utilities/transactions/transaction_test.cc`: verifies transaction commit path falls back correctly with direct write enabled. - `db_stress_tool/`: full stress test support with `--enable_blob_direct_write` and `--blob_direct_write_partitions` flags, integrated into `db_crashtest.py` with 10% random selection alongside regular blob params. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14535 Test Plan: ``` make -j128 db_blob_basic_test && ./db_blob_basic_test make -j128 blob_file_cache_test && ./blob_file_cache_test make -j128 write_batch_test && ./write_batch_test make -j128 transaction_test && ./transaction_test make -j128 check ``` Stress test: ``` python3 tools/db_crashtest.py blackbox --enable_blob_direct_write=1 \ --enable_blob_files=1 --blob_direct_write_partitions=4 \ --disable_wal=1 --threads=1 ``` Reviewed By: pdillinger Differential Revision: D98766843 Pulled By: xingbowang fbshipit-source-id: 1577653826913a59d05680a87bce5534ac5a5e69 |
||
|
Josh Kang
|
5db0603613 |
Read-triggered compactions (#14426)
Summary: Add read-triggered compaction, a new feature that reduces read amplification by compacting SST files that receive high read traffic. When an SST file's read frequency (`num_reads_sampled / file_size`) exceeds a configurable threshold, it is marked for compaction to a lower level. The feature introduces two new options: a CF option `read_triggered_compaction_threshold` (default 0, disabled) and a DB option `max_periodic_compaction_trigger_seconds` (default 43200s) that controls how often the background thread re-evaluates compaction scores on quiet databases. Both options are dynamically changeable. Lowering `max_periodic_compaction_trigger_seconds` does add some overhead, but generally is minimal, so running this every couple of minutes in a production environment seems fairly reasonable. ## Key changes - **New CF option `read_triggered_compaction_threshold`** (`advanced_options.h`): When positive, files with `reads_per_byte > threshold` are marked for compaction. Files at the last non-empty level are skipped (bottommost compaction handles those separately). Marked files are sorted by hotness (reads_per_byte descending). - **New DB option `max_periodic_compaction_trigger_seconds`** (`options.h`): Replaces the hardcoded 12-hour ceiling in `ComputeTriggerCompactionPeriod()`. Essential for read-triggered compaction on quiet DBs since there are no writes to trigger score re-evaluation. - **Leveled compaction picker** (`compaction_picker_level.cc`): Adds read-triggered as the lowest-priority compaction reason in `SetupInitialFiles()`, using the existing `PickFileToCompact` helper. - **Universal compaction picker** (`compaction_picker_universal.cc`): Adds `PickReadTriggeredCompaction` as lowest priority. Refactors shared "find output level + compute overlapping inputs + create Compaction" logic from both `PickDeleteTriggeredCompaction` and `PickReadTriggeredCompaction` into `BuildCompactionToNextLevel`, handling both single-level and multi-level universal cases. - **Periodic trigger integration** (`db_impl.cc`): `TriggerPeriodicCompaction` now also fires for CFs with `read_triggered_compaction_threshold > 0`, even without time-based compaction configured. - **Stress test & db_bench support**: Both `db_stress` and `db_bench` support the new options. `db_crashtest.py` randomly enables read-triggered compaction and sets a short periodic trigger interval when enabled. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14426 Test Plan: **Unit tests**: - `compaction_picker_test` — 7 new tests: `ReadTriggeredCompactionDisabled`, `ReadTriggeredCompactionBelowThreshold`, `ReadTriggeredCompactionAboveThreshold`, `NeedsCompactionReadTriggered`, `ReadTriggeredPicksFile`, `UniversalReadTriggeredCompaction`, `ReadTriggeredSkipsLastLevel`, `UniversalReadTriggeredNoPickWhenNotMarked` - `db_compaction_test` — `ReadTriggeredCompaction` integration test verifying end-to-end behavior with sync points - Stress test coverage **Stress test**: ``` make V=1 -j "CRASH_TEST_EXT_ARGS=--duration=600 --max_key=2500000 --max_compaction_trigger_wakeup_seconds=10 --read_triggered_compaction_threshold=0.0001 --interval=600" blackbox_crash_test ``` - confirmed read triggered compactions from LOGS **Benchmark** (`db_bench`): Setup: 5M keys (100B values, 16B keys), leveled compaction, 5 levels, 4MB target file size. DB fully compacted, then 2M overlapping keys written without compaction to create L0/L1 overlap (82 files, ~294MB). LSM shape change during readrandom with read-triggered compaction: ``` BEFORE: L0=9 files (15MB), L1=4 (16MB), L2=20 (69MB), L3=49 (194MB) — 82 files, 294MB AFTER: L3=66 files (223MB) ``` | Benchmark | Config | avg ops/s | % change | |-----------|--------|-----------|----------| | readrandom (8 threads, 5M reads) | baseline (threshold=0) | 1,086,965 | — | | readrandom (8 threads, 5M reads) | threshold=0.000001, trigger=5s | 1,453,697 | **+33.7%** | Reviewed By: xingbowang Differential Revision: D97838716 Pulled By: joshkang97 fbshipit-source-id: a21fcb270c7fadd4f78d98b9c821982f220dd3f0 |
||
|
Danny Chen
|
b6f498b2c9 |
Add verify_manifest_content_on_close option (#14451)
Summary: Add a new mutable DB option `verify_manifest_content_on_close` (default: false). When enabled, on DB close the MANIFEST file is read back and all records are validated (CRC checksums via log::Reader and logical content via VersionEdit::DecodeFrom). If corruption is detected, a fresh MANIFEST is written from in-memory state using the existing LogAndApply recovery path. This complements the existing size validation in VersionSet::Close() with content validation, reusing the same manifest reading pattern as VersionSet::Recover(). Implementation plan: ## Part 1: New DB Option — verify_manifest_content_on_close - A new mutable bool DB option (default: false) that can be dynamically toggled via SetDBOptions() at runtime, following the pattern of other mutable manifest options like max_manifest_file_size. - Propagation: SetDBOptions() -> DBImpl::mutable_db_options_ -> versions_->UpdatedMutableDbOptions() -> VersionSet::verify_manifest_content_on_close_ ## Part 2: Core Implementation — Content Validation in VersionSet::Close() - Inserted after existing size check, before closed_ = true - Opens manifest as SequentialFileReader, creates log::Reader with checksum=true - Loops ReadRecord with WALRecoveryMode::kAbsoluteConsistency, decodes each record as VersionEdit - On corruption: fires OnIOError listeners, logs error, calls LogAndApply with empty edit to trigger manifest rewrite from in-memory state - If manifest can't be opened for reading: logs warning, doesn't fail close ## Part 3: Unit Tests (in version_set_test.cc) - ManifestContentValidationOnClose_Clean: enable option, normal close, verify no manifest rotation - ManifestContentValidationOnClose_CorruptRecord: enable option, corrupt manifest via SyncPoint, verify rotation occurs and DB reopens cleanly - ManifestContentValidationOnClose_Disabled: default off, verify content validation does not run - ManifestContentValidationOnClose_SizeCheckFails: truncate manifest so size check fails first, verify recovery via size-check path ## What Happens If a Corruption is Detected If corruption was detected, four things happen: 1. **Notify listeners** — Fires `OnIOError` on all registered event listeners (from db_options_->listeners) so monitoring/alerting systems can observe the corruption event. Uses `FileOperationType::kVerify` to categorize it. 2. **Permit unchecked errors** — `PermitUncheckedError()` silences RocksDB's debug-mode assertion that every `IOStatus` must be inspected. These statuses are informational-only here; the real recovery is via `LogAndApply`. 3. **Log the error** — Writes a `ROCKS_LOG_ERROR` message with the filename for operational visibility (grep-able in production logs). 4. **Rewrite the manifest via `LogAndApply`** — This is the actual recovery. `LogAndApply` is called with an empty `VersionEdit` (no changes). Internally, `LogAndApply` detects that the current `descriptor_log_` is null (it was reset at line 5551, or by the previous `LogAndApply` in the size-check path) and creates a brand-new MANIFEST file. It serializes the entire current in-memory LSM state — all column families, all levels, all file metadata, sequence numbers, etc. — into this new file. It then atomically updates the `CURRENT` file pointer to reference the new MANIFEST. This works because the in-memory state was built from the original manifest during `DB::Open()` and has been kept fully up to date through all subsequent operations (flushes, compactions, etc.) during the DB's lifetime. The on-disk manifest is essentially a journal of changes; `LogAndApply` with an empty edit produces a fresh, compacted snapshot of that state. ## Flow Diagram of Manifest Content Validation VersionSet::Close() │ ├─ Close descriptor_log_ and check size │ └─ Size mismatch? → LogAndApply (rewrite manifest) │ ├─ Content validation (if s.ok() && option enabled) │ ├─ Open manifest for sequential reading │ │ └─ Can't open? → WARN log, continue │ │ │ ├─ For each record: │ │ ├─ ReadRecord (CRC32 check, kAbsoluteConsistency) │ │ └─ DecodeFrom (VersionEdit logical check) │ │ │ └─ Corruption detected? │ ├─ Notify OnIOError listeners │ ├─ LOG_ERROR │ └─ LogAndApply (rewrite manifest from in-memory state) │ └─ closed_ = true; return s; ## How This Relates to the Existing Size Check The existing size check (lines 5556-5582) and the new content validation are complementary: | Check | What it catches | How it checks | |----------------|-----------------------------------------|----------------------------| | Size check | Truncation, partial writes, extra bytes | Compare expected vs actual file size | | Content check | Bit-rot, silent corruption, bad records | CRC32 + VersionEdit decode | The size check catches gross corruption (file too short or too long). The content check catches subtle corruption where the file is the right size but individual bytes have been flipped (e.g., storage media bit-rot, buggy filesystem, incomplete block write). Both recovery paths use the same mechanism: `LogAndApply` with an empty `VersionEdit` to rewrite the manifest from in-memory state. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14451 Reviewed By: xingbowang Differential Revision: D96004906 Pulled By: dannyhchen fbshipit-source-id: 0b0ecdada3a74e97d2cadbba2091b8b577f1d684 |
||
|
Josh Kang
|
42eff8b632 |
Add new heauristic 'num_collapsible_entry_reads_sampled' (#14434)
Summary: Add per-file sampling of "collapsible" entry reads (single deletions, merges, and kNotFound results) that may later be used to help inform read-triggered compactions. This is a better metric than `num_reads_sampled` as it is more targeted towards reads that could be avoided via compaction. The existing behavior of `num_reads_sampled` is that reads only gets sampled on iterator creation for a file. It is problematic because next/prev() calls are not sampled, nor are additional seeks(). This PR moves sampling to per-seek/next granularity within `LevelIterator` and adds a new `num_collapsible_entry_reads_sampled` counter that tracks how often a file serves entries that could be eliminated by compaction. Note only L1+ files have iterator seeks/nexts/prevs sampled. Introducing this at L0 would require wrapping table reader iterators, introducing a performance cost. ## Key changes - **New counter `num_collapsible_entry_reads_sampled`** in `FileSampledStats` tracks sampled reads that encounter deletions, single deletions, merges, or kNotFound results in both Get and Iterator paths. - **Moved sampling from file-open to per-operation** in `LevelIterator`: sampling now happens in `SampleRead()` called from `Seek()`, `SeekForPrev()`, `SeekToFirst()`, `SeekToLast()`, `Next()`, `NextAndGetResult()`, and `Prev()`. The `should_sample` parameter was removed from `LevelIterator`'s constructor. - **Differentiated sampling rate for Next() vs Seek()**: `should_sample_file_read_next()` uses a 64x lower sampling rate (`kFileReadSampleRate * 64`) since Next() is cheaper than Seek() and called more frequently. - **Collapsible tracking in Get path**: `Version::Get()` now increments the collapsible counter when `GetContext::State()` is `kNotFound`, `kMerge`, or `kDeleted`. - **Collapsible tracking in MultiGet path**: `MultiGetFromSST` also increments the collapsible counter for the same states. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14434 Test Plan: - Added new DB tests for both num_reads_sampled and num_collapsible_entry_reads_sampled ### Benchmark results (readrandom, readseq) Setup: 1M keys, 16-byte keys, 100-byte values, no compression, fillrandom+compact | Benchmark | Params | ops/s (main) | ops/s (feature) | % change | |------------|--------------------|-------------|--------------------------|----------| | readrandom | seed=1, threads=1 | 387,194 | 389,449 | +0.6% | | readseq | seed=1, threads=1 | 5,598,371 | 5,572,975 | -0.5% | No meaningful performance regression observed — differences are within run-to-run noise. Reviewed By: xingbowang Differential Revision: D95613793 Pulled By: joshkang97 fbshipit-source-id: 9dd09c9b7527b148424bde5686f4157c7a9e1214 |
||
|
Josh Kang
|
f25fb41da6 |
Add option to validate sst files in the background on DB open (#14322)
Summary: Add `open_files_async` option for faster DB startup. When enabled, SST file opening and validation is deferred to a background thread after `DB::Open` returns, reducing startup latency for databases with many SST files. WAL recovery remains synchronous. To support this, `FindTable` is extended with a pinning mechanism that stores the cache handle directly on `FileMetaData` via a new `PinnedTableReader` class, and sets the table reader atomically so subsequent reads skip cache lookups. `FileDescriptor::table_reader` is replaced with `PinnedTableReader pinned_reader` which wraps a `std::atomic<TableReader*>` with acquire/release ordering to safely handle concurrent access between the background opener and read threads. Should validations fail, the background opener sets a `kAsyncFileOpen` background error. Future read requests will look up the table reader again via the cache, and if any validations fail there it will get propagated to the user (existing behavior when `max_open_files > 0`). This feature is most useful when `max_open_files=-1`, because otherwise file opening is already capped at 16 files and DB open should be fast. ## Restrictions - This feature also is incompatible with fifo compaction because fifo compaction requires reading table properties under DB mutex. When table reader is unpinned, this may cause a DB hang. - This feature is also incompatible with `skip_stats_update_on_db_open=false` because it will result in even longer DB open ## Key changes - New `open_files_async` DB option with C, Java, and `db_bench` bindings - `BGWorkAsyncFileOpen` background worker that opens all SST files post-`DB::Open`, with shutdown awareness via `shutting_down_` flag - New `PinnedTableReader` class in `version_edit.h` — thread-safe wrapper holding `std::atomic<TableReader*>` and `Cache::Handle*` with proper acquire/release ordering. Replaces the old `FileDescriptor::table_reader` raw pointer and `FileMetaData::table_reader_handle` - Extract `LoadTableHandlersHelper` into `db/version_util.cc` — shared between `VersionBuilder::LoadTableHandlers` (for version edits during recovery) and `BGWorkAsyncFileOpen` (for base storage post-open) - `FindTable` extended with `pin_table_handle` and `out_table_reader` params — when pinning is enabled, the table reader is stored on `FileMetaData` so Get/MultiGet/Iterator skip redundant cache lookups. `FindTable` now performs the pinned-reader fast-path check internally instead of requiring callers to check `fd.table_reader` beforehand - Note: pinning is explicit (not default) because some callers create temporary `FileMetaData`s that would need to properly clean up table handles - `CompactedDBImpl` updated to use `FindTable` + pinning instead of raw `fd.table_reader` access for Get/MultiGet - New `kAsyncFileOpen` background error reason in `listener.h` and `error_handler.cc` - Add a check in ~DBImpl to ensure async file open task has not been forgotten to be scheduled in (future) subclasses of DBImpl. Certain subclasses that never use it will need to explicitly mark it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14322 Test Plan: - `OpenFilesAsyncTest` parameterized over `num_flushes` (1, 20), `ReadType` (Get, MultiGet, Iterator), `max_open_files` (-1, 10), and `read_only` (true, false) - **ConcurrentFileAccess**: concurrent reads and compactions race with async opener - **AfterRead**: reads happen before async opener, verifying lazy open and that the opener sees already-pinned readers - **BeforeRead**: async opener completes first, verifying reads use pre-loaded table readers - **Shutdown**: DB closes before async opener starts, verifying clean cancellation with 0 file opens - **Error**: corrupted SST files, verifying `kAsyncFileOpen` background error is set and reads return corruption - **DropColumnFamily**: CF dropped before async opener runs, verifying the opener gracefully skips dropped CFs - Added to crash test ### Benchmark To simulate a high-latency remote filesystem, I set up a virtual filesystem with dm-delay using 10ms reads, 0 ms writes. ``` # Generate a DB with many L0 files TEST_TMPDIR=/data/users/jkangs/dm-delay-test/mnt ./db_bench -benchmarks=fillseq -disable_auto_compactions=true -write_buffer_size=1000 -num=1000000 ``` ``` ./db_bench -use_existing_db=true -db=/data/users/jkangs/dm-delay-test/mnt/dbbench -benchmarks=readrandom -reads=1 -report_open_timing=true -open_files_async=true -use_direct_reads -file_opening_threads=1 -skip_stats_update_on_db_open OpenDb: 25.1419 milliseconds ``` ``` ./db_bench -use_existing_db=true -db=/data/users/jkangs/dm-delay-test/mnt/dbbench -benchmarks=readrandom -reads=1 -report_open_timing=true -open_files_async=false -use_direct_reads -file_opening_threads=1 -skip_stats_update_on_db_open OpenDb: 23109.4 milliseconds ``` ### No read regressions On main branch ``` ./db_bench -use_existing_db=true -db=/dev/shm/dbbench -benchmarks=readrandom -seed=1 -threads=8 -duration=30 readrandom : 4.827 micros/op 1657100 ops/sec 30.005 seconds 49720992 operations; 183.3 MB/s (6198999 of 6198999 found) ``` On this branch ``` ./db_bench -use_existing_db=true -db=/dev/shm/dbbench -benchmarks=readrandom -seed=1 -threads=8 -duration=30 readrandom : 4.863 micros/op 1644808 ops/sec 30.007 seconds 49354992 operations; 182.0 MB/s (6099999 of 6099999 found) ./db_bench -use_existing_db=true -db=/dev/shm/dbbench -benchmarks=readrandom -seed=1 -threads=8 -duration=30 -open_files_async=true readrandom : 4.803 micros/op 1665392 ops/sec 30.004 seconds 49968992 operations; 184.2 MB/s (6222999 of 6222999 found) ``` Reviewed By: pdillinger, xingbowang Differential Revision: D93538033 Pulled By: joshkang97 fbshipit-source-id: 32ac70c112cd733b7c1e1c1e2e7ce6422318a5ae |
||
|
Andrew Chang
|
09bda51c50 |
Propagate file_checksum through FileOptions on NewRandomAccessFile (#14321)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14321 Add file_checksum and file_checksum_func_name fields to FileOptions so that downstream FileSystem implementations can access per-file checksum metadata when SST files are opened. The fields are populated from FileMetaData at all call sites where SST files are opened via NewRandomAccessFile: TableCache::GetTableReader, Version::GetTableProperties, and CompactionJob::ReadTablePropertiesDirectly. Also fixes the fallback path in TableCache::GetTableReader to use the local fopts (with temperature and checksum) instead of the original file_options. Added a kNoFileChecksumFuncName which is distinct from kUnknownFileChecksumFuncName: - kUnknownFileChecksumFuncName ("Unknown"): We have FileMetaData for this file, and the metadata says no checksum was computed (no factory was configured when the file was written). This is a property of the file itself. - kNoFileChecksumFuncName ("Unavailable"): We don't even have FileMetaData — we're opening this file in a context where there's no checksum metadata to propagate at all (e.g., SstFileDumper, SstFileReader, checksum generation). It's a property of the call site, not the file. So the assertion file_checksum.empty() is correct for both, but for different reasons — one says "the file has no checksum," the other says "we have no idea about this file's checksum." Reviewed By: pdillinger Differential Revision: D92728944 fbshipit-source-id: 8fd34ea22ca87090b26d0a55c921f354f97f1ffc |
||
|
Xingbo Wang
|
b040ab83e1 |
Add a new picking algorithm in fifo compaction (#14326)
Summary: Add a new kv ratio based compaction picking algorithm in fifo compaction Pull Request resolved: https://github.com/facebook/rocksdb/pull/14326 Test Plan: Unit test Reviewed By: pdillinger Differential Revision: D93257941 Pulled By: xingbowang fbshipit-source-id: fd2d0e1356c7b54682a1197475a1bd26cb45c9d4 |