mirror of
https://github.com/facebook/rocksdb.git
synced 2026-10-23 08:09:02 +03:00
45203a867fffed124aedb69801064501ba5206af
14176 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Anand Ananthabhotla
|
45203a867f |
Add retryable ExternalTableBuilder contract (#15342)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15342 Add an ExternalTableBuilderBase::TryAdd() contract so external table builders can distinguish accepted rows, real builder errors, and clean requests to cut the current output table and retry the same row. Add external-table-facing result, reason, and context types. Existing external table builders inherit a default TryAdd() implementation that forwards to Add(), so current implementations keep their historical behavior unless they explicitly opt into retryable file cuts. Reviewed By: xingbowang Differential Revision: D123930815 fbshipit-source-id: b5701c704938759db68b7c0bf0ebeba19d176945 |
||
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 |
||
generatedunixname3846135475516776
|
60f090647d |
Rocksdb Crash Test failed: Callstack - SharedState::SafeTerminate -> NonBatchedOpsStres (#15338)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15338 Reviewed By: jaykorean Differential Revision: D122865369 fbshipit-source-id: 65a67955dd79a68fe1e4f15116d40dc669e1519e |
||
Josh Kang
|
5b77e34985 |
Add range deletion support to external tables (#15334)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15334 External table formats cannot currently persist range tombstones, so SST ingestion and full-mode DB flushes or compactions fail when they contain `DeleteRange` entries. This change adds an opt-in contract for storing RocksDB’s canonical range-deletion block. RocksDB retains responsibility for sequence-number handling, fragmentation, and visibility through its native range-tombstone iterators. Existing factories remain unsupported by default, and existing files without range tombstones are unaffected. Missing or invalid advertised tombstone blocks are treated as corruption. RocksDB also supplies logical entry counts and sizes when a format does not persist the properties block, while preserving format-specific properties. Reviewed By: xingbowang Differential Revision: D123475827 fbshipit-source-id: de8fec401bdc311282f5d4a48b248875c682f7c9 |
||
Andrew Chang
|
3ea1ec56df |
Fix MultiGet reading from disk with kBlockCacheTier (#15321)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15321 `MultiGet` with `read_tier = kBlockCacheTier` now returns `Incomplete` for data blocks that are not in the block cache. Before this fix, it read them from disk. **Why it matters:** Callers use `kBlockCacheTier` to prevent disk I/O. `Get` obeyed it, but `MultiGet` did not. **Problem** - Without mmap reads, `MultiGet` sends cache misses to `RetrieveMultipleBlocks`. - That path called `MultiRead` with no `kBlockCacheTier` check. - `RetrieveBlock` (used by `Get`) has the check. **Fix** - `RetrieveMultipleBlocks`: if `read_tier == kBlockCacheTier`, set `Incomplete("no blocking io")` for each block and do no I/O. - `MultiGet`: when a block is `Incomplete`, return that status for the key, the same as `Get`. Before, the key status stayed `OK` with no value. - Release note in `unreleased_history/bug_fixes`. **Tests** - New `DBTest2.MultiGetBlockCacheTierSkipsColdSstData`: `Get` and `MultiGet` both return `Incomplete` for a key whose data block is not in the cache. - `DBBlobBasicTest.MultiGetBlobsFromCache` relied on the bug. Its inline key passed only because `MultiGet` read the data block from disk. The test now loads the block cache first, so it still checks the blob cache. Reviewed By: joshkang97 Differential Revision: D122431823 fbshipit-source-id: 262913930cb4b570ea0a4d8f4f92364de08a27f5 |
||
Jay Huh
|
f16157cfc5 |
Fix ClipColumnFamily dropping the file that ends at begin_key (#15320)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15320 Why the change is required: `DB::ClipColumnFamily(cf, begin_key, end_key)` keeps `[begin_key, end_key)`. It first drops whole non-L0 files outside the range with `DeleteFilesInRanges` on `(-inf, begin_key)` and `[end_key, +inf)`, but called it with the default `include_end = true`. The first range was therefore treated as `(-inf, begin_key]`, so a non-L0 file whose largest key is `begin_key` was deleted even though that key is inside the kept range. Clipping `[b, d)` on L1 files `[a, b]` and `[c, d]` lost `b`. This showed up while smoke-testing `SplitDB` (D120757697): single-key files trivially moved to the bottom level made the split destination miss its first key. New behavior: - Pass `include_end = false`, so `DeleteFilesInRanges` skips files whose largest key equals `begin_key`. The second range has no end, so it is unaffected. The remaining keys below `begin_key` are still removed by the existing `DeleteRange` and compaction steps. Reviewed By: xingbowang Differential Revision: D123306376 fbshipit-source-id: 65cb36ae730a55750ea558a6cdcd48d8fc17f0e9 |
||
|
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 |
||
|
Josh Kang
|
c0512c4c34 |
Remove deprecated write_global_seqno ingestion support (#15287)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15287 `IngestExternalFileOptions::write_global_seqno` has been deprecated since RocksDB 8.0 and will soon be removed from the API. It requires ingestion to mutate SST files, sync them, regenerate checksums, and expose file-offset plumbing solely for that mutation. Remove the option and its C, Java, and `ldb` APIs. Assigned sequence numbers now live only in the MANIFEST, leaving newly ingested SST bytes and whole-file checksums unchanged. ## Backward Compatibility New readers continue to support existing SSTs containing a nonzero global sequence number written by legacy ingestion. Because that old write did not update the properties-block checksum, checksum verification retries against a temporary copy with only the global-seqno value restored to its original zero placeholder. The parsed sequence number and file contents remain unchanged. This fallback only accepts mismatches explained by the legacy rewrite. Other checksum failures continue through normal corruption handling, and `verify_checksums=false` continues to bypass verification. Reviewed By: xingbowang Differential Revision: D122405856 fbshipit-source-id: 934e9bc755d58ea8d84e1c085e9769015bd8dfd7 |
||
Xingbo Wang
|
5f1132b884 |
Require DB mutex checks in AI code reviews (#15324)
Summary: CONTEXT: The AI review workflow had generic concurrency and performance checks, but it did not require reviewers to determine whether changed code or transitive callees execute while DBImpl::mutex_ is held. Reviews could therefore miss added contention, blocking work, lifetime errors, or snapshot and lock-ordering violations. PREVIOUS: Review agents could examine DB-mutex changes through general correctness and performance lenses, but there was no explicit call-path audit, before/after mutex-held work comparison, or evidence requirement for regressions. NEW: Every non-trivial review must state whether the DB mutex is affected. When it is affected, reviewers must inspect lock boundaries and correctness invariants, compare CPU and I/O before and after the change, keep unavoidable locked work highly optimized, and require measurements for material regressions. IMPLEMENTATION: Add a DB Mutex Review Perspective covering locking contracts, object lifetime, snapshot boundaries, lock ordering, hidden callee costs, blocking work, validation, and performance evidence. Assign its checks to the correctness and performance reviewers and add required context and review checklist entries. SAFETY: This changes only the AI code-review instructions. It does not change RocksDB runtime behavior, APIs, file formats, or compatibility. Changes that cannot execute under the DB mutex only need to state that the mutex is not affected. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15324 Test Plan: - `git show --check HEAD` - `git diff --check upstream/main...2026_10_05_main` - Documentation-only change; no runtime tests were run. Reviewed By: joshkang97 Differential Revision: D123415306 Pulled By: xingbowang fbshipit-source-id: 0f78e0b1d677edf003cd5dc6ed5ff8123aff4f98 |
||
|
Josh Kang
|
c2b86f3ec0 |
Lazily allocate MultiGet coroutine scratch buffers (#15298)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15298 ## Summary BlockBasedTable::MultiGetCoroutine() embedded the synchronous 8 KiB scratch buffer in every coroutine frame, including cache-hit requests. The change uses zero coroutine stack capacity and allocates the required heap size only when I/O is needed, while preserving synchronous behavior. ## Benchmarks | Variant | Coroutine frame allocation | | Before | 21,048 bytes | | After | 12,864 bytes | | Change | -8,184 bytes (-38.9%) | No API, format, or read-result changes. Reviewed By: xingbowang Differential Revision: D122810207 fbshipit-source-id: 003a8349b8da72de2eca2e62fb9f7b3d8802baed |
||
Yoshinori Matsunobu
|
8585299552 |
Rate-limit DestroyDB cleanup of existing trash (#15285)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15285 Why the change is required: MyRocks bulk-load v2 creates a temporary RocksDB and calls DestroyDB when the load is committed, rolled back, abandoned, or recovered. Successfully ingested SSTs are moved into the main DB, so leftover SST cleanup is primarily an error, abort, or recovery case; WALs, obsolete files, and compaction-created trash can still remain. All physical deletion performed by DestroyDB should honor the configured SstFileManager rate, including rocksdb_sst_mgr_rate_bytes_per_sec in MyRocks. Previous behavior: DestroyDB sent recognized table, blob, and WAL files through SstFileManager, but ignored filenames ending in .trash because ParseFileName rejects that suffix. The final best-effort DeleteDir then silently left the directory behind. DB::Open handled the same files through CleanupDirectory, forcing MyRocks cleanup code to discover and register trash itself. A global WaitForEmptyTrash workaround would also drain unrelated compaction, DROP TABLE, and DROP INDEX deletion work from every DB sharing the manager. New behavior: DestroyDB waits only for pending deletion work associated with each DB, data, WAL, and archive directory. It then schedules any remaining .trash file through the existing rate-limited scheduler and waits only for the private bucket created for this DestroyDB call. Directory keys are normalized for bookkeeping, so equivalent path spellings identify the same pending work while filesystem operations retain the original path. Accounting and failure handling: A failed accounted trash deletion retains its original file path and remaining trash-byte charge. If DB open or DestroyDB later discovers the file, the retry preserves the accounted classification, avoids charging the trash bytes twice, and clears SstFileManager tracked-size and trash-size accounting after successful deletion. Scope and safety: There is no new sysvar, public RocksDB API, or persisted-format change. Existing rate settings, hard-link handling, partial deletion, and separate-WAL foreground deletion remain unchanged. The bookkeeping is confined to file enqueue/completion and DestroyDB cleanup. A shared FIFO can still delay this DB behind unrelated work already ahead of its files, but unrelated work queued behind this DB no longer gates DestroyDB. Reviewed By: xingbowang Differential Revision: D122221503 fbshipit-source-id: ec106f57efa70e24827234b4173366e5dbcbda5c |
||
|
Xingbo Wang
|
866e8e9f06 |
Make full error-recovery flush atomic (#15293)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15293 Why the change is required: Full background-error recovery flushes all column families because WAL durability can be uncertain. A later recovery-flush failure or process crash must not persist different WAL replay boundaries for different column families. Otherwise reopen can recover only part of a write batch spanning those column families. The risk includes non-file-scoped WAL write or sync errors. A failed write can have reached the WAL without reaching the memtables. An earlier unsynced write can instead remain in the memtables while its WAL bytes are not durable; retrying the fsync after resetting the writer error does not by itself prove those bytes survived. Sequential recovery can make either case visible in only some column families after reopen. Previous behavior: File-scoped WAL-error recovery already set FlushOptions::force_atomic_flush. Other full kErrorRecovery paths flushed column families sequentially when DBOptions::atomic_flush was false. Each successful per-column-family flush independently advanced its MANIFEST log number, so a later flush failure or process crash could leave mismatched replay boundaries. New behavior: Every full kErrorRecovery atomically commits the flush results and WAL replay boundaries for all participating column families. A successful recovery commits every column family; a failed recovery commits none. If an atomic recovery attempt fails, all of its flush output is discarded and recreated by a later attempt instead of retaining output from column families that finished first. Implementation: Set FlushOptions::force_atomic_flush for every full error-recovery FlushAllColumnFamilies call. Remove the now-unreachable recovery-only behavior from FlushMemTable and assert that recovery does not enter the non-atomic path. Keep the two-write-queue sequence fence at the atomic memtable-switch point. Add an end-to-end fault-injection test for a retryable, non-file-scoped WAL sync error after a multi-column-family record is appended. It covers successful and failed recovery flushes, including a column family with no other memtable data, and verifies all-or-none behavior after reopen. Update the manual_wal_flush comment and add a release note. Safety, compatibility, and limitations: Only full kErrorRecovery changes. Normal, manual, shutdown, and kErrorRecoveryRetryFlush behavior is unchanged. File-scoped WAL-error recovery was already atomic. There are no API, ABI, option-default, or file-format changes. The additional work is limited to the cold recovery path. A failed atomic recovery can redo more flush work than the previous sequential path, but this avoids partial multi-column-family recovery. Automatic recovery with manual_wal_flush remains disabled as fatal pending separate analysis and testing; this change does not remove that workaround. Validation covers local Linux builds and focused recovery tests; platform CI for this version is pending. Reviewed By: pdillinger, rban1 Differential Revision: D122469648 fbshipit-source-id: f72845903d9b6bff70f5d881e02a4cb2e920ed72 |
||
|
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 |
||
xingbowang
|
4cd0450f31 |
Fix big-endian builds for the fault-injection filesystem (#15297)
Summary: The persistent injected-error log writes `InjectedErrorLog::Entry` objects using their native in-memory layout, while the log format's consumer expects little-endian multi-byte fields. The existing `static_assert(port::kLittleEndian)` protected that format by rejecting every big-endian build that included the fault-injection filesystem, even though the log is an optional diagnostic. This change keeps RocksDB and the fault-injection filesystem available on supported big-endian platforms while disabling only the incompatible persistent log. Log-file creation now returns an inactive descriptor when `port::kLittleEndian` is false, which makes `Record()` and `Flush()` remain safe no-ops. The fixed 256-byte entry-size assertion remains in place, and format-dependent tests are bypassed on big-endian targets. The change also adds a release note and documents big-endian support plus the preferred capability-check pattern for optional architecture-specific features. ## Compatibility and limitations - Little-endian behavior and the on-disk log layout are unchanged. - On big-endian platforms, configuring an injected-error log path does not create or populate a log file; the rest of fault injection remains available. - Windows already leaves this POSIX-backed diagnostic disabled. - This does not introduce a portable log encoding. Supporting persistent injected-error logs on big-endian platforms would require a format change. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15297 Test Plan: - `build_tools/rockstest.sh fault_injection_fs_test '--gtest_filter=InjectedErrorLogTest.*'` (little-endian macOS): all 5 tests passed. - `make check-sources` - `git diff --check upstream/main...2026_10_01_endian_fix` Native big-endian build and test execution were not available locally; that path relies on the compile-time `port::kLittleEndian` capability and should be covered by big-endian CI. Reviewed By: jaykorean Differential Revision: D122880760 Pulled By: xingbowang fbshipit-source-id: 59dae9661a47608aa12f41078ad1b3ef0fa7231e |
||
|
Xingbo Wang
|
b58cbcf13c |
Apply transaction visibility to entity reads (#15269)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15269 Why the change is required: Transaction entity reads must enforce the same commit visibility and read-your-own-write rules as value-only reads. Without the transaction-specific read callback, GetEntity-family APIs can return a value that is not visible to the caller. Previous behavior: TransactionBaseImpl handled GetEntity and MultiGetEntity through non-virtual helpers that called WriteBatchWithIndex without a ReadCallback. WritePrepared transactions could expose a prepared but uncommitted update. WriteUnprepared transactions could miss their own write after it had been flushed as an unprepared batch, although that write remained hidden from other transactions. New behavior: GetEntity, MultiGetEntity, and GetEntityForUpdate now follow the same snapshot visibility and read-your-own-write semantics as Get and MultiGet for WritePrepared and WriteUnprepared transactions. Implementation: Make the internal entity-read helper hooks virtual. Override them in WritePreparedTxn and WriteUnpreparedTxn to construct the existing policy-specific ReadCallback and pass it through WriteBatchWithIndex. The WritePrepared batched path validates the callback refreshed read sequence so reads without an explicit snapshot do not bypass eviction validation. Invalid callback or snapshot state returns TryAgain. Add shared transaction entity-read assertions and parameterized regression coverage for prepared visibility and unprepared read-your-own-write. Safety boundaries and unsupported cases: There is no public API or persisted-format change. Other transaction policies keep the existing base implementation. The change adds only the callback and validation already used by value-only reads. Validation covers synchronous snapshot and non-snapshot entity reads, prepared visibility, and unprepared read-your-own-write in normal and checked-Status builds. Coroutine-specific and named wide-column payload cases were not separately tested, and the full repository suite was not run. Reviewed By: rban1 Differential Revision: D121793013 fbshipit-source-id: e300642ed360b640814056e4d95cced5f5c180d8 |
||
|
Josh Kang
|
4d180247ae |
Add PreparedFileInfo serialization APIs (#15274)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15274 Expose `SerializePreparedFileInfo()` and `DeserializePreparedFileInfo()` so opaque metadata produced by SST writers or DBs can be persisted and transferred across process boundaries. The representation uses RocksDB `OptionTypeInfo` adapters for `InternalKey` and `TableProperties` and includes a CRC32C integrity check. Deserialization now rejects malformed nested string-map entries, including missing key/value separators and invalid hex, with `InvalidArgument` instead of asserting in debug builds or accepting malformed input in release builds. Valid serialized data and the wire format are unchanged. The `PreparedFileInfo` type metadata is also initialized on first use to avoid throwable namespace-scope initialization. Reviewed By: xingbowang Differential Revision: D121897852 fbshipit-source-id: 59a3d221bfd20e6c257bd60477823a12d05a35c5 |
||
|
Jay Huh
|
ded3cb72a5 |
Fix trace replay on the default column family and error reporting in multi-threaded replay (#15296)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15296 **Why:** trace replay (`DB::NewDefaultReplayer()`, db_bench `replay`) cannot replay a trace of a DB with only the default column family, and multi-threaded replay reports success when records fail. Together these make db_bench replay look successful while replaying no reads. **Previous behavior:** - `TraceExecutionHandler` resolves a record's column family ID only against the `handles` passed to `NewDefaultReplayer()`. Records on a column family missing from `handles`, including the default one, fail with `Corruption: Invalid Column Family ID.`. An empty `handles` also trips `assert(!handles.empty())` in debug builds. - db_bench fills its handle list (`db_.cfh`) only when `--num_column_families` > 1, so with the default of 1 it passes an empty list. Every Get, MultiGet and iterator record fails. Write records are not affected, because `DB::Write()` resolves the column family from the batch itself. - In multi-threaded `Replay()` (`num_threads` > 1), `BackgroundWork()` sends only decode errors to the error callback. Execution errors are dropped, so `Replay()` returns OK. Single-threaded replay stops and returns the error, including a NotSupported from execution (for example `DB::Write()` of a DeleteRange with `row_cache` set). It skips only records whose trace type it cannot decode. - Result in db_bench on a single column family DB: `--trace_replay_threads=1` prints `Replay failed. Error: Corruption: Invalid Column Family ID.`. `--trace_replay_threads=4` prints `Replay completed` with `rocksdb.number.keys.read` = 0. **New behavior:** - The replay handler always includes the DB's default column family, so `handles` may be empty or leave it out. A handle the caller passes for a column family is still the one used. - Multi-threaded `Replay()` reports execution errors through the same callback as decode errors. It stops scheduling new records and returns the error of the failed record with the earliest trace timestamp, as the existing code already does for decode errors. Get records that find no key still count as success, since the handler treats NotFound as OK. - As in single-threaded replay, NotSupported is skipped only when decoding returns it (a trace type the replayer does not support). NotSupported from executing a record stops the replay. - The scheduling loop no longer reads the shared status `bg_s` while worker threads write it. It already did so for decode errors; with execution errors reported, TSAN flags it in the new test (see Test Plan). **Implementation:** - `TraceExecutionHandler` constructor: drop the non-empty assert; after the caller's handles, insert `db->DefaultColumnFamily()`. `unordered_map::insert` does not overwrite, so a caller's handle wins. - `TraceExecutionHandler` constructor: skip the default column family if `DefaultColumnFamily()` returns nullptr (some test `DB` mocks do), so such DBs behave as before. - `ReplayerImpl::BackgroundWork()`: call `error_cb` when `Accept()` returns an error. On a decode error, skip `error_cb` for NotSupported, matching single-threaded replay. - `ReplayerImpl::Replay()`: `error_cb` no longer filters out NotSupported (the decode path now does that); it still keeps the error of the earliest record. - `ReplayerImpl::Replay()`: `error_cb` also sets an `std::atomic<bool> bg_failed` while holding `mtx`, and the scheduling loop checks that flag instead of `bg_s.ok()`. `bg_s` is read only after `WaitForJobsAndJoinAllThreads()`. - Document both contracts in `db.h` (`NewDefaultReplayer`), `trace_record.h` (`NewExecutionHandler`) and `replayer.h` (`Replay`). - Release notes in `unreleased_history/bug_fixes/`. **Safety, compatibility, limitations:** - No API or ABI change. - Callers that already pass the default column family handle see no change from the first fix. - Behavior change: multi-threaded replay callers that got OK while records failed now get the error, including NotSupported from execution. That matches single-threaded replay and the new `Replay()` comment. - A record read by the scheduling loop while a failing worker is setting `bg_failed` may still be scheduled. This is allowed by the `Replay()` comment (records already scheduled still run) and does not change the returned status, which is that of the earliest failed record. - Cost: one extra map entry per replayer, and one branch per record in threaded replay. The loop's `bg_s.ok()` read becomes a relaxed atomic load. - Read records on column families that are neither passed nor the default still fail with Corruption, as before. Write records never used `handles` and are unchanged. Reviewed By: xingbowang Differential Revision: D122626982 fbshipit-source-id: 9a0973448710bac25233feedbaf264696ac85035 |
||
|
Josh Kang
|
aeebcbded4 |
Add coroutine external table reads (#15291)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15291 Add experimental coroutine interfaces for implementing asynchronous basic and full `ExternalTableReader`s. Implementations opt in through `CoroExternalTableReaderBase<Mode>` by providing `GetCoroutine()` and may override `MultiGetCoroutine()`; the default implementation runs the individual Get operations concurrently. The external-table adapter uses these APIs when supported and otherwise retains the existing synchronous behavior. As a smaller supporting fix, Single Get coroutine reads using non-mmap, non-direct I/O on a kFSBuffer-capable `FileSystem` now use `FSRandomAccessFile::SubmitReadAsync()` instead of falling back to synchronous `Read()`. Reviewed By: xingbowang Differential Revision: D122187491 fbshipit-source-id: 217e2953756938bbd3f30358a997a7ca3ed5543e |
||
|
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 |
||
|
Jay Huh
|
9d358e63d6 |
Widen compaction service counts to full kNumOfReasons (#14924)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14924 Widen the serialized "counts" array in CompactionServiceResult from kNumOfReasons - 1 back to the full kNumOfReasons, so the count for the newest CompactionReason is included in the remote compaction result. The width was previously reduced by one to match readers that expected the narrower array; restoring it changes what is written. This is safe only because deserialization of "counts" is now size-tolerant (a reader expecting fewer elements drops the extra, one expecting more zero-fills the missing). **It must not land until that size-tolerant read D106321320 has been deployed to every primary, including rollback targets, so should be >= 2 releases ahead; otherwise a primary can still or fallback to running the strict reader would reject the wider result and fail deserialization of this field.** == Test plan == **Legend** CompactionReason is an enum whose last entry is kNumOfReasons (currently 21). CompactionStats::counts is an int[kNumOfReasons] holding one count per reason; it is serialized into the remote compaction result and read back by the primary. - "narrow" = kNumOfReasons - 1 counts (omits the newest reason); what current code writes. - "wide" = kNumOfReasons counts (includes the newest reason). - strict reader: accepts only its exact expected element count; any other count fails deserialization (the plain OptionTypeInfo::Array behavior). - tolerant reader: accepts any count -- more elements than expected are dropped, fewer are zero-filled; never fails on a count mismatch (this change, SizeTolerantDeserializeArray). **Unit test** CompactionServiceTest.CompatCheckCountsWidthTolerance -- verifies that deserializing "counts" tolerates a serialized element count equal to, one greater than, or one less than the reader's expected width (extras dropped, missing zero-filled), and that the binary serializes exactly the expected number of elements. **Manual cross-version verification (ldb remote_compaction)** Test 1: expect-narrow tolerant-read primary reads wide counts a worker write (extra reason dropped) ``` $ tolerant_wide remote_compaction_worker --db=$DB --job_dir=$JOB & $ tolerant_narrow remote_compaction_primary --db=$DB --job_dir=$JOB remote_compaction_worker: OK (4915 bytes result) remote_compaction_primary: OK $ grep 'remote compaction result' $DB/LOG [default] [JOB 3] Received remote compaction result, output path: /tmp/cs_run/output, files: 000022.sst ``` => PASS: the primary parsed and installed the remote result. Test 2: expect-wide tolerant-read primary reads narrow counts a worker writes (missing reason zero-filled) ``` $ strict_narrow remote_compaction_worker --db=$DB --job_dir=$JOB & $ tolerant_wide remote_compaction_primary --db=$DB --job_dir=$JOB remote_compaction_worker: OK (4911 bytes result) remote_compaction_primary: OK $ grep 'remote compaction result' $DB/LOG [default] [JOB 3] Received remote compaction result, output path: /tmp/cs_run/output, files: 000022.sst ``` => PASS: the primary parsed and installed the remote result. Test 3: expect-wide tolerant-read primary reads wide counts a worker writes ``` $ tolerant_wide remote_compaction_worker --db=$DB --job_dir=$JOB & $ tolerant_wide remote_compaction_primary --db=$DB --job_dir=$JOB remote_compaction_worker: OK (4917 bytes result) remote_compaction_primary: OK $ grep 'remote compaction result' $DB/LOG [default] [JOB 3] Received remote compaction result, output path: /tmp/cs_run/output, files: 000022.sst ``` => PASS: the primary parsed and installed the remote result. Reviewed By: hx235 Differential Revision: D111068809 fbshipit-source-id: d78dd4b7501cb74e60bcb27dccd869b73052ac6d |
||
|
Xingbo Wang
|
31675ca6d0 |
Recover from poisoned WAL handles on remote file systems (#15193)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15193 Keep `DB::Write`'s existing immediate `IOError` contract while allowing RocksDB to recover safely when a remote file system reports that a WAL handle is permanently unusable. The public API documentation makes the persisted-state boundary, reopen behavior, and safe post-resumption retry point explicit. Normal operation remains unchanged; the additional protocol is entered only after a supported file-scoped WAL error. ### Motivation A WAL write crosses an acknowledgement boundary: storage can accept the complete record and then fail before the caller receives confirmation. The resulting `IOError` therefore cannot tell RocksDB or the application whether the record is absent, partial, or complete. At the same time, the remote file handle can become permanently poisoned, so retrying that same handle can never succeed. The previous recovery path retried the poisoned handle. Simply replacing it is not sufficient: RocksDB must also prevent reuse of an ambiguous sequence range, keep every column family on one durable boundary, and prevent WAL tailers from publishing a physically complete record that successful live resumption deliberately abandoned. The new protocol quarantines the affected WALs, reserves the ambiguous sequence range, creates a replacement WAL, atomically persists acknowledged state, retires the quarantined handles, and only then resumes writes. ### Behavior changes #### 1. Recovering from a poisoned remote WAL handle - **Why required:** A remote file system can permanently poison a WAL handle after an I/O failure. Retrying that handle cannot succeed and leaves the database write-stopped. - **Previous behavior:** RocksDB retried the same WAL handle during recovery. - **New behavior:** The remote file system marks the failure as file-scoped. RocksDB creates a replacement WAL, atomically persists all state that could depend on the failed WAL, and then removes the quarantined WAL handles from active use. Physical file deletion remains part of normal cleanup. #### 2. Preserving an indeterminate WAL write outcome - **Why required:** A WAL record can reach storage before the append returns `IOError`; neither RocksDB nor the application can know the outcome from that error alone. - **Previous behavior:** Recovery could reuse a file-scoped failed write group's sequence numbers, allowing a later acknowledged write to overlap a record that might already be in the WAL. The public write API did not explain the acknowledgement ambiguity or distinguish live resumption from close/reopen recovery. - **New behavior:** `DB::Write` still returns the original `IOError` immediately and does not retry the logical batch. Normal DB reads can continue, but the failed write is not visible in that DB session. The persisted DB remains indeterminate with respect to the failed write until supported file-scoped live resumption succeeds without closing or reopening the DB. On reopen, or for any other WAL error without the durable-abandonment guarantee, applications must resolve consistency within RocksDB's documented guarantees, including `WALRecoveryMode`, through idempotency, deduplication, reconciliation, or an equivalent mechanism. After successful supported file-scoped live resumption, RocksDB has durably abandoned the failed batch and reserved its sequence range; retrying the same logical write is then recommended to preserve one linear logical history. #### 3. Keeping TransactionLogIterator consistent with live resumption - **Why required:** A physically complete error-returned record can remain readable from its WAL even though supported file-scoped recovery excludes that batch from the database. Returning it from `GetUpdatesSince()` would make replication or change-data-capture consumers diverge from the recovered DB. - **Previous behavior:** An iterator opened before recovery could return the abandoned batch. A new iterator could also return it while the physical WAL remained visible. - **New behavior:** For file-scoped failures, RocksDB records the exact sequence range and WAL number of each indeterminate write. For the lifetime of that open DB, existing and new WAL iterators stop with `Status::NotFound` before the range, whether the physical record is complete or absent. A caller can begin a new iterator after the reserved range. Iterator creation snapshots the recovery-gap generation together with its WAL-file view so a failure on a later, unseen WAL returns `TryAgain` for the old iterator instead of skipping its valid prefix. #### 4. Keeping all column families on one recovery boundary - **Why required:** A write batch can update multiple column families. Retiring its WAL after independently persisting only some column families can expose a partially applied batch after reopen. - **Previous behavior:** With `atomic_flush=false`, recovery could persist column families independently before abandoning the WAL. - **New behavior:** File-scoped WAL recovery force-flushes every live column family atomically and verifies that each one advanced beyond the failed WAL before writers are retired or writes resume. #### 5. Merging concurrent WAL failures - **Why required:** Another WAL write can fail while an earlier recovery attempt temporarily releases the DB mutex. - **Previous behavior:** A later failed WAL cutoff or ambiguous sequence could be overwritten or cleared by the earlier recovery attempt. - **New behavior:** Each error episode monotonically merges the largest failed WAL cutoff and the sequence range of the latest ambiguous write group. Recovery clears the quarantine cutoff only if no concurrent failure extended it; otherwise the newer failure remains write-stopping for the next attempt. Iterator fencing tracks the actual ambiguous write range separately, so an older failed `SyncWAL()` and a newer ambiguous write are not incorrectly paired. #### 6. Retaining file scope through `NoSpace` recovery - **Why required:** `NoSpace` can still identify one permanently unusable WAL handle. - **Previous behavior:** The `SstFileManager` recovery path entered generic recovery without carrying the failed WAL context. - **New behavior:** File-scoped `NoSpace` carries the same cutoff and sequence evidence through the `SstFileManager` path as other file-scoped WAL errors. #### 7. Scoping recovery context to one error episode - **Why required:** A completed recovery must not affect a later unrelated background error. - **Previous behavior:** WAL recovery context could remain latched after `ClearBGError` succeeded. - **New behavior:** Successful background-error clearing resets the context. A failed recovery retains it so a later retry has the same safety information. The separate iterator gap record remains for the lifetime of the open DB because later WAL readers must continue to reject the abandoned physical record. #### 8. Preserving successful `SyncWAL` metadata - **Why required:** A healthy WAL can finish syncing while a separate recovery error already exists. - **Previous behavior:** `SyncWAL` could return the existing background error before applying the successful WAL's `VersionEdit`, dropping its MANIFEST update. - **New behavior:** `SyncWAL` skips quarantined writers, applies metadata for successfully synced healthy WALs, and only then returns any existing recovery error. #### 9. Classifying only the intended remote-file-system failure - **Why required:** File-scoped recovery is valid only for an actual WAL `IOError` from a handle whose ownership is still valid. - **Previous behavior:** Poison state could be queried before ownership fencing was resolved, and file scope could be stamped onto a non-I/O status. - **New behavior:** Ownership fencing is resolved first, poison state is queried afterward, and only an actual `IOError` can be classified as file-scoped. `IOFenced` remains fatal. #### 10. Preserving `paranoid_checks=false` retry behavior - **Why required:** Non-paranoid mode historically cleared a writer's sticky error so a later operation could retry after storage recovered. - **Previous behavior:** Avoiding reclassification of a cached synthetic writer error also bypassed `WALIOStatusCheck`, so later `SyncWAL` calls kept returning the stale error. - **New behavior:** A cached error still fails the current `SyncWAL` call, but the non-paranoid path resets the writer for a later retry without setting a background error or entering file-scoped recovery. #### 11. Preserving async WAL precreation across recovery - **Why required:** A precreated WAL is an unpublished replacement and is not the active WAL. Recovery can also consume the prepared slot while background work is stopped. - **Previous behavior:** A file-scoped failure while creating or starting the unpublished replacement was attributed to the active WAL, unnecessarily quarantining it. After successful recovery consumed a prepared WAL, the next slot was not replenished, so the following rotation created its WAL synchronously. - **New behavior:** Only an active WAL `WriteBuffer()` failure attaches the active-WAL cutoff. A failed unpublished candidate is discarded while the current WAL remains active, and successful recovery schedules the next async precreation after `ClearBGError()` re-enables background work. ### Implementation - Carry `DBRecoverContext` through generic, `NoSpace`, and `SstFileManager` recovery, merging WAL cutoffs and failed write-group sequence ranges independently of error severity. Generic errors retain the range only so a later retry that establishes file scope can activate the stronger protocol; they do not persist a reservation or install an iterator gap on their own. - Document the WAL acknowledgement ambiguity, persisted-state boundary, reopen behavior, application consistency requirements, and post-resumption retry guidance on the public `DB::Write` API. - Publish a lock-free quarantine cutoff used by write and sync paths; replace even an empty failed current WAL; persist file-scoped ambiguous sequence reservations; force and verify an atomic all-column-family flush; and detach quarantined writers only after that durable boundary. - Record each file-scoped ambiguous write group's exact WAL number and sequence range in a DB-lifetime tracker shared by `WalManager` and all `WalIteratorImpl` instances. Stop an iterator before a batch that contains or crosses the range, and use a generation snapshot to preserve the existing `TryAgain` behavior when an iterator's fixed WAL-file view predates the failure. - Keep successful healthy-WAL tracking edits in `SyncWAL`, reset completed recovery context, and preserve legacy non-paranoid writer reset behavior. - Distinguish failures on an unpublished replacement WAL from failures flushing the active WAL. Log the failed replacement identity, keep the active WAL out of quarantine, and replenish the async-precreation slot after successful recovery. - Classify poisoned remote-file-system handles after ownership fencing and only for `IOError`. - Add lifecycle logging and a state-machine comment covering quarantine, sequence reservation, iterator fencing, replacement, atomic persistence, retirement, and concurrent-error handoff. - Add an opt-in `db_stress` mode that injects file-scoped faults only into WAL writes, disables WAL tracking in that mode, validates unsupported direct invocations, and sanitizes crash-test parameters to the supported recovery envelope. A small fraction of ordinary non-specialized crash-test runs select this mode automatically; existing stress behavior is unchanged when the flag is off. - Make the concurrent-error regression test hold the initial background flush until `SyncWAL` records the intended older-WAL error, preventing scheduler-dependent retirement of the test WAL before fault injection. ### Safety boundaries and unsupported cases - Automatic file-scoped WAL recovery fails closed with `NotSupported` for manual WAL flush, 2PC/transactions, two write queues, pipelined or unordered writes, WAL recycling, WAL TTL or size retention, either WAL-tracking mode, or blob direct writes. - Unsupported recovery leaves the DB write-stopped. `DB::Resume()` returns `NotSupported`; after correcting the I/O condition, operational recovery requires close and reopen. - The failed logical batch is never retried. Only supported file-scoped recovery durably reserves its sequence range and fences `GetUpdatesSince()`. Generic WAL errors retain the existing indeterminate outcome and can replay after close/reopen; applications must use idempotency or reconciliation unless successful file-scoped live resumption has provided the stronger guarantee. - Failed recovery attempts retain their context. Only a successful `ClearBGError` resets it. - Physical WAL deletion is not part of the recovery correctness boundary; normal obsolete-file cleanup removes files later. - The TransactionLogIterator ambiguity fence is kept for the lifetime of the DB instance that observed the file-scoped error and is not persisted as new MANIFEST metadata. After a later close/reopen, normal WAL recovery and obsolete-WAL retention/deletion semantics apply. - The in-memory iterator-gap tracker can grow with the number of recovered file-scoped failures during one DB lifetime. Safe pruning must account for retained WALs and existing iterator snapshots; it is left for follow-up. - Async WAL precreation remains disabled with WAL recycling. A prepared WAL is unpublished until its start records succeed; failure before installation does not make the active WAL file-scoped. - The new stress mode is test-only and defaults off, so normal `db_stress` and database operation without an error are unchanged. - Automatic stress-profile selection is limited to fully parsed, supported configurations. Explicit targeted runs reject options files and other unsupported combinations. ### Tests and known limitations - The affected recovery, WAL-manager, WAL-iterator, fault-injection, async-WAL-precreation, and secondary-close binaries build successfully. - Checked-status suites pass for all 70 `error_handler_fs_test` cases, all 7 `wal_manager_test` cases, all 11 `db_log_iter_test` cases, all 8 `fault_injection_fs_test` cases, and all 5 `DBWALTest.AsyncWalPrecreate*` cases. `DBSecondaryTest.SecondaryCloseFiles` passes 20 consecutive repetitions. - Focused regressions cover file-scoped gap fencing after an iterator snapshot, generic errors not installing a file-scoped sequence gap, and later recovery discovering that a generic error is file-scoped. - Earlier validation on this diff also passed crash-test unit coverage, remote-file-system adapter tests, repeated concurrency tests, two bounded multi-column-family stress runs, and bounded black-box crash/reopen dry runs. - The exact internal RocksDB format check and `arc f --take-formatters --enforce-lint-clean` report no formatting or lint issues. - The full crash/stress matrix and TSAN were not run. Reviewed By: pdillinger Differential Revision: D119324616 fbshipit-source-id: 05779d2bff7018f0ae69c4cbbe5b6455a45675d6 |
||
|
Josh Kang
|
ff29163e47 |
Fix GCC range-loop warning in table test (#15292)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15292 D120637416 introduced a range-for whose `const std::string&` loop variable binds to temporary strings created from a string-literal initializer list. GCC diagnoses this with `-Wrange-loop-construct`, breaking warnings-as-errors builds. Iterate by value to make the intended string construction explicit without changing test behavior. ___ Differential Revision: D122474818 fbshipit-source-id: c7f30a374dc7569ceb1a4ecc6f7f8c3c3637be00 |
||
|
Josh Kang
|
176d7c7ea1 |
Add full external table support and crash testing (#15271)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15271 ## Summary Extend the experimental external-table API with a full mode that preserves RocksDB internal keys, sequence numbers, and entry types. Unlike the existing ingestion-oriented mode, full external tables can represent normal flush and compaction output and participate in live databases with puts, merges, deletions, snapshots, `Get`, `MultiGet`, iteration, multiple LSM levels, and external-file ingestion. The existing `ExternalTable*` interfaces remain aliases for the original sequence-zero, Put-only mode, preserving compatibility for current implementations. ## Implementation details - Introduce `ExternalTableMode` and mode-specific reader and factory interfaces. Full-mode builders and iterators receive complete internal keys, while the original mode continues to receive user keys. - Adapt full-mode point lookups to RocksDB’s `GetContext`, allowing readers to return multiple versions of a user key in internal-key order so RocksDB can resolve snapshots, merges, and deletions correctly. - Add a batched `MultiGet` context that preserves per-request lookup state and maps external-reader results back to RocksDB’s original request range. - Translate iterator seek boundaries and returned keys while ensuring file-wide global sequence numbers are applied exactly once. - Track sequence-number bounds, deletion and merge counts, user-collected properties, file size, and unique IDs when building tables. Unsupported entries fail and abandon the builder. - Forward checksum verification to the external reader while retaining compatibility with implementations that do not override it. - Add a standalone test implementation shared by basic and full modes. It uses a simple length-prefixed file format with a RocksDB properties block and checksum, avoiding dependencies on native table implementations. ## Crash testing Add `use_full_external_table` support to `db_stress` and a dedicated blackbox `db_crashtest.py` profile. The profile exercises randomized reads, writes, merges, ingestion, flushes, compactions, snapshots, checksum verification, and crash recovery. The profile disables unsupported combinations, including range deletions, user timestamps, sequence-number time tracking, backward scans, multiple column families, backup reconstruction, remote compaction, dynamic table-factory replacement, randomized aggregate/all-table property probes, and global-sequence-number rewriting during ingestion. Because the simple reader materializes each open table in memory, the profile limits open files and generated file sizes to bound total database memory usage. Reviewed By: xingbowang Differential Revision: D120637416 fbshipit-source-id: aa2cbd06999e4325a234210d9c45987addd37550 |
||
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 |
||
|
Xingbo Wang
|
eed2f96676 |
Wait for in-flight secondary read-view reclamation (#15289)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15289 Why the change is required: Secondary catch-up must release reader-free retired read views before scanning for obsolete files. T290043099 and duplicate T290043100 exposed a race where that ordering was lost, leaving two obsolete table files open and failing DBSecondaryTest.SecondaryCloseFiles. The assertion failure also exposed unsafe test cleanup because the secondary DB could outlive the test-local Env. Previous behavior: Retiring a read view always woke the background reclaimer. The worker could remove a reader-free view from the retirement queue and pause before destroying its SuperVersions. Synchronous cleanup then saw no queued work, ignored the in-flight reclamation, and continued to obsolete-file discovery while the old files were still referenced. The test also used a static lambda that captured the first fixture across repeated runs and closed the secondary only on the success path. New behavior: Catch-up synchronously reclaims reader-free views, or waits when a competing background worker has already claimed them. Views still held by active readers remain non-blocking and are reclaimed asynchronously after the last reader releases them. The test now closes the secondary before its local Env on every exit path and creates its verification lambda per test invocation. Implementation: Publishing a replacement view no longer wakes the background reclaimer; the immediately following synchronous cleanup owns that work. Reader release still wakes the worker. Cleanup waits on the existing in-flight counter when a worker won an earlier race. The test uses a Defer guard for lifetime ordering and removes the repeat-unsafe static lambda. Safety, compatibility, and limitations: This changes no public API, ABI, file format, recovery behavior, or read consistency. Cleanup does not wait for views retained by active readers, so the non-blocking multi-column-family iterator behavior is preserved. It waits only for reclamation already in progress. Validation covered the internal Linux ASAN/UBSAN target; the external GitHub Actions matrix was not run. Repository make-based format/source checks require a Git worktree and were unavailable in this Sapling checkout. Reviewed By: jaykorean Differential Revision: D121816425 fbshipit-source-id: a86cfff43a0cedd10b90e526a56e862f18d2a58d |
||
|
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 |
||
|
Omkar Gawde
|
7510773057 |
Fix ClipColumnFamily key lifetime and atomic-flush wait alignment (#15273)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15273 **What**: `ClipColumnFamily` keeps boundary keys valid across unlocked writes, and atomic-flush waits pair every column family with the correct memtable bound. **How**: `DBImpl::ClipColumnFamily` copies boundary keys from the current `Version` while holding the DB mutex, with a sync point that lets the regression test replace that version before deletion. `DBImpl::AtomicFlushMemTables` and `DBImpl::RetryFlushesForErrorRecovery` construct wait bounds in `cfds` order and supply no-bound fallbacks when a column family is absent from the flush-request map. Both wait sites gain a debug invariant asserting each id matches the map entry for the cfd at the same index. **Why**: Background version replacement could leave `ClipColumnFamily` reading freed metadata, while unordered or missing atomic-flush map entries could cause early completion or out-of-bounds access for idle or duplicate column families. Reviewed By: xingbowang Differential Revision: D121840843 fbshipit-source-id: 8e49b5c953a1aa29a0244fc0aae2ca3ab84be74f |
||
|
xingbowang
|
5eed79a0e1 |
Scale container shared memory by runner core count (#15284)
Summary: GitHub Actions container jobs previously used a fixed 16 GB `/dev/shm` allocation regardless of runner size, while the clang-tidy container did not set one explicitly. This over-allocated shared memory on smaller runners and gave larger, more highly parallel runners no additional shared-memory capacity. Scale container shared memory to 2 GB per runner core across the PR, nightly, nightly-candidate, weekly, and clang-tidy workflows: - 4-core jobs use 8 GB. - 16-core jobs use 32 GB. - 32-core jobs use 64 GB. - The nightly-candidate ARM job uses 8 GB, matching its four-way build. The clang-tidy job now explicitly receives 8 GB. Existing unrelated container options, including the mini crash test's `nofile` limit, remain unchanged. ## Safety and compatibility This only changes container resource configuration for GitHub Actions jobs. It does not change RocksDB code, APIs, persistent formats, or non-containerized jobs. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15284 Test Plan: - `make check-workflow-yaml` - `git diff --check upstream/main...2026_09_28_shm` Both checks pass. The CI workloads themselves were not executed locally, so the new shared-memory allocations will be exercised by PR and scheduled workflow runs. Reviewed By: omkarhgawde Differential Revision: D122218094 Pulled By: xingbowang fbshipit-source-id: aa577c37683022ba01c4a073a011dbe0032abbf6 |
||
|
Omkar Gawde
|
3d381e1bd4 |
Fix heap-use-after-free in DBSecondaryTest.SecondaryCloseFiles Env teardown (#15283)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15283 **What**: `DBSecondaryTest.SecondaryCloseFiles` no longer calls into a destroyed `Env` when the test body returns early, fixing a ThreadSanitizer heap-use-after-free at secondary DB close. **How**: The test opens `db_secondary_` (a fixture member) on a stack-local `traced_env`. A `Defer` now closes the secondary before `traced_env` is destroyed on every exit path, so the later `~DBSecondaryTestBase` close never reaches `DBImpl::CloseHelper`'s `env_->UnSchedule` on a freed Env. This mirrors the sibling `DBSecondaryCatchUpFaultTest` fixture, which already closes the secondary in its destructor before its Env member for the same reason. **Why**: On any early `ASSERT_*` return before the explicit close, `traced_env` (a body local) was freed first while `db_secondary_` (a fixture member) was closed later in the base destructor, which made a virtual call on the freed Env. ThreadSanitizer flagged this as a land-blocking heap-use-after-free in the `build-linux-clang21-mini-tsan` CI job. Failed CI run: https://fburl.com/jo6gtllf ___ Differential Revision: D122195601 fbshipit-source-id: 3d8b427ba50759c40f15d2b512379594b6057234 |
||
Steph Pontikes
|
f8d3141e5c |
Thread column family names to table readers and add ExternalTableFactory recovery (#15265)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15265 This change adds column family name reflection to an external table reader and allows the user to extract an external table factory from a top level table factory, if it exists Motivation Adding column family metadata to the reader allows the reader to handle CF specific metadata, also add external table reflection Previous/New Behavior No change in behavior. Added a defaulted parameter threaded through many constructors to preserve existing API contract while introducing new capability. Added a function which allows unwrapping of an external table factory Nothing much to test Reviewed By: xingbowang Differential Revision: D121335729 fbshipit-source-id: 74f9b1be6a8f301bdbe4540de0656b83e26f88ab |
||
|
Jay Huh
|
4052fccde9 |
Enable SecondaryDB in CF-consistency crash tests (#15270)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15270 D121318547 added SecondaryDB batch-iterator coverage to CfConsistencyStressTest::ContinuouslyVerifyDb(), but the automated CF-consistency crash-test profile never selected test_secondary. Automated runs therefore exercised only the primary DB path while the targeted manual validation used test_secondary=true. Randomize test_secondary between 0 and 1 in cf_consistency_params. Each generated run can now exercise either the existing primary-only path or the SecondaryDB path, preserving both configurations in the continuous matrix. The regression test deterministically verifies both random outcomes. Existing sanitization may still disable SecondaryDB when another selected option is incompatible, such as remote compaction or primary UDI mode. Reviewed By: xingbowang Differential Revision: D121813002 fbshipit-source-id: 4080e2131c1f063d590d24cc0bcee9b9096de63a |
||
|
Xingbo Wang
|
ac30c4d738 |
Add stress scan witness diagnostics (#15199)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15199 ## Why PrefixScan and MultiScan verification failures are difficult to reconstruct from the terminal assertion alone. The iterator options, bounds, seeks, preparation state, and immediately preceding movements are often the evidence needed to distinguish a database defect from an invalid test assumption. ## Why the existing recovery trace is not enough The expected-state trace exists to reconstruct the expected database state after recovery. It preserves ordered write batches and deliberately filters out reads and iterator seeks, then recovery consumes a sequence-number-selected prefix. Consequently it contains none of the scan-specific evidence needed here: read options, bounds, seek targets, iterator validity, lazy-value preparation, movements, or the exact mismatch state. Scan witnesses capture that bounded human-readable context only when a verification failure needs it; they are best effort, are never replayed, and do not alter the recovery trace or its failure semantics. ## Previous behavior The scan paths emitted generic failure output but did not retain a focused bounded witness of iterator setup and state transitions. Adding instrumentation naively would also format diagnostic strings on the normal path even when disabled and could create one file per failing operation. ## New behavior An opt-in bounded ScanWitnessLog records scan configuration and recent iterator events for PrefixScan and MultiScan variants. On a qualifying failure it writes the latest complete witness for that DB, PID, thread, and scan type. The operation ordinal remains in the file contents. Repeated failures atomically replace the fixed witness rather than creating unbounded files. ## Implementation The witness captures sanitized read options, input ranges, bounds, seeks, movement, PrepareValue results, iterator status, keys, value size and hash, and wide-column state. MultiScan tracks whether the current primary iterator value was actually prepared so failure snapshots report accurate state. Disabled witnesses avoid scan-label ownership, range formatting, expected-count formatting in release builds, and numeric iterator-label conversion. Witness writes share the per-thread diagnostic I/O-disable latch with breadcrumbs, write to a fixed temporary path, close, and rename over the final file. Entry counts are capped at 65,536. A formatting-only follow-up normalized all touched C++ lines with the repository clang-format rules. ## Safety and unsupported cases The feature is stress-tool-only and raw db_stress keeps it disabled by default. It observes public iterator state and does not change database contents, expected-state tracing, recovery replay, iterator decisions, or fault-injection outcomes. Diagnostic I/O remains best effort and cannot fail the workload. Files are bounded per retained PID, thread, and scan type. The witness covers the instrumented PrefixScan and MultiScan implementations only and is not a general RocksDB trace. Files are not fsynced and therefore are not a power-loss durability mechanism. ## Tests and limitations Validated with 47 tools/db_crashtest_test.py tests, exact-candidate debug and release db_stress builds, arc lint over the complete stack, oversized-entry rejection, and clean independent correctness reviews of the final integrated stack. make check-sources is unavailable because it invokes Git in this Sapling checkout; equivalent added-line hygiene checks pass. Only the newest complete witness per DB, PID, thread, and scan type is retained, so later failures intentionally replace earlier witnesses. ## Rebase update Rebased onto fbsource master dc59f986b964. The logical diff is unchanged; the rebased stack passes debug and release db_stress builds, all 47 db_crashtest Python tests, and arc lint. Reviewed By: jaykorean Differential Revision: D119160911 fbshipit-source-id: 0e9b040c3bbb482e0a3b1b11070a7c5cc519f060 |
||
|
Josh Kang
|
dbc2db5228 |
Make external table APIs more consistent with native tables (#15241)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15241 Native `TableFactory` implementations receive the RocksDB `WritableFileWriter` in `NewTableBuilder()` and `RandomAccessFileReader` in `NewTableReader()`. These wrappers provide buffering, checksums, rate limiting, statistics, listeners, tracing, and direct-I/O handling. External table implementations previously received a limited writable-file adapter and reopened readers from a path, bypassing parts of this wrapper logic. This change makes `WritableFileWriter` and `RandomAccessFileReader` implement `FSWritableFile` and `FSRandomAccessFile`, respectively, so the same RocksDB wrappers can be passed to external tables. `ExternalTableFactory::NewTableReader()` now receives the already-opened reader and file size. The filesystem interfaces expose standard append, flush, sync, fsync, file metadata, synchronous reads, asynchronous reads, and batched reads. `RandomAccessFileReader::SubmitReadAsync()` schedules its wrapped coroutine read on the configured filesystem read executor, and the DB table-cache and `SstFileReader` construction paths pass that filesystem through. `SstFileReader` also supplies its configured statistics, listeners, rate limiter, and file temperature, removing the need for a separate external-table metrics filesystem wrapper. For direct I/O, `MultiRead()` checks whether requests are already sorted. When alignment and merging are needed, it constructs a sorted index only for unsorted input, preserving caller request order and result mapping. Fully aligned requests using aligned caller buffers bypass this path and may be supplied in any order. Reviewed By: xingbowang Differential Revision: D121035539 fbshipit-source-id: 27cb897726c31cb1a4dc811c65c8d79e690d82b1 |
||
|
Xingbo Wang
|
07cbacadb4 |
Avoid materializing huge SSTs in size-boundary test (#15264)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15264 Why the change is required: LargeSizeSstFileWriter creates approximately 12 GiB of SST output in the shared /dev/shm used by build-linux. With 32 concurrent test jobs in a 16 GiB tmpfs, this can exhaust the filesystem and make unrelated fault-injection and repair shards fail with ENOSPC. Previous behavior: The test wrote an approximately 8 GiB large-key SST and an approximately 4 GiB large-value SST even though it never read or ingested either file. It validated the size limits and successful Finish calls, but the physical output competed with every other test process. New behavior: The test still runs the real SstFileWriter and table builder for over-limit rejection and maximum-size acceptance, including Finish, sync, and close. The generated byte stream is counted and discarded instead of materialized in the shared tmpfs. Implementation: Add a test-local FileSystem wrapper whose writable file counts appended bytes and records sync and close calls. Inject it only into LargeSizeSstFileWriter and verify that the writer-reported logical size matches the observed append stream, the lifecycle completes, and no physical SST is created. Safety, compatibility, and limitations: Production code, shared filesystem implementations, serialized formats, and other tests are unchanged. The fake does not validate SST contents, but the previous test did not read the generated files either. Table construction still consumes substantial memory, so the existing HasBigMem guard remains; this change addresses tmpfs exhaustion only. Reviewed By: rban1 Differential Revision: D121483663 fbshipit-source-id: 055372bc07e3a1f7289e34ebc35694bc3c4bfad9 |
||
|
Ranjan Banerjee
|
662cdbc9ec |
Update RocksDB main for 11.11 release (#15263)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15263 Why: keep internal RocksDB main ready for 11.12.0 development while the 11.11 release is cut. Previous behavior: Conveyor opened a GitHub pull request for this update, which could be missed and was not stacked with the internal release diff. New behavior: Conveyor owns an internal Phabricator diff that the release-import diff stacks on top of. The release manager exports this diff manually with ExportIt. Implementation: - Bump version.h from 11.11.0 to 11.12.0 - Add `11.11.fb` to `check_format_compatible.sh` - Sync HISTORY.md `## 11.11.` section from `11.11.fb` - Delete 19 consumed `unreleased_history/` note file(s) from main Safety and unsupported cases: - Only version.h, check_format_compatible.sh, HISTORY.md, and consumed release-note files are allowed. - Open legacy GitHub update-main PRs block this flow; this automation never modifies, exports, or lands them. - Concurrent direct CLI invocations are unsupported; Conveyor serializes the Release Cut node to one active run. - The tool refuses unrelated commits, mismatched release metadata, and non-canonical GitHub repositories. Known limitations: - ExportIt remains manual. Bot create permission for internal_repo_rocksdb/repo is a rollout prerequisite. Reviewed By: xingbowang Differential Revision: D121434642 fbshipit-source-id: 3c6c90f425e043ff9c13318ecc671aedbdef0347 |
||
|
Jay Huh
|
ffd4ac31af |
Make SecondaryDB multi-CF iterator reads non-blocking (#15259)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15259 Publish immutable SecondaryDB read views after open and successful catch-up so `NewIterators()` can acquire one cross-CF sequence and SuperVersion set without waiting for `DBImpl::mutex_`. Keep failed catch-ups on the previous view, reject refresh for batch iterators, and exercise concurrent catch-up plus batch scans in the CF-consistency stress profile. Reviewed By: xingbowang Differential Revision: D121318547 fbshipit-source-id: 3198f4766c90ab0bb39400963df3c329b4ecf6bf |
||
|
Omkar Gawde
|
0675830967 |
Verify prefetched block checksums with retry-safe cleanup (#15254)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15254 **What**: `IODispatcher` verifies prefetched data blocks before serving or caching them and safely falls back when block construction fails. **How**: `CreateAndPinBlockFromBuffer` validates every returned span and, when `ReadOptions::verify_checksums` is enabled, verifies its checksum and records the standard counters. `PollAndProcessAsyncIO` handles coalesced blocks independently and retires shared request state before failed entries fall back through `SyncRead`; `ReleaseFailedSyncPrefetchMemory` returns limiter capacity for unmaterialized synchronous blocks. **Why**: Unverified prefetches could serve or cache bit rot, while newly detected failures could otherwise leave async state referencing released provider buffers or permanently consume bounded prefetch capacity. Reviewed By: xingbowang Differential Revision: D121277687 fbshipit-source-id: 65cc1d5b9dcc53bdc64f8ee5d672bfe2cda7b40b |
||
|
Xingbo Wang
|
14a82121d4 |
Add shared diff and pull request summary guidance (#15261)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15261 Why the change is required: Internal Phabricator diffs and external GitHub pull requests need the same substantive explanation of a RocksDB change. Existing local instructions focus on code generation and review but do not define what a summary must tell reviewers, so summaries can omit prior behavior, safety boundaries, compatibility, or validation evidence and can drift between the two review surfaces. Previous behavior: There was no CLAUDE.md at the internal_repo_rocksdb wrapper root, the RocksDB CLAUDE.md did not point authors to summary guidance, and there was no shared document covering both internal diff and external pull request summaries. New behavior: The wrapper-level CLAUDE.md requires authors to consult a shared summary guideline before writing or updating an internal diff summary. The repository CLAUDE.md applies the same guideline to external pull request summaries. The guideline requires motivation, previous and new behavior, implementation, safety and compatibility boundaries, and test evidence, with RocksDB-specific advice for correctness, persisted formats, concurrency, and performance changes. Examples are separated by access boundary: the internal wrapper contains Phabricator diff examples, while the repository guideline contains publicly accessible GitHub pull request examples. Implementation: Add a concise wrapper-level CLAUDE.md, add claude_md/pull_request_summary_guideline.md as the shared source of truth, and link it from repo/CLAUDE.md. Keep annotated internal diff references in the wrapper-level file and corresponding public pull request references in the repository guideline, including a recent change from Peter Dillinger. Safety boundaries and unsupported cases: This is documentation-only and changes no runtime, API, build, test, or CI behavior. The repository guideline contains no internal links, so external readers do not need Meta access. The guidance is manually followed by authors and agents; it does not mechanically validate summaries, replace reviewer judgment, or require a rigid section layout for small changes. ___ Differential Revision: D121388476 fbshipit-source-id: bb09ab7ba8ef6dc2b2830a42735322890536622f |
||
Andrei Preda
|
507206e46b |
Fix stale readahead trim prefix on SeekToFirst (#15216)
Summary: `BlockBasedTableIterator::SeekImpl()` sets `seek_key_prefix_for_readahead_trimming_` only when given a seek key, and `ResetBlockCacheLookupVar()` does not clear it. After a `Seek()`, a `SeekToFirst()` on the same iterator trims readahead against the previous seek key's prefix. `DBIter::SeekToFirst()` under `prefix_same_as_start` takes its prefix from the first key found, so the old prefix does not describe the keys being scanned. With the stale prefix, `IsNextBlockOutOfReadaheadBound()` is true at every block instead of only at the prefix boundary, and each prefetch is clamped to the current block. The same predicate sets `is_index_out_of_bound_`, which `FindBlockForward()` converts to `is_out_of_bound_` and ends the scan. Under `ReadOptions::async_io` this drops keys. The bug needs `auto_readahead_size` and a preceding `Seek()` that reads no data block. A `Seek()` that lands on a key leaves the prefetch buffer anchored above the scan, and the trimming callback is not called. The bug does not occur under `allow_mmap_reads`, where `FilePrefetchBuffer` is constructed disabled. The fix clears the prefix in `SeekToFirst()`, so prefix-based trimming is disabled for that positioning. The `auto_readahead_size` documentation in `include/rocksdb/options.h` already says prefix-based trimming requires `Seek(key)`. On buffered reads with `prefix_same_as_start`, `auto_readahead_size` and `readahead_size=8192`, a cold-cache `SeekToFirst()` scan of 2M keys in a single 222 MB file, preceded on the same iterator by a `Seek()` past the last key, took 391 ms before and 308 ms after. The same scan with no preceding `Seek()` takes 307 ms. Under direct IO on a local disk it went from 33.1 s to 11.8 s. Data blocks read are unchanged; file reads drop by 3x. Also fixes a pre-existing test bug: `SeekParallelizationTestWithPosix` asserts on `number_async_seek` without resetting that cumulative thread-local, unlike its three siblings. The new tests move `prefetch_test` from 11 shards to 13, so different tests share a process, and the assertion fails at the parent commit under that grouping. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15216 Test Plan: New regression test in `PrefetchTrimReadaheadTestParam`, in sync and `async_io` variants over the fixture's three `index_shortening` modes. It runs two scans of the same keys from an equally cold block cache, differing only in a preceding `Seek()`. Reverting the `clear()` fails 3 of 6 sync parameterizations on `READAHEAD_TRIMMED` (6 vs 40) and 3 of 6 `async_io` parameterizations on key count (40 vs 1). `make check` and `ASSERT_STATUS_CHECKED=1 make check` show no new failures. `prefetch_test` 122/122, `db_iterator_test` 249/249, `table_test` 7629/7629, `block_based_table_reader_test` 5347/5347, `db_basic_test` 300/300. New cases ran 1200 times under `COERCE_CONTEXT_SWITCH=1` without failure. Added db_stress coverage for `SeekToFirst()` after `Seek()` under `prefix_same_as_start`. With the fix reverted and io_uring available, the configuration in the second commit fails 8 of 8 seeds with "Iterator diverged from control iterator"; with the fix, the same seeds pass. Reviewed By: rban1 Differential Revision: D120228466 Pulled By: xingbowang fbshipit-source-id: a4935f03a0080cc8dd4c7a8c1438404e9b8f5c3a |
||
zaidoon
|
3c30d43baf |
Promote IndexFactory as UDI SPI (#14962)
Summary: Make `IndexFactory` the public interface for custom indexes. Keep the old `UserDefinedIndex` names, header, and `NewBuilder()` / `NewReader(Slice&)` overloads so existing implementations can still compile. UDI and options tests pass on the full stack, both normally and with `ASSERT_STATUS_CHECKED=1` and `COERCE_CONTEXT_SWITCH=1`. The C API test and generation/compatibility checks pass. Full `make check` runs had test timeouts and hit the 30-minute local limit. These test results are from before the rebase. Part 5 of 9. Previous: https://github.com/facebook/rocksdb/issues/14961 (landed). Next: https://github.com/facebook/rocksdb/issues/14963. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14962 Reviewed By: jaykorean Differential Revision: D119960591 Pulled By: xingbowang fbshipit-source-id: 9703950bd48723c2c524da89cbea63bf00254cc1 |
||
Denis Yaroshevskiy
|
96e7ef0269 |
Use memcpy instead of strncpy in Status::CopyState to speed up Status copies (#15238)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15238 Reviewed By: MatzeB Differential Revision: D116157932 fbshipit-source-id: 56f52f7e5a92154be2be256151e4cc6f63bbbc05 |
||
|
Xingbo Wang
|
5d6f321b70 |
Document behavioral compatibility review requirements (#15218)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15218 Why the change is required: Public and serialized RocksDB inputs can change observable performance or resource behavior without a signature or default change. These changes require deliberate human judgment, clear documentation, and research into downstream production impact. The automated CI review also needs an explicit perspective for identifying gaps in that analysis. Previous behavior: CLAUDE.md described source and ABI compatibility but did not explicitly guide authors or reviewers through behavioral changes to already accepted inputs. The CI review prompt had only a brief API-compatibility checklist item and no shared instructions for evaluating intentionality, release notes, downstream usage, or regression coverage. New behavior: CLAUDE.md states that behavior changes are human judgment calls that must be intentional, documented, and researched. It gives concrete guidance for existing accepted inputs while asking agents and reviewers to call out gaps or ambiguity. The CI review prompt applies the same perspective to every review agent and makes clear that a behavior change is not automatically a defect. Implementation: Revise the Behavioral Compatibility policy in CLAUDE.md, reduce its workflow entry to a concise pointer, add a dedicated Behavioral Compatibility Review Perspective to ci_review_prompt.md, and point the API and compatibility reviewer to that shared section. Safety boundaries and unsupported cases: This is documentation-only and changes no runtime, API, build, or CI execution behavior. The guidance supports human judgment; it does not automatically decide whether a behavior change is acceptable, guarantee exhaustive discovery of downstream consumers, or replace release-note, testing, and production-impact review. Reviewed By: pdillinger Differential Revision: D120344151 fbshipit-source-id: aa5bbb00fbbe167ef21b91b3551868c4d1b4046d |
||
|
Josh Kang
|
256a699be3 |
Support MultiGet and global seqnos in external tables (#15237)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15237 Route `DB::MultiGet` batches through `ExternalTableReader::MultiGet` instead of `TableReader`'s per-key fallback, preserving per-key status and pinned-value handling. Allow basic external tables to honor DB-assigned global sequence numbers across overlapping levels, snapshots, `Get`, `MultiGet`, and iteration. Preserve ingestion-only behavior by rejecting compaction output, and support both `allow_global_seqno` and deprecated `write_global_seqno` files. Reviewed By: xingbowang Differential Revision: D120629061 fbshipit-source-id: 366f48660e97a300afb59627b9a1a41fe3805590 |
||
|
Omkar Gawde
|
7530287bde |
Stop over-counting limited SstFileDumper reads (#15228)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15228 **What**: Correct the limited-read count and add regression coverage for recompression source-read errors. **How**: - `ReadSequential()` checks `read_num_limit` before consuming and counting the next entry. - `RecompressReturnsSourceReadError` injects data-block read failures and verifies `ShowAllCompressionSizes()` returns the error. **Why**: `sst_dump --read_num` reported one extra key, while the recompression input-materialization error path lacked regression coverage. Reviewed By: joshkang97 Differential Revision: D120182510 fbshipit-source-id: d3b07794c9f7b74c29577242ad64c9c1f4a081cf |
||
|
Peter Dillinger
|
b03a058d61 |
Persist more SST table properties for compression analysis (#15231)
Summary: These changes improve the block-based SST table properties available for compression benchmarking and optimization. - Fix: `uncompressed_data_size` was computed while building a file but was never written to the properties block, so it always read back as 0. It is now persisted (written in AddTableProperty and read back in ParsePropertiesBlock). - Add: a new packed `lsm_info_at_creation` property records the LSM level and bottommost status at the file's creation. A new `LsmInfoAtCreation` struct owns the (extensible) uint64 encoding via symmetric Encode()/DecodeFrom(), keeping TableProperties a bare data struct. It distinguishes three states: "unknown" (nothing recorded, e.g. older files), "not applicable" (no LSM position, e.g. SstFileWriter/misc output), and "applicable" (level + bottommost, with an unknown-level sentinel for the DB repairer). The level is stored as its natural value. - Change: the `_compression_manager` detail in the `compression_options` property now records the manager's full serialized configuration (via ToString) instead of only its GetId(). New properties are additive and backward compatible: older files without them read back as "unknown"/0. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15231 Test Plan: Added coverage in db/db_table_properties_test.cc: LsmInfoAtCreation encode/decode round-trips (natural levels, unknown level, not-applicable vs unknown); end-to-end persistence of uncompressed_data_size and lsm_info_at_creation after flush (L0, not bottommost) and a forced bottommost compaction; not-applicable recorded for an SstFileWriter file; and full compression manager configuration present in compression_options. Updated options/options_settable_test.cc for the new field. Also made table_test's BlockBasedTableTest.CompressionRatioThreshold resilient to per-file overhead changes (such as these added table properties): it now verifies the compression decision via the persisted data_size vs uncompressed_data_size rather than modeling total file size against a fixed overhead constant, which had become too tight. Full table_test passes. Reviewed By: rban1 Differential Revision: D120553038 Pulled By: pdillinger fbshipit-source-id: 07ede62dbcbe31c040fd9b037c7636a49558c4d9 |
||
|
Xingbo Wang
|
8108b49e0c |
Add db_stress operation breadcrumbs (#15194)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15194 ## Why Intermittent db_stress and db_crashtest failures often report only the terminal assertion, after the operations that established the failing state have scrolled out of logs. The diagnostics must retain the latest operations even when the stress process is killed before it can run a failure handler. ## Why the existing recovery trace is not enough The existing expected-state trace is a correctness oracle for recovery: after an expected-state checkpoint it records ordered write batches, and recovery replays the prefix implied by the database sequence number. Reads and iterator seeks are deliberately filtered out, trace-write failures are fatal, and the trace is machine-consumed rather than intended as a concise failure narrative. It therefore cannot show which read, scan, maintenance operation, or operation phase immediately preceded an assertion. Breadcrumbs fill that observability gap with a bounded, human-readable, per-thread history. They are best effort, never replayed, and remain independent of the recovery-validation contract. ## Previous behavior db_stress did not retain a bounded per-thread operation history. db_crashtest did not automatically provision a local directory for these artifacts. The first version of this diff kept the ring in process-local memory and periodically wrote a complete temp-file snapshot, which could lose every operation since the last snapshot when the process crashed. ## New behavior An opt-in per-thread memory-mapped breadcrumb ring records recent operation begin, context, and end events directly in a local file. Committed records therefore remain in the kernel page cache when only the db_stress process exits or is killed; no periodic flush is required. db_crashtest enables breadcrumbs by default, assigns a local diagnostics directory, retains the newest eight process groups after failed child runs, and removes managed artifacts after success. ## Implementation Each ThreadState lazily creates a uniquely named, preallocated file and maps it through Env::NewMemoryMappedFileBuffer(). The file contains a fixed header and 512-byte text records. A record slot is marked I before its body is replaced and V only after the complete body is published; E denotes an unused slot. Each valid record carries a monotonically increasing sequence number so wrapped records can be ordered. Oversized context is bounded and ends with details_truncated=1. Lazy initialization avoids empty files for auxiliary ThreadState objects, and a process-wide file ID prevents collisions between ThreadState objects that share a logical thread ID. The periodic-flush flag, temp-file/rename snapshot path, and verification-failure fallback flush are removed. Enabled operation-context formatting continues to reuse a per-thread string and uses std::to_chars for integers; disabled call sites avoid formatting. Diagnostic directory setup and mapping failures are reported but do not change the workload result. Auto-directory pruning and cleanup accept only generated filename schemas, reject symlink roots, preserve unrelated files and explicit user directories, and retain at most eight PID groups. ## Safety and unsupported cases This is stress-tool-only diagnostic behavior. Raw db_stress keeps breadcrumbs disabled by default. The expected-state recovery trace and scan-witness contracts are unchanged. The mapping deliberately does not use msync or fsync: it is intended to survive a process crash while the kernel remains running, not power loss, host reset, or kernel failure. Filesystems or Env implementations without shared writable mappings disable breadcrumbs non-fatally. Replacing or truncating a live mapped file is unsupported. Records are fixed width, so unusually large operation context is explicitly truncated. Concurrent live readers are not promised a consistent snapshot; post-termination readers must ignore I and E records and order V records by sequence number. Nested operation scopes remain unsupported. ## Tests and limitations Validated with all 47 tools/db_crashtest_test.py tests, exact integrated debug and release db_stress builds, complete-stack arc lint, lower/upper/combined clang-format checks, normal ring wrap, distinct files for four worker threads, an oversized 256-column-family context, and process-crash tests. A release process killed with SIGKILL retained all 16 committed ring records without an explicit flush. A debugger-forced SIGKILL after the I store but before V publication left the interrupted slot marked I. In five CPU-pinned alternating release pairs of 50,000 operations, enabled and disabled medians were both 2.15 seconds. This remains a bounded latest history rather than a complete log and provides no host-crash or power-loss durability. ## Rebase update Rebased onto fbsource master dc59f986b964. The logical diff is unchanged; the rebased stack passes debug and release db_stress builds, all 47 db_crashtest Python tests, and arc lint. ## Review follow-up The mapped-slot invalidation now uses an acquire-release atomic exchange so later body stores cannot become visible before the I marker on weakly ordered architectures. Reviewed By: anand1976 Differential Revision: D119160910 fbshipit-source-id: ffe172af9ebc75c617196592806dc9641a539a25 |
||
|
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 |
||
|
Omkar Gawde
|
7d721f398b |
Reject reopening an active SstFileWriter (#15232)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15232 **What**: Make `SstFileWriter::Open()` return `InvalidArgument` without replacing the active writer when a file is already open. **How**: Check `rep_->builder` before constructing new output resources, matching `OpenWithEmbeddedBlobs()` and leaving the original file finishable. **Why**: A second open otherwise destroys an unfinished `TableBuilder`, violating its lifecycle contract and triggering an assertion. Reviewed By: pdillinger Differential Revision: D120469611 fbshipit-source-id: 8daa4607725eea574fe80012f9df98132160ce3e |
||
|
Jay Huh
|
bcf8711165 |
Add per-column-family ReadOptions to NewIterators (#15230)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15230 Add a raw C++ `DB::NewIterators()` overload accepting parallel vectors of `ReadOptions` and column family handles. Each iterator receives its own options while all iterators share one consistent sequence snapshot. Require identical snapshot pointers and tailing modes across the options. Implement the overload for primary, secondary, read-only, stackable, and write-prepared DBs. Building on D120048299, both secondary overloads share one helper that captures all SuperVersions and the sequence number under the DB mutex. Preserve read-only `table_filter` support when range-tombstone conversion is configured, and reject that unsafe combination in write-prepared read-write iterators before allocating any iterator. Add validation, per-CF prefix-seek coverage, a secondary catch-up regression test, table-filter contract tests, and a public API release note. Reviewed By: xingbowang Differential Revision: D120296578 fbshipit-source-id: 3775884537ea29a0d32bb3fe10330cf62ac5a406 |