mirror of
https://github.com/facebook/rocksdb.git
synced 2026-10-23 08:09:02 +03:00
main
14217 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Peter Dillinger
|
021374c62c |
Start format_version=8 development toward new features (#15104)
Summary: format_version background: SST files are shared across RocksDB versions. format_version is a critical tool in ensuring prior (and later) versioned readers do not misinterpret a file created by some version. Opt-in features can sometimes rely on previously-rejected corners of the schema space, but not always. This change: lands much of the format_version=8 (fv8) format surface as an internal-only draft -- readable and testable but not user-writable -- so upcoming features can build on a stable base. fv8 is NOT published: kLatestBbtFormatVersion stays 7, and reading or writing fv8 is gated behind the existing TEST-only TEST_AllowUnsupportedFormatVersion() hook. Publishing fv8 (raising the ceiling) is deferred to a later change. The pieces of fv8 added here, and what each unlocks: - Index value-delta escape. Existing behavior: index blocks (format_version >= 4) store each block handle as a delta against the previous entry's handle, and signal "this value is a full handle instead" by reusing the key sentinel `shared == 0`. Because the two are coupled, an index entry whose handle is not contiguous with the previous one must be forced to a restart, costing a full key as well as a full value. fv8 decouples them with an in-value escape codepoint: varint 0 means "a full BlockHandle follows," and genuine size deltas are remapped to zigzag(delta)+1 so that 0 stays free. A non-contiguous entry can then remain an ordinary key-delta-encoded, on-cadence entry. The escape applies to every index-shaped block, which includes the top-level index of index partitions and the index of filter partitions, not just the index over data blocks. Unlocks: the upcoming optimize_key_common_prefix index feature (removes its need to special-case super_block_alignment) and value-delta encoding for embedded-blob SSTs (previously disabled outright because interleaved blob records break handle contiguity). Guarded strictly on fv8; format_version <= 7 encoding and decoding are byte-for-byte unchanged. Note that restart points must not delta-encode, so they still have full values without a sentinel. - Data block footer bit 30 is reserved as an "extended metadata present" escape. Decode rejects a set bit as unsupported/corrupt at all versions. Unlocks: adding block-level metadata in the future through the one shared footer decoder, without another format_version bump or plumbing format_version into every block parser. (Older reader versions relied on this bit being zero for some integrity/consistency checking; format_version bump now makes it safe to use.) - Footer metaindex gap. For fv8, the top 16 bits of the footer's 4-byte metaindex-size field record the gap (in bytes) between the end of the metaindex block and the start of the footer; the low 16 bits hold the metaindex block size. Unlocks: a space very near the end of the file to save full file checksum state, for a "files carry their own checksums" feature that doesn't require SST parsing to extract the checksum states (to run them to completion on the trivial file tail). - Reserved "user_key_common_prefix" table property. The reader returns NotSupported when it is present and non-empty. Unlocks: a future whole-file optimization that stores the common prefix of all user keys once instead of in every restart key; until implemented, fv8 readers refuse such a file rather than mis-read it. - Because of the problem of inferring such a prefix at the start of constructing an SST file, we expect it will rely on SstPartitioner. Here we've added SstPartitioner::ShouldPartitionByPrefix() (not yet used) that will enable this optimization, and avoid the overhead of a virtual callback to SstPartitioner on every key written to an SST file. Supporting cleanups: the Footer's "unset" format_version sentinel is named (kInvalidFormatVersion) and the FormatVersionUses*() predicates assert against it, as each is a `version >= N` test that an unpopulated Footer would silently satisfy. Index block reader entry points (Block::NewIndexIterator() and friends) drop their defaulted trailing arguments so each caller states which codec it expects, and BlockBuilder::Add() takes delta_value by const reference rather than a pointer whose null case no caller used. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15104 Test Plan: New unit tests, all exercising fv8 through TEST_AllowUnsupportedFormatVersion(): - block_test: value-delta escape codec round-trip (delta 0, negative, large, and the escape; asserts format_version <= 7 bytes match the legacy encoding and a legacy 0x00 still decodes as delta 0), and rejection of footer bit 30 at all versions. - db_flush_test: fv7-vs-fv8 read equivalence for super_block_alignment, including the separate_key_value_in_data_block intersection, confirming padding events (hence escape entries) occur and fv8 index values are delta encoded. - sst_file_reader_test: fv7-vs-fv8 read equivalence for embedded-blob SSTs, confirming fv8 (unlike fv7) delta-encodes their index values and actually emits escape entries (counted through a sync point). - block_based_table_reader_test: a builder+reader test that injects a footer metaindex gap via a sync point (verifies the gap bytes are written and the reader locates the metaindex via the recorded gap, and that too large a gap surfaces as a builder error), and that a non-empty reserved user_key_common_prefix property makes the reader return NotSupported. - table_test: two-level index with partitioned filters and index_block_restart_interval > 1, the configuration in which both top-level indexes delta-encode their values; verifies scan and point lookup round trips at each tested format_version including fv8. - (Not strictly fv8) Basic SstPartitionerFixedPrefix tests in slice_test (ShouldPartition, ShouldPartitionByPrefix, their mutual consistency, and CanDoTrivialMove), which had no non-DB coverage before. fv8 is also added to the table_test harness (GenerateArgList) and the shared footer version set (test::kFooterFormatVersionsToTest), so the existing table-level and DB-level property/checksum suites now also run at fv8. And added to crash test (minor revisions to db_stress). ## Performance details Release db_bench (PORTABLE=0, -march=native) A/B/C on the read/write fast path, comparing (a) parent, (b) this commit @ fv7, (c) this commit @ fv8. All DBs on tmpfs, 20M keys, block_size=2048, compression=none. Note index_block_restart_ interval=16 (not the default 1) is required to actually exercise the fv7<->fv8 index value-delta codec -- with restart interval 1 every index entry is a full- handle restart and the changed path never runs (confirmed delta-value=1 via sst_dump). Reads single-thread + core-pinned, warm cache, interleaved/rotated rounds, paired deltas. Reads (readrandom + seekrandom, non-partitioned single ~1.9MB index block and partitioned index+filters): fv7->fv8 in the *same* binary is within noise on all four workloads (largest -1.0%, CIs cross zero). before->after (fv7 path) is also within noise; perf shows a deterministic +0.4-0.6% instructions/op from the added branches (the format_version>=8 value_delta_escape guard + refactored data-block footer decode), which is below the throughput noise floor. Writes (fillseq + compact total CPU): fv7->fv8 null. compact (which rewrites all index blocks) is null before->after. fillseq showed +3.5% CPU before->after, but perf attributes it entirely to cycles/IPC with *identical* instructions/op (+0.01-0.04%) -- a code-layout effect on the untouched memtable-insert loop, not this change. Sanity: any single throughput/CPU delta <~1% is within the build-to-build noise floor, measured directly by comparing two byte-identical binaries (swings up to ~0.9% reads; the pre-native/PORTABLE=1 build showed a spurious -1.85% that vanished under -march=native). Net: no measurable regression from fv8 draft; fv7 encoding/decoding byte-for-byte unchanged, only the noted sub-1% instruction bump on the shared read path. Reviewed By: xingbowang Differential Revision: D116105558 Pulled By: pdillinger fbshipit-source-id: ead776cdd396d497d47872c0956ea8c651c0d13f |
||
Josh Kang
|
9475798205 |
Refresh recovery plan after error escalation (#15111)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15111 T284514290 exposed an online-recovery race between the automatic recovery coordinator and a foreground synchronous write. A retryable no-WAL flush error selected lightweight retry-flush recovery. While that recovery attempt waited for the failed background flush, `Delete(sync=true)` appended and synced sequence N to the active WAL, then failed while persisting WAL-tracking metadata to the MANIFEST. Because memtable insertion and sequence publication happen after that MANIFEST step, the record was durable but logically unpublished. OLD: the concurrent error triggered another attempt, but the attempt reused a stale recovery plan. That preserved both ingredients needed for a duplicate: the first `Delete(key, N)` remained in the active WAL, while the published sequence remained `N-1`. ``` Recovery coordinator Foreground sync-write thread ------------------------------------ -------------------------------- [db_mutex held] plan = RetryFlush (copied once before the retry loop) attempt 1: ResumeImpl(plan) WaitForBackgroundWork() bg_cv_.Wait() [atomically releases db_mutex] ---------> assign sequence N append + sync Delete(key, N) [db_mutex held] MANIFEST WAL edit fails no memtable insert no LastSequence publication write returns I/O error recovery_error_ = Hard (requests another attempt) bg_error_ = Hard recover_context_ eventually becomes FullRecovery [db_mutex released] [db_mutex reacquired] <--------- ClearBGError() sees recovery_error_ attempt 1 cannot clear the error attempt 2: recovery_error_ = OK ResumeImpl(plan) ^ +-- still the cached RetryFlush plan RetryFlushesForErrorRecovery() retries the failed immutable flush only does not call SwitchMemtable() does not switch the active WAL ClearBGError() succeeds caller retries Delete(key) LastSequence is still N-1 => assigns sequence N again appends Delete(key, N) to same WAL memtable insert succeeds publishes LastSequence = N Reopen / WAL replay: first (key, N, deletion) -> memtable insert succeeds second (key, N, deletion) -> TryAgain("key+seq exists") normal WAL recovery has seq_per_batch_ == false assert(seq_per_batch_) fails ``` The cached argument to `ResumeImpl()` is therefore the direct bridge to the assertion: lightweight retry-flush recovery can report success without retiring the WAL that contains the unpublished record. Since it also does not publish sequence N, the successful caller retry writes the identical internal key into that same WAL. NEW: the same recovery coordinator consumes the concurrent error through a mutex-ordered handoff. ``` Recovery coordinator Foreground sync-write thread ------------------------------------ -------------------------------- [db_mutex held] attempt 1: plan = recover_context_ // RetryFlush ResumeImpl(plan) WaitForBackgroundWork() bg_cv_.Wait() [atomically releases db_mutex] ---------> append + sync Delete(key, seq=N) [db_mutex held] MANIFEST WAL edit fails CheckAndSet...(Hard, FullRecovery): recovery_error_ = Hard bg_error_ = Hard recover_context_ = FullRecovery [db_mutex released] [db_mutex reacquired] <--------- ClearBGError() sees recovery_error_ current attempt cannot clear the error next retry, with db_mutex held: recovery_error_ = OK plan = recover_context_ // FullRecovery ResumeImpl(plan) SwitchMemtable() switch to a new WAL clear the background error ``` The foreground unlock of `db_mutex_` happens before the recovery coordinator reacquires it and snapshots the next plan. Publishing `bg_error_` and `recover_context_` together under that mutex prevents the next attempt from observing a new severity with an old recovery requirement. `recovery_error_` is the per-attempt latch: a concurrent I/O error cannot be cleared as part of the in-flight attempt; it either stops recovery or forces another attempt to consume the updated context. If another error arrives while `ResumeImpl()` releases the mutex again, the same handoff repeats. No second recovery coordinator is needed. Manual `Resume()` now uses the retained recovery context instead of reconstructing one from the background-work scheduling flag, and the soft-error clear-only shortcut is allowed only when that flag shows there is no pending recovery work. The write ordering and `assert(seq_per_batch_)` detector are unchanged. Reviewed By: anand1976, xingbowang Differential Revision: D116106177 fbshipit-source-id: a78450c0924a326b5302ae6709e367f07bd3d5a6 |
||
|
Josh Kang
|
323d915dbc |
Prevent late async-read stats context restoration (#15100)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15100 Fix the TSAN race in T284274705 where `CoroutineStatsRequestData::onSet()` can access its borrowed `Env*` after `AsyncCallback::OnComplete()` allows the caller to destroy that environment. When `co_await baton` suspends, the Task's implicit `ViaIfAsync` saves the read's statistics `RequestContext` (`S`) as the context needed by the real RocksDB continuation. The old explicit `executor->add()` also ran the Baton bridge under `S`. Consequently, `baton.post()` resumed the Via trampoline under `S`, and the trampoline's own `executor_->add()` caused EventBase to capture `S` a second time as the queued continuation's outer context. After the read removed its stats scope and called `OnComplete()`, the Via guard restored that outer `S`, triggering a late `onSet()` that dereferenced the potentially destroyed `Env*`. Remove the redundant outer executor hop and call `baton.post()` directly under an explicitly null `RequestContext`. If the read is suspended, `post()` resumes only the Via trampoline; the trampoline still queues the real continuation on the bound read executor. The Via promise retains its saved `S`, so `executeContinuation()` installs `S` while RocksDB executes, but EventBase now captures null as the outer context restored afterward. If filesystem completion happens before `co_await baton`, `post()` only marks the Baton ready and the read continues inline under its existing context. Thread Race: `E` = temporary backup `Env` `S` = statistics `RequestContext` containing `CoroutineStatsRequestData{env_ = E}` `P` = parent context without RocksDB statistics `B` = old explicit Baton bridge `C` = `ViaIfAsync` callback that resumes the real read ``` Caller / stress thread RocksDB read EventBase thread ---------------------- ----------------------------- BackupInfo owns shared_ptr<Env> E Call MultiGetAsync() Wait for OnComplete() Start the RocksDB read coroutine Install S: P -> S S.onSet() Call SubmitReadAsync() No io_uring is available, so: Read() executes synchronously filesystem callback runs inline callback is still under S Old code calls executor->add(B) EventBase records: B.outer_context = S Reach co_await baton B has not executed, so Baton is not ready ViaIfAsync: saves execution_context = S gives trampoline V to Baton Suspend the real read Leave S: S -> P S.onUnset() Later, EventBase runs B Install B.outer_context: P -> S S.onSet() B calls baton.post() -> V resumes inline -> V queues callback C -> EventBase records: C.outer_context = S Later, EventBase runs C Install C.outer_context: P -> S S.onSet() Via executeContinuation(): remembers previous context = S installs saved execution context S resumes the real RocksDB read Read finishes Statistics scope removes S: S -> P S.onUnset() Call OnComplete() Wait returns <--------------- OnComplete signals caller ================ PUBLIC READ IS COMPLETE ================ Destroy restored DB C is still unwinding Release BackupInfo Via guard restores previous context: Last shared_ptr<Env> disappears P -> S Begin Env E destruction S.onSet() -> E->GetThreadID() Env::~Env() <--------------- TSAN race ---------------> S.onSet() ``` The same context was captured in two roles: ``` Via execution_context = S required while the real read executes C.outer_context = S accidentally restored after the read completes ``` The fix removes bridge `B` and posts under null. For an outstanding asynchronous read, the Via trampoline therefore queues `C` with `outer_context = null`, while retaining `execution_context = S`. The real read still executes under `S`, but after `OnComplete()` the Via guard restores null instead of reactivating `S`. Reviewed By: anand1976 Differential Revision: D115967513 fbshipit-source-id: 87d744ecdbad3421c4d2a1a7be6423a3f0373573 |
||
|
Peter Dillinger
|
2dc6bc51b4 |
Fix TSAN data race on speculative auto_skip_bypassed read (#15094)
Summary: The parallel-compression auto-skip feature (https://github.com/facebook/rocksdb/issues/15079) introduced a data race that TSAN reports in block_based_table_reader_test. In StateTransition(), the auto-skip loop reads ring_buffer[cslot].auto_skip_bypassed speculatively, from a not-yet-committed atomic_state snapshot, before the CAS that would validate that snapshot. Unlike every other BlockRep field (touched only after a successful CAS grants exclusive slot ownership), a stale snapshot can point that read at a physical ring-buffer slot the emit thread is concurrently recycling for a new block in EmitBlockForParallel, racing the plain write there. The speculative result is discarded when the CAS fails, so there is no logical corruption, but the plain read/write is a data race (UB) and is flagged by TSAN. Make auto_skip_bypassed a RelaxedAtomic<bool> so the speculative read and the emit-thread write are atomic. Relaxed ordering suffices: correctness of the auto-skip decision is still gated by the CAS, and when the CAS succeeds the value was published by the emit thread through the existing acquire/release ordering on atomic_state and is not modified elsewhere. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15094 Test Plan: Covered by existing block_based_table_reader_test parameterizations that build with parallel compression (parallel_threads > 1) under TSAN, which is where the race was originally reported. Reviewed By: joshkang97 Differential Revision: D115777206 Pulled By: pdillinger fbshipit-source-id: 7ee969ec2d95890255d2e8b76eceaa5876b82170 |
||
|
Peter Dillinger
|
7217bf34a7 |
Add SST build context: is_remote_compaction, db_name, kSstFileWriter (#15085)
Summary: Custom filter and compression strategies decide what to build per SST from a FilterBuildingContext (shared by FilterPolicy::GetBuilderWithContext and CompressionManager::GetCompressorForSST). This adds more context so those strategies can adapt to where and how a file is being produced. FilterBuildingContext gains two fields: * is_remote_compaction: true only when the file is produced by an offloaded CompactionService worker. Detected with a new virtual CompactionJob::IsRemoteCompaction() (default false) overridden by CompactionServiceCompactionJob, mirroring the existing GetTableFileName hook. Local/fallback compactions and flushes are false. * db_name: a non-owning Slice naming the DB, populated for flush, compaction, recovery, and repair, and empty only where there is no DB, as with SstFileWriter. Under remote compaction it is the DB as opened by the worker, which can differ from the primary's. Both are threaded through TableBuilderOptions and populated in BlockBasedTableBuilder::Rep, whose "is this file in the LSM" test becomes a switch with no default case, so that a future TableFileCreationReason has to be classified there. Separately, TableFileCreationReason is now backed by uint8_t and gains a kSstFileWriter value, which SstFileWriter reports instead of the catch-all kMisc. It is appended after kMisc so existing enumerator values are unchanged; those values are public API, surfaced to EventListener and mirrored by the Java binding (updated here). Note that SstFileWriter neither notifies listeners nor consults CompactionFilterFactory, so for now the new value is only observable through FilterBuildingContext. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15085 Test Plan: * New CompactionServiceTest.IsRemoteCompactionInCompressorContext installs a custom CompressionManager and asserts GetCompressorForSST sees is_remote_compaction == true for the offloaded worker's output files and false for the primary's flushes in the same run, so the flag is shown to be per-file rather than global. IsRemoteCompactionFalseForLocalCompaction covers the same manager with no compaction_service configured. * DBBloomFilterTest.ContextCustomFilterPolicy now reports db_name as a token (expected / none / UNEXPECTED:<value>), so the value rather than just its presence is checked for flush and compaction, and expects kSstFileWriter instead of kMisc for the external SstFileWriter file. * bloom_test's "Like SST file writer" case uses kSstFileWriter so that it keeps modelling SstFileWriter. * Java EventListenerTest references TableFileCreationReason.MISC.getValue() instead of a hardcoded enum byte, so it stays correct across enum changes. Reviewed By: jaykorean Differential Revision: D115603616 Pulled By: pdillinger fbshipit-source-id: ad73fbc391a5377ace406725238ae6eede46c8d9 |
||
Xingbo Wang
|
58d835d951 |
Add db_stress liveness profile and fix lost flush request (#14830)
Summary: Add a db_crashtest liveness mode that runs db_stress with a watchdog for foreground progress. The profile disables fault injection and uses longer runs so failures are more likely to represent real forward-progress bugs instead of expected injected I/O errors. The watchdog tracks completed operations, active operation type/start time, and per-operation completion counts to distinguish stuck writes from long admin operations. Bug: The new mode exposed a latent flush scheduling bug. A flush request can be queued with an effective max memtable id, then a newer immutable memtable can be added before the background flush picks work. PickMemtablesToFlush() could pick only the older memtables and still clear flush_requested_. A later queued flush for the newer memtable would then be skipped because the column family no longer looked flush-pending, leaving the immutable memtable permanently unflushed. In stress this surfaced as IngestExternalFile() waiting in WaitForFlushMemTables(), with writes eventually blocked behind the same unflushed memtable. Fix: Keep flush_requested_ set until all unstarted immutable memtables have been picked. Add a direct MemTableList regression for the partial-pick case and a DB-level regression covering the ingest/get-live-files ordering that reproduced the lost request. ## Testing - CI - Local liveness mode 32 full 20-minute runs passed, 0 failures Pull Request resolved: https://github.com/facebook/rocksdb/pull/14830 Reviewed By: joshkang97 Differential Revision: D107895052 Pulled By: xingbowang fbshipit-source-id: ca67c5f9376899eefdc9af7bad4af06200b52446 |
||
Michael C Huang
|
276495843d |
Add blob file writer buffer size option (#15061)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15061 Adds mutable ColumnFamilyOptions field `blob_file_writable_file_max_buffer_size` to cap blob-file WritableFileWriter buffers independently from DBOptions::writable_file_max_buffer_size. The default value is 0, which preserves existing behavior by inheriting the DB-wide writer buffer cap. Non-zero values apply to both flush/compaction blob writers and direct-write blob writers. Direct-write updates are propagated through DBImpl::SetOptions to newly opened blob files without requiring a DB restart. Also wires option parsing, dumping, db_bench, db_stress, crashtest, and release-note coverage. Reviewed By: xingbowang Differential Revision: D114979265 fbshipit-source-id: 3f764b797628eee0ce379110c1a1f8a16beb3fa2 |
||
|
Peter Dillinger
|
ec5106b3b8 |
Fix AI code review hiding details, stale+misleading driver (#15087)
Summary: The Claude/Codex CI reviewer was posting PR comments like "the review has been written to review-findings.md" instead of the review itself, and the comment footer cited claude_md/code_review.md as the review methodology even though CI stopped feeding that file to the model in https://github.com/facebook/rocksdb/issues/14507 (it switched to claude_md/ci_review_prompt.md). code_review.md had been orphaned from CI for months while still advertising itself as the CI methodology, so edits to it (e.g. the "post directly in your response" note) had no effect on CI. Separately, an over-long review exceeded GitHub's per-comment character limit and was rejected outright, losing the whole review. Changes: - ci_review_prompt.md: make the final response the review -- it becomes the PR comment verbatim -- and forbid ending the turn with a meta-summary such as "written to review-findings.md". review-findings.md is demoted to a recovery backstop; the per-phase incremental writes and the turn-limit recovery path are unchanged, so both the normal and recovery comment paths still produce a real review. - Repoint the "Review methodology" footer in parse-claude-review.js and parse-codex-review.js to claude_md/ci_review_prompt.md (the file CI actually uses), so the citation no longer points at a deleted file. - build-ai-review-comment.js: cap the assembled comment at GitHub's 65536-char limit, trimming only the model response (the sole unbounded input) and appending a pointer to the execution-log artifact, so an oversized review is truncated cleanly instead of rejected by GitHub. The header, footer, and <details> scaffolding are always preserved. - Delete claude_md/code_review.md (CI-dead since https://github.com/facebook/rocksdb/issues/14507; superseded locally). - Add claude_md/manual_code_review.md: a lightweight, single-reviewer local wrapper around ci_review_prompt.md (no multi-agent team/debate, no scratch files, inline output) for running a modest review by hand. It sources its methodology from ci_review_prompt.md so it cannot drift. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15087 Test Plan: - The workflow scripts have no CI unit coverage; verified both parse-*-review.js still parse, and that no reference to code_review.md remains anywhere. - Dry-ran manual_code_review.md by reviewing the current commit locally; it produced a right-sized single-reviewer report without spawning agents or writing files, and helped find+fix the comment limit issue. - Exercised the end-to-end GitHub CI path (per https://github.com/facebook/rocksdb/issues/14696, review-workflow changes not yet on main can only be tested from a branch on facebook/rocksdb): pushed these changes to the fix_hidden_review_details_test branch and ran the Claude Code Review workflow (claude-review.yml) via "Run workflow" against PR https://github.com/facebook/rocksdb/issues/15079, confirming the posted comment carries the full review inline (not a review-findings.md summary) with the ci_review_prompt.md footer. https://github.com/facebook/rocksdb/actions/runs/31537781229 https://github.com/facebook/rocksdb/pull/15079#issuecomment-5259127037 Reviewed By: xingbowang Differential Revision: D115635558 Pulled By: pdillinger fbshipit-source-id: d3ca7188162a086cd42d7b8bd8e60f3fc3df3bad |
||
Andrew Chang
|
f9e4c5c51f |
Add stat for number of bottom-pri compactions. (#15071)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15071 __Why__: Give operators visibility into currently running bottom-priority compactions. Low-priority running compactions can be derived from `rocksdb.num-running-compactions - rocksdb.num-running-bottom-compactions`. __What__: Expose `rocksdb.num-running-bottom-compactions` as a new statistics property. __How__: We already record `num_running_bottom_compactions_`, so this feeds that value through the DB property path. Reviewed By: xingbowang Differential Revision: D114989532 fbshipit-source-id: efa19ef3a69eaecf4cbbbc9e4fc9cd55fce09ca5 |
||
|
Peter Dillinger
|
fcf382c95c |
Integrated and ratio-aware compression auto-skip (#15079)
Summary: Auto-skip compression (stop attempting compression on data that isn't compressing well, to save CPU) was previously an experimental CompressionManager wrapper (CreateAutoSkipCompressionManager) whose predictor lived in each per-thread working area. Besides being fragmented across parallel workers and reset on a tumbling window, it decided based on how OFTEN blocks failed to compress, ignoring how MUCH they compress -- so on mixed data it could give up substantial compression to save a little CPU (see performance results). This makes auto-skip a first-class, optional, ratio-aware feature of the block-based table builder, controlled by two new CompressionOptions fields: - auto_skip (default false): the on/off switch. There is no separate threshold knob; the decision reuses max_compressed_bytes_per_kb (the existing per-block "minimum compression worth keeping" bar) as the bar for whether attempting compression on the stream is worthwhile. - auto_skip_min_sample_every: sampling interval that bounds the local impact around a transition back to compressible data. 0 selects an internal default (currently 64 skipped data blocks between forced samples). The emit thread makes the skip/attempt decision once per data block (serial and parallel paths alike; data blocks only), so a skipped block is never dispatched to a compression worker. It is driven by a continuously updated, shared estimate q_hat of the aggregate stored/uncompressed ratio the builder would achieve by attempting -- each block counted at its achieved ratio, or 1.0 when stored raw: attempt while q_hat is at or below the bar, otherwise skip. Because the estimate is payoff-weighted, a stream whose blocks each save little -- or where only a few blocks compress well -- does not clear the bar and is skipped, while a stream that compresses substantially is attempted. The compression given up per KB is bounded by (1024 - max_compressed_bytes_per_kb). q_hat uses an asymmetric exponentially weighted moving average (EWMA), slightly faster toward better compression than toward worse; the fast side is deliberately gentle so a single marginally-compressible sample does not flip a whole file back to attempting. It is kept on its own cache line to avoid false sharing. Under parallel compression q_hat is updated with relaxed atomics, and the emit thread can decide from a value that lags the learning it drives by the in-flight depth -- tolerable concessions of accuracy and responsiveness for a coarse heuristic. The first data block of each file is always attempted (the sample countdown starts at 0), and while skipping the builder keeps sampling (one block after a randomized gap) so a return to compressible data is detected. q_hat is carried across the files a flush/compaction thread emits via thread_local state keyed by preferred compression type, so a sustained region does not re-learn on every file. The old CompressionManager-based AutoSkip is removed (CostAware is retained); db_stress, db_bench, and db_crashtest are migrated to the new option. db_bench is extended so it can generate the relevant regimes: --compressible_data_fraction and --compressible_data_run_length produce a mix of compressible/incompressible blocks -- i.i.d. per value (run length 1) or in alternating regions (large run length). The default (fraction 1.0) reproduces the historical single-pool behavior. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15079 Test Plan: New/updated unit tests in `util/compression_test.cc` (`DBAutoSkipTest`): - skips sustained incompressible data; - never skips compressible data; - disabled by default never bypasses a block; - 4-thread parallel compression round-trips all data across alternating compressible/incompressible regions. **Performance** (`db_bench fillseq`, release, zstd level 3; 100k values of 16 KiB each, roughly one value per data block; WAL and auto-compaction off; median of 5 runs on a quiet host). Auto-skip trades one cost for another, so each cell reports two costs, both lower-is-better: CPU cost as process CPU time (user+sys) per GB of uncompressed input (s/GB; ~1.64 GB uncompressed here), and storage cost as the resulting DB size. Auto-skip does not appear to change parallelizability, so wall-clock s/GB tracks CPU s/GB (the two agree here); CPU s/GB is reported. Percentages are relative to the no-auto-skip baseline: negative CPU = cheaper, positive size = more storage (shown only where storage grows). | Workload | baseline (off) | (a) old wrapper | (b) draft | (c) this version | |---|---|---|---|---| | Incompressible | 2.33 s/GB, 1568 MB | 0.96 (-59%), 1568 MB | 0.82 (-65%), 1568 MB | 0.82 (-65%), 1568 MB | | Compressible | 3.55 s/GB, 710 MB | 3.54 (~0%), 710 MB | 3.56 (~0%), 710 MB | 3.55 (~0%), 710 MB | | Marginal-payoff mix | 3.80 s/GB, 1376 MB | 1.54 (-59%), 1523 MB (+11%) | 3.81 (~0%), 1376 MB | 1.43 (-62%), 1531 MB (+11%) | | High-payoff mix | 2.20 s/GB, 929 MB | 1.14 (-48%), 1417 MB (+53%) | 2.16 (~0%), 929 MB | 2.17 (~0%), 929 MB | A strategy wins by cutting CPU cost without paying much storage cost. On incompressible data all three cut CPU ~60% at no storage cost; on compressible data none skip (within run-to-run noise). The two mixes (50% incompressible, 50% compressible per block) separate the strategies: - Marginal-payoff mix: the compressible half barely clears the per-block bar, so the stream is not worth compressing -- skipping is right. (c) and (a) cut CPU ~60% for ~11% more storage. That extra storage is compression (c) intentionally declines because its aggregate benefit is below the configured worthwhile threshold (max_compressed_bytes_per_kb, ~12.5% per KB); it stays within the (1024 - max_compressed_bytes_per_kb) bound. The ratio-unaware draft (b) cannot see the low payoff, keeps compressing, and saves no CPU. - High-payoff mix: the compressible half compresses hard, so compressing is right. (c) and (b) keep full compression at baseline storage; the old wrapper (a) cuts CPU 48% but inflates storage +53% -- far beyond that bound -- giving up worthwhile compression. Only (c) makes the right call on both mixes: it never gives up worthwhile compression (unlike (a)) and does not waste CPU on low-payoff data (unlike (b)). Reviewed By: xingbowang Differential Revision: D115444567 Pulled By: pdillinger fbshipit-source-id: 05e122d25de95ddcd0edeb0dd07c9d25bead7293 |
||
|
Xingbo Wang
|
66d7035708 |
Do not test GetEntityLazy on read-only primary handle (#15084)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15084 MaybeOpenReadOnlyOnPrimary is intended to exercise opening and closing a read-only DB on the primary live directory, then verify that read-only close does not delete files still live in the primary. D115278311 diagnosed T283967366 as a race between GetEntityLazy on that read-only handle and the primary deleting SST files. After checking the call path, the ENOENT is from GetEntityLazy's initial point lookup opening an SST through TableCache, not from deferred lazy column resolution. Other read APIs are not exercised on this read-only-on-primary handle, so adding GetEntityLazy there made this old open/close harness test also perform a live-directory point read. Remove the read-only-handle GetEntityLazy exercise and keep GetEntityLazy coverage in the normal stress read paths, consistent with the other read operations. Reviewed By: pdillinger Differential Revision: D115569968 fbshipit-source-id: fcef30d09903525c2a7897b9c52bc9ef951bed6a |
||
|
Andrew Chang
|
9381544cd9 |
Add num-running-remote-compactions DB property (#15074)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15074 Why: We want to be able to monitor how many remote compaction jobs are running on a given server. Currently we know the total number of running compactions but not the local vs remote breakdown. What: Add a new `rocksdb.num-running-remote-compaction-jobs` statistic. This measures in-progress remote compaction jobs. Note that remote compaction work is scheduled per subcompaction, so one RocksDB compaction may contribute more than one running remote compaction job. Reviewed By: jaykorean Differential Revision: D115324149 fbshipit-source-id: 9397e39acd696c88ce7499ecb0da68de215bf1f3 |
||
|
Xingbo Wang
|
f8c601b91c |
Fix io_activity assertion in stress test after MultiGetEntityLazy (#15076)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15076 Fix the `io_activity == Env::IOActivity::kUnknown || io_activity == options.io_activity` assertion failure in the crash test. The crash occurs because `TestMultiGetEntity` in `no_batched_ops_stress.cc` changes the thread operation to `OP_GETENTITY` during its consistency-check phase (the `check_results` lambda calls `SetThreadOperation(OP_GETENTITY)` before calling `GetEntity` for comparison). When `MaybeTestMultiGetEntityLazy` is called afterward, it invokes `MultiGetEntityLazy` which correctly sets `read_options.io_activity = kMultiGetEntity`, but the thread operation is still stale at `OP_GETENTITY`. The debug assertion in `DbStressRandomAccessFileWrapper::Read` then sees `TEST_GetExpectedIOActivity(OP_GETENTITY) = kGetEntity != kMultiGetEntity` and aborts. The fix restores `SetThreadOperation(OP_MULTIGETENTITY)` before each call to `MaybeTestMultiGetEntityLazy` in `TestMultiGetEntity`. Triggered by commit cb73fc845fe0 which added the `MaybeTestMultiGetEntityLazy` calls. The `--lazy_entity_read_one_in=100` flag (non-default) enables this code path. Reviewed By: pdillinger Differential Revision: D115021723 fbshipit-source-id: e3eed600db4986cab27d7b1924282b1dfb8b0a7a |
||
|
Josh Kang
|
5dfde1c262 |
Add coroutine reads to stackable RocksDB utilities (#15080)
Summary: ## Summary Pull Request resolved: https://github.com/facebook/rocksdb/pull/15080 Introduce reusable `CoroStackableDBBase` forwarding and public `CoroDB::CoGet()` / `CoMultiGet()` entry points matching the `DB::Get()` / `MultiGet()` overloads. These helpers schedule native coroutine reads on the read executor and use the synchronous DB API when coroutine support or a read executor is unavailable. Use the stackable coroutine hooks for TTL and WritePrepared transaction reads. TTL preserves expiration decoding and timestamp behavior, while WritePrepared preserves transaction visibility. TTL keeps a single `DB` inheritance path while adding `CoroDB`: ``` DBWithTTLImpl `-- CoroStackableDBBase<DBWithTTL> |-- DBWithTTL -> StackableDB -> DB `-- CoroDB ``` `CoroDB::CoGet()` dispatches through the stackable coroutine hook and schedules the operation on the read EventBase. `CoMultiGet()` follows the same path: ``` CoroDB::CoGet(db, ...) |-- db->GetCoroDB() -> DBWithTTLImpl |-- no CoroDB or read executor -> db->Get() `-- schedule on read EventBase `-- virtual CoroDB::GetCoroutine() `-- DBWithTTLImpl::GetCoroutine() |-- CoroStackableDBBase<DBWithTTL>::GetCoroutine() | `-- wrapped CoroDB::GetCoroutine() `-- strip TTL timestamp ``` Reviewed By: xingbowang Differential Revision: D115232510 fbshipit-source-id: 59e5090db28d15bcff1a8b9b8c0b30ca7eeb742d |
||
|
Xingbo Wang
|
44e0294c1d |
Fix remote compaction filter test flake (#15075)
Summary: - Disable automatic compactions in `CompactionServiceTest.CompactionFilter`. - Keep the test focused on the explicit `CompactRange()` path that verifies remote compaction filter behavior. - Avoid a flaky race where background remote compaction can obsolete an input SST before the manual compaction runs. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15075 Test Plan: - `make format-auto` - `AUTO_CLEAN=1 COERCE_CONTEXT_SWITCH=1 make -j32 compaction_service_test` - `tools/gtest_parallel_repro.py --binary ./compaction_service_test --gtest_filter=CompactionServiceTest.CompactionFilter --processes-per-iteration 32 --iteration-count 30 --cpu-count 4 --timeout 120 --stop-on-failure --out /tmp/ws28_compaction_service_flake_fixed_20260809_1` - `git diff --check` - `AUTO_CLEAN=1 make check-sources` Reviewed By: jaykorean Differential Revision: D115357593 Pulled By: xingbowang fbshipit-source-id: 9c640b807b8d0c204a23fb805e511b39db69acbc |
||
|
Xingbo Wang
|
3f4d840e1a |
Fix multi-DB checkpoints in legacy regression test (#15078)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15078 The legacy regression test runner attempted to discover multi-DB origin directories by running find through run_remote, then assigned dirs=$?, which captured only find's exit code. For NUM_MULTI_DB > 1 this meant the checkpoint loop operated on the value 0 instead of the discovered sub-DB directories. For NUM_MULTI_DB > 1, build checkpoints for the deterministic numeric sub-DB directories 0..NUM_MULTI_DB-1 and explicitly fail if any expected origin DB is missing. This avoids rediscovering DBs through find output, so checkpoint creation no longer depends on pipeline exit status or whitespace-sensitive word splitting. Reviewed By: joshkang97 Differential Revision: D115380183 fbshipit-source-id: 24adfa37f872dd7b8ee4ee986cf4a7a4e58562d0 |
||
|
Andrew Chang
|
3949208acf |
Make max_open_files configurable via options_map in OpenAndCompact (#15077)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15077 __Why__: Allow offload compaction workers to configure `max_open_files` via `options_map`. __Problem__: `OpenAndCompact` hard-codes `max_open_files` to -1. This overwrites any value set via the `options_map` and prevents configuration in the remote compaction workers. __Fix__: Remove the hard-coding assignment. The value now comes from: 1. OPTIONS file via `LoadOptionsFromFile` 2. `override_options.options_map` via `GetDBOptionsFromMap` The default value of `max_open_files` was already -1, so existing behavior is preserved. Reviewed By: jaykorean Differential Revision: D115373592 fbshipit-source-id: fa742397246092544ab6f9004678be7ae938d051 |
||
|
Josh Kang
|
88752298ff |
Add native coroutine read dispatch to RocksDB (#15073)
Summary: ## Summary Pull Request resolved: https://github.com/facebook/rocksdb/pull/15073 The callback-based async read APIs make coroutine-native implementations and StackableDB composition awkward because wrappers must bridge callbacks back into tasks. This change introduces `CoroDB` as a side capability exposing lazy `GetCoroutine` and `MultiGetCoroutine` tasks. `DBImpl` and `CompactedDBImpl` implement this capability directly, while `DB::GetAsync` and `DB::MultiGetAsync` remain available as compatibility entry points with synchronous fallback. The native interface should also be more efficient because callers directly await the DB coroutine. It avoids the callback/Baton adapter, callback context handoff, and explicit rescheduling required for each request. Native tasks must run on the DB filesystem read executor's EventBase; debug builds verify this affinity. Public API documentation, a release note, and native-coroutine support in `db_bench` and `db_stress` are included. ## Dispatch DB::GetAsync -> GetCoroDB() on the concrete DB -> virtual CoroDB::GetCoroutine(...) -> DBImpl::GetCoroutine or CompactedDBImpl::GetCoroutine -> AsyncCallback::OnComplete(...) Virtual dispatch selects the coroutine implementation belonging to the concrete DB while preserving the existing callback API. ## Statistics Coroutine reads preserve RocksDB's TLS statistics behavior while keeping concurrent requests isolated. Each top-level task copies only the caller's statistics configuration—`PerfLevel`, per-level perf context enablement, and IO statistics enablement—into fresh request-local `PerfContext` and `IOStatsContext` objects. Existing counters are not inherited. If both perf and IO statistics are disabled, no request statistics context is installed. Folly `RequestContext` preserves these contexts across suspension and resumption on the read executor's EventBase. At completion, RocksDB publishes the request's collected values to TLS, allowing the awaiting caller to use `get_perf_context()` and `get_iostats_context()` as after a synchronous read. Callers must consume or copy them before leaving that EventBase. Statistics scopes are installed only by top-level coroutine reads and cannot be nested. ## Benchmarks Compared the previous `GetAsync` callback bridge with native `GetCoroutine`. Both variants used optimized builds, the same compacted DB, a fixed seed, eight executor threads, ten jobs per thread, and 2.56M reads per invocation. Results are the mean of three independent invocations using the default RocksDB block cache. A sequential read warmed the OS page cache before the non-direct-read measurements. | Workload | `GetAsync` bridge | Native `GetCoroutine` | Change | | Direct I/O | 237,710 ops/s | 243,588 ops/s | +2.47% | | Warm OS page cache | 783,859 ops/s | 891,531 ops/s | +13.74% | The direct-I/O difference is within observed run-to-run variation. When reads are served from the OS page cache and API overhead is a larger portion of latency, the native interface improves throughput by 13.74%. Every invocation completed all requested reads. ```bash db_bench \ --benchmarks=readrandomcoroutine \ --db=$CORO_BENCH_DB \ --use_existing_db=true \ --num=2000000 \ --value_size=1024 \ --compression_type=none \ --threads=8 \ --coro_jobs_per_thread=10 \ --reads=320000 \ --use_direct_reads=<true|false> \ --perf_level=1 \ --statistics=true \ --seed=20260807 ``` Reviewed By: xingbowang Differential Revision: D115232511 fbshipit-source-id: 30b48da2a798094d9080178f5430aae26e6c5954 |
||
|
Andrew Chang
|
1dfa729793 |
Expose number of unscheduled compactions as a statistic (#15060)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15060 Expose the `rocksdb.num-unscheduled-compactions` DB statistic to help troubleshoot high L0 counts. The metric captures the raw `unscheduled_compactions_` count. It measures compaction work waiting in the DB internal compaction queue, but not yet assigned to a background job. This complements `rocksdb.compaction-pending`, which tells us "some compaction work is needed," and `rocksdb.num-running-compactions`, which tells us how many background compactions are executing. `rocksdb.compaction-pending` is just a boolean value so we can't infer too much from it. `rocksdb.num-unscheduled-compactions` can be used to indicate that we may need to increase the max number of background jobs. Reviewed By: xingbowang Differential Revision: D114539448 fbshipit-source-id: 3657951bd9d8a4f4a7bc884513f9a06a87892b50 |
||
|
Peter Dillinger
|
8e1569b351 |
Lazy wide-column byte-range (partial) blob reads (#15067)
Summary: Builds on the lazy wide-column read API (DB::GetEntityLazy / DB::MultiGetEntityLazy) to serve true byte-range reads: resolving a sub-range of an uncompressed blob column now reads only the requested bytes from storage instead of fetching and slicing the whole column. This covers blobs in separate blob files and embedded (same-file) blobs in the SST, so a caller that knows a large value's layout can pull just the parts it needs. A partial read skips whole-record checksum verification (a strict sub-range cannot cover the checksum) and does not populate the blob cache (the cache entry holds the whole record). A compressed column, a whole-column read, a cache hit, or a read with LazyColumnReadRequest::force_verify still resolves (and verifies) the whole column; force_verify forces verification even when ReadOptions::verify_checksums is off. Mechanics: new range primitives mirror the whole-value read paths -- BlobFileReader::GetBlobRange, BlobSource::GetBlobRange, Version::GetBlobRange, and VersionBlobFetcherBase::FetchBlobRange for separate-file blobs; ReadSimpleGen2BlobRange, BlobSource::GetSimpleGen2BlobRange, and SameFileBlobReader::GetSameFileBlobRange (implemented by BlockBasedTable) for embedded blobs. ReadPathBlobResolver::ResolveColumnRange owns the partial-vs-whole decision and routes same-file references through the current SST's reader; the whole-column path also gained VersionBlobFetcherBase::FetchBlobForceVerify to honor force_verify. Observability: deferred resolve reads are attributed to a new Env::IOActivity::kLazyResolve (distinct from the kGetEntity / kMultiGetEntity of the initial entity read), and lazy resolution's storage reads are counted by new statistics rocksdb.blobdb.lazy.read.count / .read.bytes (all lazy reads) and rocksdb.blobdb.lazy.partial.read.count / .partial.bytes.saved (the byte-range subset and the bytes it avoided). The whole-column read sites gate recording on kLazyResolve so non-lazy reads are excluded. Also fixes a latent bug in LazyWideColumnsBatch::MultiResolve where an empty/default-constructed batch (null rep_) would accept a foreign standalone column (null owning_batch_rep_) through a nullptr == nullptr comparison instead of returning InvalidArgument. Follow-ups (in a local plan): unifying the blob-read API surface -- folding the whole/range method twins and the verify-policy flags (verify_checksums x force_verify) into one internal BlobReadRequest primitive (a range descriptor plus a 3-valued verify policy), which retires FetchBlobForceVerify and the whole/range method pairs; this is deferred for feature velocity and is not public-API affecting (the public force_verify bool stays). Marked in code with TODO(lazy-blob-resolution-cleanup) in db/blob/blob_fetcher.h and db/wide/read_path_blob_resolver.cc. Also cross-key coalescing (Phase 3) and async execution (Phase 4). Pull Request resolved: https://github.com/facebook/rocksdb/pull/15067 Test Plan: Extended db/wide/db_lazy_entity_test.cc: separate-file and embedded partial reads return the exact sub-span and read only the requested bytes; a partial read does not fill the blob cache and a cache hit slices with no I/O; a compressed column resolves whole and slices; force_verify does a full verified read and is detected via a tampered record even with verify_checksums off; the lazy statistics distinguish whole vs partial reads and are not touched by non-lazy reads; deferred reads carry kLazyResolve (verified with a FileSystem wrapper); and a regression test for the empty-batch ownership check. Added an IOActivityToString case in env/env_test.cc and registered the new tickers in the statistics name map. Crash test: extended the db_stress lazy-vs-eager differential (db_stress_tool/db_stress_test_base.cc) to fuzz per-column byte ranges. Each resolved column now randomly picks a whole-column read or a partial (offset, length) sub-range -- occasionally with an offset past the end or a length beyond the remainder to exercise clamping -- for both GetEntityLazy and MultiGetEntityLazy; when an eager reference is available the resolved bytes are compared against the same clamped sub-range of it. No new flags (reuses --lazy_entity_read_one_in); force_verify fuzzing remains a follow-up. Reviewed By: xingbowang Differential Revision: D115130388 Pulled By: pdillinger fbshipit-source-id: 454368dea8fae069d9de649c2c51611469d3c809 |
||
Darkhan Imangaliyev
|
5fe225357a |
Extend parallel checkpointing and db_stress coverage for column-family-subset checkpoints (#15059)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15059 **What**: 1. Extends the parallel `CheckpointEngine` so it can also checkpoint a subset of column families. 2. Adds `db_stress` and `db_crashtest.py` coverage for subset checkpoints, over both the serial and the parallel API. **How**: - **Engine API.** `CreateCheckpointOptions` gains a `std::vector<ColumnFamilyHandle*> column_families` field (empty, the default, still means all column families). This goes on the existing per-operation options truct rather than adding a second pure virtual to `CheckpointEngineBase`: it avoids overload-resolution hazards with the defaulted `sequence_number_ptr`, and it is the pattern the engine API already uses. `CreateCheckpointOptions` has no users outside the engine API, so adding a field breaks nothing. - **Shared validation.** The handle checks that were inline in `CheckpointImpl::CreateCheckpoint` -- reject null handles, handles belonging to another DB, and handles for already-dropped families; coalesce ids; always add the default CF -- move to `static CheckpointImpl::ResolveIncludeColumnFamilyIds()`, now called by both APIs. Rejecting a foreign handle matters because `handle->GetID()` alone would silently coerce a foreign id onto whichever family in this DB happens to share it. Validation runs before any staging directory is created, so a rejected request leaves nothing behind. - **Almost no new plumbing.** `CheckpointImpl::CreateCheckpointImpl()` already accepted `include_cf_ids` and is shared by the serial and parallel movers, and the MANIFEST `kColumnFamilyDrop` rewrite runs after `mover->Finish()`, so it was already mover-agnostic. The engine change is just resolving the ids and forwarding them. - **db_stress.** New `subset_cf_checkpoint_one_in` knob (mirrors `parallel_checkpoint_one_in`): each checkpoint operation covers a random proper subset with probability 1/N. It is a no-op when the DB has only the default column family, and it is independent of `parallel_checkpoint_one_in`, so subset checkpoints get exercised through both the serial API and the engine. Subset selection includes each non-default family with probability 1/2 and then force-excludes one, so every subset checkpoint exercises the MANIFEST rewrite instead of degenerating into a whole-DB checkpoint; the default handle is passed in only half the time, covering the "default is always included whether or not you pass it" contract on both sides. - **db_stress validation.** `DB::ListColumnFamilies()` on the checkpoint must return exactly the requested set -- the direct check on the MANIFEST post-processing, catching both an excluded family that was not recorded as dropped and a requested family that went missing. The checkpoint is then opened read-only with exactly those descriptors, proving the reduced MANIFEST and reduced file set are self-consistent under the default `paranoid_checks`. The pre-existing per-key comparison against `SharedState::Exists()` still runs, restricted to the families actually present. - **Incompatibility.** `clear_column_family_one_in > 0` is rejected together with the new knob, the same way D113103899 did for `abort_and_resume_cf_compactions_one_in`: a subset checkpoint hands live handles from `column_families_` to the API and validates against `column_family_names_`, and neither is stable while families are dropped and recreated underneath. `db_crashtest.py` already pins `clear_column_family_one_in` to 0. - **Test cleanup.** The subset tests shared a lot of setup and verification, so `PopulateFourCfsForSubsetCheckpoint()`, `VerifyFourCfsIntact()` and `VerifyDefaultAndTwoSubsetCheckpoint()` were extracted into the test file's anonymous namespace and are reused by the pre-existing `CheckpointSubsetOfCF` as well as the four new engine tests. This extends `StressTest::TestCheckpoint` with a mode rather than adding a separate operation, which is how `parallel_checkpoint_one_in` was integrated. Reviewed By: rban1 Differential Revision: D114832267 fbshipit-source-id: f3f843819614477497cc1466910547922cafc51d |
||
|
Xingbo Wang
|
e8bbe08760 |
Make ARM nightly crash test swap setup idempotent (#15065)
Summary: - make the ARM nightly crash-test swap setup skip `/swapfile` creation when it is already active - keep the existing 4 GiB swap creation path for hosts without active `/swapfile` - print the active swap devices after setup for easier workflow diagnostics ## Testing - `git diff --check upstream/main...2026_08_06_arm_failure` Pull Request resolved: https://github.com/facebook/rocksdb/pull/15065 Reviewed By: archang19 Differential Revision: D115045941 Pulled By: xingbowang fbshipit-source-id: ff2248b8501c82f5f9e7561bd77937d4823bc67f |
||
|
Peter Dillinger
|
fd7399004f |
Add lazy wide-column read API (GetEntityLazy / MultiGetEntityLazy) (#15012)
Summary:
Adds an experimental **lazy wide-column read API** that lets callers enumerate a
wide-column entity and read back only the columns (and, in a later phase, the
byte ranges) they actually need, instead of eagerly materializing every
blob-backed column like `GetEntity()` / `MultiGetEntity()`.
New public entry points (`include/rocksdb/db.h`):
- `DB::GetEntityLazy(read_options, cf, key, LazyWideColumns* result)`
- `DB::MultiGetEntityLazy(read_options, cf, num_keys, keys, LazyWideColumnsBatch* result, statuses, sorted_input)`
Both leave a wide-column entity's **inline** columns materialized up front but
its **blob-backed** columns as *unresolved references*, so blobs for columns
that are never pulled are never read.
### Result types (`include/rocksdb/lazy_wide_columns.h`)
- **`LazyWideColumn`** — the lazy analogue of `WideColumn`: `name()`, `index()`,
`inline_value()` (an `OptSlice`; empty ⇒ unresolved reference),
`is_reference()`, and `logical_size()` (`std::optional<uint64_t>`; known for
inline and uncompressed references, empty for compressed ones). All no-I/O.
- **`LazyWideColumns`** — a move-only result that is **indexable and iterable
like a vector** of `LazyWideColumn` (`size()`, `operator[]`, range `begin/end`).
Unlike `PinnableWideColumns`, it may be used after the producing call returns:
it **pins the SuperVersion** (like an iterator) so referenced blob files / SST
readers stay valid for deferred reads.
- **`LazyWideColumnsBatch`** — the `MultiGetEntityLazy` result; vector-like over
its per-key `LazyWideColumns`.
### Resolution
- **`LazyColumnReadRequest { const LazyWideColumn* column; uint64_t offset;
size_t length; bool force_verify; PinnableSlice* result; Status* status; }`** —
a read names a **typed column pointer**, not an untyped index. Because a
column identifies its owning result (and, for a batch, its owning entity),
`MultiResolve` routes each read correctly and **rejects a null or foreign
column** with `InvalidArgument` on that read's per-read status — cross-result
misuse can't be silently mis-indexed.
- **`LazyWideColumns::MultiResolve` / `LazyWideColumnsBatch::MultiResolve`** take
the same request type (pointer+count and `std::vector` sugar overloads); the
batch form resolves reads spanning any entities of the batch in one call.
- **`ResolveColumn(column, result)` / `ResolveColumnRange(column, offset, length,
result)`** — per-column sugar over a one-entry `MultiResolve`.
- A column's blob is fetched **at most once** over the result's lifetime (bytes
are cached in the result), so repeated reads of the same column reuse that
fetch. Resolution honors the `ReadOptions` captured at `GetEntityLazy()` time
(e.g. `read_tier`, `verify_checksums`); the resolve methods intentionally take
no per-resolve `ReadOptions`.
### Where it works
- Primary, **read-only**, and **secondary** DB instances (each `GetImpl`
override forwards the lazy signal and transfers a SuperVersion pin via the
shared `DBImpl::TransferSuperVersionPin` helper).
- Forwarded through `StackableDB` wrappers (e.g. `TransactionDB`) rather than
falling through to the `NotSupported` base default.
- Requires `max_open_files == -1` (immortal table readers) so same-file
("embedded") references stay resolvable lazily; otherwise the call returns
`InvalidArgument`.
## Implementation notes
- Read path: `GetContext` gains a lazy mode that leaves blob references
unresolved and captures the SST's `SameFileBlobReader`; `Version::Get` forwards
the new (defaulted) `lazy_columns_same_file_reader` out-param.
- `ReadPathBlobResolver` performs on-demand, cached blob resolution bound to the
pinned `Version` + decoded blob references.
- Internal identity is by heap-stable `Rep` pointers, so column/result/batch
pointers stay valid across moves with no move-time fix-ups.
## Scope / current limitations
- Resolves **whole columns**; byte ranges are served by slicing the whole column
for now (so `length`/`offset`/`force_verify` don't yet save I/O or skip
checksums). Single column family. Synchronous. `kBlockCacheTier` on the
originating call yields `Incomplete` on a blob-cache miss.
- User-defined timestamps: entities aren't writable under UDT, so `GetEntityLazy`
is at parity with `GetEntity` (same read-timestamp validation; plain values
return a single default column).
- Planned follow-ups: partial (byte-range) blob reads that skip checksum/cache-
fill, cross-key coalescing, and asynchronous execution.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/15012
Test Plan:
- **`db/wide/db_lazy_entity_test.cc` (15 tests, all passing):** enumeration with
no blob I/O; single range and whole-column resolution; `MultiResolve` of many
blobs and mixed ranges/columns (incl. the `std::vector` overload); cross-key
batch resolution; result-outlives-call / pin survives move; unpulled columns
never read; `max_open_files == -1` requirement; `kBlockCacheTier` ⇒
`Incomplete`; foreign/null column ⇒ `InvalidArgument` (single-entity and
batch); UDT parity with `GetEntity`; `StackableDB` forwarding; and read-only
and secondary instances.
- **db_stress:** `GetEntityLazy` / `MultiGetEntityLazy` run as a differential
against eager `GetEntity` / `MultiGetEntity` (gated by `--lazy_entity_read_one_in`,
which only takes effect under `--open_files=-1`), including coverage on
read-only (`--open_read_only_one_in`) and secondary (`--test_secondary`)
instances. Smoke-run (`EXIT=0`, "Verification successful") with integrated blob
files + `--use_put_entity_one_in`.
- `make format-auto` clean; builds `ASSERT_STATUS_CHECKED`-clean.
Reviewed By: xingbowang
Differential Revision: D113792365
Pulled By: pdillinger
fbshipit-source-id: cb73fc845fe0e20ed352d697e552e7b47b2eb9cf
|
||
|
Josh Kang
|
d1fbdffdae |
Log failed Status values in db_stress assertions (#15054)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15054 `db_stress` often terminates on raw `assert(status.ok())` checks, which discard the underlying error and make crash-test failures difficult to diagnose. Log the expression and full status before terminating. A message variant preserves operation-specific context and variadic formatting. In debug builds, assert after logging to retain assertion diagnostics and stack traces. In optimized builds, where assertions are disabled, exit with status 1. Reviewed By: jaykorean Differential Revision: D114756034 fbshipit-source-id: 5a0d4409bac69820b6560c8200cc401b7bfc65d6 |
||
|
Josh Kang
|
dc4006480b |
Tolerate InvalidArgument from full_history_ts_low race when persist_user_defined_timestamps=false (#15052)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15052 When `persist_user_defined_timestamps=false`, `full_history_ts_low` is automatically advanced by flushes to just above the newest timestamp in the flushed memtable. This creates the following race: ``` Reader Concurrent writer / flush ------ ------------------------- T0: Sample read timestamp: 892779959679761 T1: Write with a newer timestamp: 892779963289806 T2: Flush the memtable. T3: Advance full_history_ts_low: 892779963289807 T4: Validate the read: 892779959679761 < 892779963289807 Result: InvalidArgument ``` This is expected RocksDB behavior because history below `full_history_ts_low` is no longer guaranteed to be complete. However, the stress test intends these operations to read the latest value. This change adds ten seconds of headroom to read timestamps when user-defined timestamps are not persisted. This makes it substantially less likely that a concurrent flush overtakes a read timestamp. Unlike tolerating every `InvalidArgument`, this preserves reporting for genuine timestamp errors. Persisted read timestamps and write timestamps remain unchanged. The triggering non-default options are `persist_user_defined_timestamps=false` and `user_timestamp_size=8`. Reviewed By: xingbowang Differential Revision: D114565978 fbshipit-source-id: 6939df9163fd97fefec2fa185bc2ca72231ab04f |
||
|
Peter Dillinger
|
af0eae4de3 |
Rename TransactionLogIterator to WalIterator and document its behavior (#15056)
Summary:
`TransactionLogIterator` iterates the write-ahead log. It does not return transactions, does not require `TransactionDB`, and in fact returns `Status::NotSupported()` for two of the three `TransactionDB` write policies (WritePrepared and WriteUnprepared). The name predates `TransactionDB` and comes from "transaction log" being an old synonym for WAL. Everything else in this header was already renamed away from that terminology -- `LogFile` -> `WalFile`, `VectorLogPtr` -> `VectorWalPtr` -- and the C API has long used `rocksdb_wal_iterator_t` and `rocksdb_wal_iter_*`. This finishes the job.
The public class is now `WalIterator` in `include/rocksdb/wal_iterator.h`. `include/rocksdb/transaction_log.h` remains as a forwarding header and `TransactionLogIterator` remains as an alias, both marked deprecated by comment only (not `[[deprecated]]`) so existing users compiling with -Werror are unaffected, matching how `LogFile` and `VectorLogPtr` were retired. `DB::GetUpdatesSince()` keeps its name.
More importantly, the documentation was incomplete in ways that could cost a user data, so this states plainly what the API has always done:
- It only sees the WAL. Writes made with `WriteOptions::disableWAL`, and sequence numbers consumed by `IngestExternalFile()`, advance the DB sequence number without producing a WAL record. They leave permanent holes that end iteration. This was not documented anywhere, so it was unreasonable to expect a user to know those features silently break WAL tailing.
- The three iterator states, in particular that `!Valid()` with an OK `status()` means caught up rather than finished, and that calling `Next()` again is the intended way to tail a DB. The old comment on `Next()` said "REQUIRES: Valid() to be true", which forbids the primary use case.
- `Status::TryAgain`, which a caller must handle but which appeared nowhere in the public headers.
- That `GetUpdatesSince(seq)` silently starts later when `seq` is unavailable. Combined with the above this is the sharp edge: recovering from a spent iterator means calling `GetUpdatesSince()` again, so the recovery step is exactly where data can be skipped unnoticed. A consumer applying these batches to a copy of the DB diverges permanently and undetectably if it misses one, so the header now says such consumers must verify continuity themselves using `BatchResult::sequence` and `WriteBatch::Count()`.
Almost all of this diff is mechanical: renaming the type and its files, updating `#include` paths, rewriting comments, and updating build lists (`src.mk`, `CMakeLists.txt`, `BUCK`, `java/Makefile`, `java/CMakeLists.txt`). `db/c.cc` is regenerated from `tools/c_api_gen/c_base.cc`. Reviewers should focus on the four places that are not:
1. `WalIteratorImpl::UpdateCurrentWriteBatch()` in `db/wal_iterator_impl.cc` (renamed from `db/transaction_log_impl.cc`) drops dead code. On a sequence discontinuity it set `current_status_` to `NotFound` and then called `SeekToStartSequence()`, which returns immediately when `current_status_` is not OK. That guard and the early return in `Next()` were both added by
|
||
Omkar Gawde
|
dbf9328c71 |
Add per-column-family compaction abort and resume (#14982)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14982 **What**: Add reference-counted `DB::AbortCompactions(cf)` / `DB::ResumeCompactions(cf)` to abort and resume compactions for a single column family without stopping unrelated column families. They compose with the DB-wide `AbortAllCompactions()` / `ResumeAllCompactions()` and are no-ops once DB shutdown has begun. **How**: - **Per-CF abort flag.** `ColumnFamilyData` gains `compaction_aborted_`, wrapped in `CacheAlignedWrapper` so the lock-free reads done by compaction workers do not false-share with the DB-mutex-written fields packed around it. A new `IsCompactionAborted(cfd)` helper checks it alongside the DB-wide counter at every entry point (`CompactRange()`, `RunManualCompaction()`, `CompactFilesImpl()`), and `CompactionJob::ProcessKeyValue()` checks both on entry and in its periodic cron check. - **Parking.** Automatic work for an aborted CF is preserved rather than dropped. `PickCompactionFromQueue()` now returns `std::variant<CompactionQueuePicked, CompactionQueueThrottled, CompactionQueueParked>` and moves an aborted CF into `parked_compaction_cfds_` only once it reaches the front of the queue, so CFs behind it keep their place and the scheduler credit is not restored (which would cause a hot re-pick loop). `EnqueuePendingCompaction()` parks newly needed work for an aborted CF and now early-returns for dropped CFs. `RestoreParkedCompaction()` re-enqueues on the final resume; a CF dropped while parked is handled in `DropColumnFamily()`, and one dropped before its aborted queue entry is reached is handled by a fast path in `PickCompactionFromQueue()`. - **Knowing when it is safe to return.** `ColumnFamilyData::num_in_flight_compactions_` counts actually-picked work, and `Defer` balances `BeginInFlightCompaction()` / `EndInFlightCompaction()` on every `BackgroundCompaction()` and `CompactFiles()` exit path. That covers the `NotifyOnCompactionCompleted()` window that `CompactionPicker::compactions_in_progress_` does not, since the picker set is cleared during `Install`'s manifest write. `MustWaitForCompaction()` combines the counter with the picker set (filtered to skip `kExternalSstIngestion` / `kRefitLevel` reservations, which are not real jobs) because only the picker set covers the LOW-to-BOTTOM priority handoff, where the counter briefly dips to 0. - **Waking the waiter.** `AbortCompactions()` cancels matching `manual_compaction_dequeue_` entries and then waits on `bg_cv_`; `per_cf_compaction_abort_waiters_` forces the compaction completion paths to signal, because the picker's `UnregisterCompaction()` does not. `CloseHelper()` drains `parked_compaction_cfds_` and releases their references. - **API surface.** The two methods are new pure virtuals on `DB`, so `StackableDB` forwards them and the `db_test.cc` DB wrapper stubs them. `db.h` documents how the per-CF API composes with DB-wide abort, `DisableManualCompaction()`, `PauseBackgroundWork()`, shutdown, ingestion reservations, level refit and remote compaction, plus the parked-work caveat on `WaitForCompact()`. Comments in `listener.h`, `statistics.h` and `status.h` now name both abort APIs. - **Coverage.** 10 new cases in `db_compaction_abort_test.cc` (CF scoping, counter composition, parked-work restore, CF drop both while parked and before parking, trivial-move rejection, waiting for the completion callback, post-close no-op). `db_stress` gains `abort_and_resume_cf_compactions_one_in` and `TestAbortAndResumeCfCompactions()`, rejected in combination with `clear_column_family_one_in` because that would drop the column family out from under an active abort. Also a `db_crashtest.py` knob and one `new_features` release note. - **Known gap.** A `TODO` records the pre-existing post-pick stale compaction-score problem: aborting after `PickCompaction()` releases the inputs without recomputing the score, so the resume can observe a stale sub-trigger score and re-enqueue nothing. `ResumeAllCompactions()` has the same gap and carries a pointer to the same TODO; fixing it is left to a separate change covering both APIs. **Why**: Atomic-replace bulk loads only conflict with their target column family, but the DB-wide abort suppresses compaction for every column family colocated in the same RocksDB instance. Scoping the abort keeps unrelated column families compacting for the duration of a bulk load. Reviewed By: pdillinger Differential Revision: D113103899 fbshipit-source-id: 9bef924132676ed118a37aee86da6ee8bcefef09 |
||
zaidoon
|
751bde0f7f |
Add IndexFactory compatibility names (#14954)
Summary: Part 1 of 13 in the UDI split. Stack order: 1. https://github.com/facebook/rocksdb/issues/14954 Add IndexFactory compatibility names 2. https://github.com/facebook/rocksdb/issues/14959 Add UDI index mode API vocabulary 3. https://github.com/facebook/rocksdb/issues/14960 Add optional UDI builder protocols 4. https://github.com/facebook/rocksdb/issues/14961 Add built-in index factory wrappers 5. https://github.com/facebook/rocksdb/issues/14969 Add built-in index factory tests 6. https://github.com/facebook/rocksdb/issues/14962 Promote IndexFactory as UDI SPI 7. https://github.com/facebook/rocksdb/issues/14963 Wire IndexFactory through block-based tables 8. https://github.com/facebook/rocksdb/issues/14964 Add IndexFactory table routing tests 9. https://github.com/facebook/rocksdb/issues/14970 Add IndexFactory parallel routing tests 10. https://github.com/facebook/rocksdb/issues/14965 Account UDI blocks as index blocks 11. https://github.com/facebook/rocksdb/issues/14966 Support parallel AddIndexEntry in trie index 12. https://github.com/facebook/rocksdb/issues/14967 Cover trie index modes 13. https://github.com/facebook/rocksdb/issues/14968 Add IndexFactory stress and benchmark flags Previous: none. Next: https://github.com/facebook/rocksdb/issues/14959. This PR adds `include/rocksdb/index_factory.h` as a source-compatible set of new names over the current experimental UDI API: - `IndexFactoryBuilder` -> `UserDefinedIndexBuilder` - `IndexFactoryIterator` -> `UserDefinedIndexIterator` - `IndexFactoryReader` -> `UserDefinedIndexReader` - `IndexFactoryOptions` -> `UserDefinedIndexOption` - `IndexFactory` -> `UserDefinedIndexFactory` - `kIndexFactoryMetaPrefix` -> `kUserDefinedIndexPrefix` This intentionally does not change table building, table reading, SST format, OPTIONS parsing, or the C API. Existing `UserDefinedIndexFactory` subclasses that only implement the old `NewBuilder()` and `NewReader(Slice&)` shape still compile and work through the new `IndexFactory` name. Validation: - `build_tools/rockstest.sh table_test --gtest_filter='IndexFactoryCompatibilityTest.*'` - `make check-sources` - GitHub Actions PR matrix passed on the current head Pull Request resolved: https://github.com/facebook/rocksdb/pull/14954 Reviewed By: jaykorean Differential Revision: D114358164 Pulled By: xingbowang fbshipit-source-id: 5c830dc1f9dfcf11373423a03103a595fd8ad921 |
||
|
Josh Kang
|
14ecf7f13c |
Route single-blob reads through BlobFetcher (#15046)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15046 `BlobFetcher` is the policy boundary for single-blob reads, but `Version::Get()` and primary direct-write postprocessing still called lower-level backends directly. This duplicated backend selection and could bypass future fetcher policy or decorators. Route those paths through their existing `VersionBlobFetcher` and remove the redundant Slice-based `Version::GetBlob()` overload. This is behavior-preserving: each fetcher delegates to the same Version or direct-write backend with the same `ReadOptions`, cache, and lifetime context. Reviewed By: xingbowang Differential Revision: D114616978 fbshipit-source-id: 0d94cc389138f27791de636e2cca585e51cb142a |
||
Ranjan Banerjee
|
b1b50fe102 |
Parallel checkpointing stress test (#15037)
Summary: Adds db_stress / crash-test coverage for the parallel `CheckpointEngine` so it is exercised alongside existing features (atomic flush, sync fault injection, cf_consistency, backups). - Opens one shared `CheckpointEngine` per db_stress process, configured by `--checkpoint_engine_max_background_operations` and `--checkpoint_engine_use_link_file_when_available`. - `--parallel_checkpoint_one_in=N`: each checkpoint independently uses the parallel engine with probability 1/N, otherwise the legacy serial `Checkpoint` API, so serial and parallel paths run concurrently across threads. - `db_crashtest.py` randomizes all three per run. This coverage surfaced the missing destination fsync in the parallel copy path fixed by the parent diff. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15037 Reviewed By: xingbowang Differential Revision: D114414578 Pulled By: rban1 fbshipit-source-id: 48e7920a62b9b663a1c6054fc60b5765667bcbe5 |
||
|
Ranjan Banerjee
|
cda4a73a19 |
Fsync destination files in parallel CheckpointEngine copies (#15038)
Summary: Parallel checkpoint copies were never fsynced. `CopyEngine::CopyOrCreateFile` gated the destination sync on `use_fsync` (wired to `db_options.use_fsync`) -- but `use_fsync` means "fsync vs fdatasync", not "whether to sync". With the default `use_fsync=false`, the parallel path skipped the sync, so a copied MANIFEST/SST stayed in the unsynced buffer and was dropped by `TestFSWritableFile::Close` under `sync_fault_injection`, leaving a 0-byte file and checkpoint corruption: Corruption: no log_file_number, next_file_number, last_sequence entry in MANIFEST Corruption: Sst file size mismatch between expected N and file system 0 Serial `CopyFile` always syncs (using `use_fsync` only to pick the flavor), so it never hit this. Fix: split `WorkItem` / `CopyOrCreateFile` into `sync` (whether to sync) and `use_fsync` (fsync vs fdatasync). CheckpointEngine now passes `sync=true, use_fsync=db_options.use_fsync`, matching serial. BackupEngine is unchanged -- `sync=options_.sync` with the new `use_fsync=false` default reproduces the prior behavior. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15038 Reviewed By: xingbowang Differential Revision: D114414620 Pulled By: rban1 fbshipit-source-id: e0f1f3dbadcd48c5a5c2a11111a266dbed623f86 |
||
|
Josh Kang
|
00e73b99a2 |
Centralize db_stress validation errors (#15034)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15034 `db_stress` startup validation had accumulated multiple error-reporting paths, including direct `fprintf`/`exit(1)` pairs. This made invalid configurations inconsistent and bypassed normal unwinding. Centralize validation failures through `ReturnValidationError`, including diagnostics with runtime values, while leaving operational I/O failures on their existing paths. The variadic stream interface keeps the existing messages without `printf` casts. Reviewed By: jaykorean Differential Revision: D114384751 fbshipit-source-id: f396f8e7d099ef87c72aaf19455b777f2125f4df |
||
|
Xingbo Wang
|
460aea1523 |
Fix C API compatibility baseline selection in CI (#15039)
Summary: The C API compatibility job always fetched `main` as its baseline. On release branches, that can report false API removals for symbols that were added to `main` after the release branch was created. Select the compatibility baseline from the triggering event instead: - Pull requests use the merge target commit. - Pushes use the branch's pre-push commit. - Newly created branches or tags use `HEAD`, since they have no prior ref state to preserve. Fetch the selected commit by SHA, fail explicitly for unsupported events or missing event data, and pass the resolved baseline to `check-c-api-gen`. Also update the Makefile documentation to describe the PR and push behavior. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15039 Test Plan: CI Reviewed By: pdillinger Differential Revision: D114437175 Pulled By: xingbowang fbshipit-source-id: ee66f54bcabba238477d07a69ec7dcb42219c088 |
||
|
zaidoon
|
4b35e9966c |
Add batch-owned pinned MultiGet C API (#14988)
Summary: - add `rocksdb_batched_multi_get_pinned_cf` for batched reads using `rocksdb_slice_t` keys - return one `rocksdb_pinnable_multi_get_t` owner for all values and per-key errors - preserve input ordering, duplicates, misses, and sorted-input behavior - expose indexed borrowed results plus count and destroy functions ## Why `rocksdb_batched_multi_get_cf_slice` returns one separately allocated `rocksdb_pinnableslice_t` wrapper for every found key. Language bindings that want to retain a full batch have to manage and destroy each wrapper independently. The new API keeps successful `PinnableSlice` values in one owner. It stores only compact value and error payloads, while a result index maps them back to the original key order. Found values and error messages remain borrowed until the batch is destroyed. The batch must be destroyed before closing the DB that created it because pinned values can retain DB-owned cache state. ## Compatibility This is additive. Existing MultiGet APIs and ownership rules are unchanged. ## Testing `c_test` covers: - found and missing keys - duplicate keys and input ordering - empty values - per-key merge errors - sorted and unsorted input - empty batches - out-of-bounds access - borrowed value and error lifetimes Validation run locally: - `make -j8 check-c-api-c_test` - `CXX=clang++ make check-c-api-gen CHECK_C_API_GEN_STRICT=1 CLANG_FORMAT_BINARY=clang-format` - C API link-completeness and backward-compatibility checks This API is intended to replace a temporary compatibility extension in `rust-rocksdb` once it is available in a RocksDB release. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14988 Reviewed By: jaykorean Differential Revision: D114357453 Pulled By: xingbowang fbshipit-source-id: a35785f7bb4d097b391932e2b9f96fc177d1ba1c |
||
|
Peter Dillinger
|
b7b6d7c8c0 |
Mirror CI third-party build deps on a GitHub release instead of S3 (#15041)
Summary: CI/build downloaded pinned third-party binaries (compression source tarballs, Java test jars, and clang-format-diff.py) from an S3 bucket. Move the mirror to assets on an unlisted "third-party-deps" GitHub release, so the files live alongside the code with no separate bucket to maintain and no per-repo bandwidth quota (unlike Git LFS). Recent releases on the RocksDB release page are not polluted (https://github.com/facebook/rocksdb/releases/, far down the list probably from tagging a very old release). The compression download variables are refactored from *_DOWNLOAD_BASE (a directory URL) to *_DOWNLOAD_PREFIX (directory + "<project>-", or "...archive/v"), so each release asset can carry a project-name prefix in the otherwise flat release namespace while the upstream defaults still resolve to byte-identical URLs. The Java DEPS_URL defaults (Makefile and CMake) and the clang-format-diff.py download are repointed at the release. A comment at each dependency site links to the release edit page for adding/updating assets. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15041 Test Plan: CI and... Downloaded all 11 assets from the release and confirmed their SHA256 sums match the files previously served from S3 (tarball/jar sums are also enforced by the existing Makefile checksum checks). Verified the refactored Makefile upstream defaults expand to the same URLs as before, and that the CI *_DOWNLOAD_PREFIX overrides plus DEPS_URL resolve to the uploaded asset names. Covered by the normal PR/nightly/weekly jobs that build compression from source and run the Java tests (Make and CMake). Reviewed By: xingbowang Differential Revision: D114443886 Pulled By: pdillinger fbshipit-source-id: 8c59435e7064f0d9931cba103c8120fe113b0d6d |
||
|
Peter Dillinger
|
afc922bf5f |
Make ReadOptions::table_filter a caller-owned pointer for cheaper copies (#15005)
Summary: ReadOptions is copied frequently on read paths, and its only non-trivially-copyable member, table_filter, is a std::function that is almost always empty yet still gets deep-copied on every ReadOptions copy (and can heap-allocate). This changes table_filter to a pointer to a caller-owned std::function instead of storing one by value: when non-nullptr, the caller is responsible for keeping the pointed-to function alive for the duration of the read (like the other pointer fields such as iterate_upper_bound). With this change every ReadOptions member is trivially copyable, so copying ReadOptions no longer touches a std::function at all. That trivial-copyability is locked in with a static_assert in options_test.cc (kept out of the public header). This is inspired by D113109635, which targeted the same ReadOptions-copy hotspot by relocating table_filter to the end of the struct so the trivial members memcpy in one contiguous block. This is intended to be a superior solution: rather than only defragmenting the copy, it removes the std::function copy entirely. This is a breaking API change, tolerated because table_filter is a relatively obscure option. A nullptr, or a pointer to an empty std::function, means no filtering (preserving the previous empty-function behavior). A shared internal helper HasTableFilter(const ReadOptions&) in options_helper.h centralizes the nullptr + empty-function check used on the read paths. The C API and Java bindings now own the std::function and hand ReadOptions a pointer to it; their public signatures are unchanged, and the C API rocksdb_readoptions_t clears the owned function when the callback is null. Making ReadOptions trivially copyable also let -Werror=unused-but-set-variable fire on a pre-existing dead local in db_bench's MultiScan: a ReadOptions that set async_io/readahead_size but was never passed to NewMultiScan (the warning had been suppressed while ReadOptions had a non-trivial member). That local was in fact inert for multiscan -- BlockBasedTableIterator::Prepare overrides ReadOptions::async_io with MultiScanArgs::use_async_io for the coalesced prefetch, so multiscan async I/O is driven by --multiscan_use_async_io, not ReadOptions::async_io. Removed the dead local; MultiScan now uses read_options_ directly, consistent with MultiScanRandom. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15005 Test Plan: Existing table_filter coverage was adapted to the new pointer API and still exercises the behavior end to end: db_iterator_test.TableFilter (callback invoked once per table; returning false hides a table) and its range-conversion rejection case; db_bloom_filter_test SstQueryFilter/FixedWidthSegments range queries; and trie_index_db_test's range-query iterator (which returns iterators outliving the filter, exercising the caller-owned lifetime). db/c_test covers the C API set/clear/has_table_filter path. options_test asserts std::is_trivially_copyable_v<ReadOptions>. Manually verified the db_bench MultiScan async behavior with a ~200MB DB and --use_direct_reads: with ReadOptions::async_io held true, async reads tracked only --multiscan_use_async_io (rocksdb.read.async.micros count 329511 with use_async_io=true vs 0 with false), confirming ReadOptions::async_io does not drive multiscan async and the removed local had no effect. Reviewed By: anand1976 Differential Revision: D113613525 Pulled By: pdillinger fbshipit-source-id: 38d73b607500a5c7ca2b89dd8f4187ef43940aa4 |
||
anand1976
|
6e73763b8a |
Update version to 11.9.0 for 11.8 release (#15023)
Summary: - Bump version.h from 11.8.0 to 11.9.0 - Add `11.8.fb` to `check_format_compatible.sh` - Sync HISTORY.md `## 11.8.` section from `11.8.fb` - Delete 5 consumed `unreleased_history/` note file(s) from main Part of 11.8 release workflow. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15023 Reviewed By: xingbowang Differential Revision: D114304679 Pulled By: anand1976 fbshipit-source-id: 6fecd5c5175140dfa1f6ba16aec42748eee91172 |
||
|
Darkhan Imangaliyev
|
f9df618e4d |
Api to checkpoint only selected set of column families (#15019)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15019 RocksDB's Checkpoint API currently copies all column families. For use cases that only need a subset, this is wasteful. This diff adds a new `CreateCheckpoint(dir, column_families, ...)` overload that: - Validates input handles (rejects null/foreign handles, coalesces duplicates, always includes default CF). - Filters GetLiveFilesStorageInfo to capture only the requested CFs' table/blob files. - Post-processes the copied MANIFEST by appending kColumnFamilyDrop records for excluded CFs, so the checkpoint opens without referencing missing files. - An empty column_families vector falls through to the existing whole-DB path. Reviewed By: xingbowang, pdillinger Differential Revision: D113963616 fbshipit-source-id: d1becbbb9528dd13c2dc314165880b76123757f0 |
||
|
Peter Dillinger
|
026f63de6c |
Parallelize the non-unit-test checks in make check (#15028)
Summary:
`make check` ran a series of extra checks serially, one after another, after the GNU-parallel unit tests: the Python syntax check, ldb/db_crashtest/ rocksdb_dump tests, and the format/buck/sources/workflow/c-api-gen source checks. These had accumulated into a non-trivial single-threaded tail on every `make check`.
Each extra check is now a make target collected in a single CHECK_EXTRA_TARGETS list, and gen_check_scripts emits a `t/run-<target>` wrapper per enabled check. check_0 already runs every `t/run-*` as a gnu_parallel job, so the checks now run alongside the unit tests, hiding their cost behind the (dominant) test phase. The slow ones (check-c-api-gen, check-format) are front-loaded via slow_test_regexp, and under CI sharding the checks are distributed round-robin so each runs on exactly one shard. The J=1 / no-gnu-parallel fallback runs them serially. New check-all-python and rocksdb_dump_test targets wrap the two steps that were previously inlined; the guards match the old serial tail exactly.
Each wrapper frames its result with a gtest-style
`[ RUN ]` / `[ OK ]` / `[ FAILED ] >>> make <check> <<<` line so a non-unit-test failure is easy to distinguish and re-run.
The wrappers spawn sub-makes concurrently, and each make invocation otherwise re-runs build_detect_platform, which rewrites the shared make_config.mk non-atomically -- concurrently that corrupts it ("missing separator"). The wrappers pass a new ROCKSDB_RECURSIVE_MAKE_SAME_CONFIG=1, which reuses the parent's already-generated make_config.mk (guarded on it existing, so normal builds still detect) and also silences the parse-time banners for clean output.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/15028
Test Plan: `make check` runs the same set of auxiliary checks as before, now as parallel jobs. Verified that gen_check_scripts emits one wrapper per enabled check, that the SKIP_FORMAT_BUCK_CHECKS / ASSERT_STATUS_CHECKED / AIX guards still gate the set, that CI_TOTAL_SHARDS distributes the checks one-per-shard (union across shards is the full set), and that many concurrent wrappers no longer corrupt make_config.mk. A seeded check failure surfaces framed as `[ FAILED ] >>> make <check> <<<`.
Reviewed By: xingbowang
Differential Revision: D114245857
Pulled By: pdillinger
fbshipit-source-id: 519f78ccdcfbfdd88509980f78944e527a572331
|
||
Anand Ananthabhotla
|
e2ccb78cf2 |
Avoid deletefile test purge/flush deadlock (#15027)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15027 `BackgroundPurgeCFDropTest` blocks `DBImpl::BGWorkPurge:start` to verify dropped-CF files remain until background purge is released. Runtime OPTIONS-file cleanup can also schedule a purge on the HIGH pool when `avoid_unnecessary_blocking_io` is enabled, so installing the sync-point dependency before test setup can park that OPTIONS purge before the foreground `Flush()` has a chance to run. This does not need a broader product change. Runtime OPTIONS-file writes are rare, and the hard deadlock is a test artifact caused by the sync-point dependency. Outside the test, the observable impact would be limited to a foreground `Flush()` API call waiting longer if its flush is queued behind the OPTIONS-file purge. Scope the sync-point dependency to the behavior under test by installing it only for the background-purge case, after the setup flush has produced the SST and just before `DropColumnFamily()`. This keeps setup-side OPTIONS-file purge work outside the blocked window while still verifying that dropped-CF file deletion waits for background purge. Reviewed By: joshkang97 Differential Revision: D114241457 fbshipit-source-id: 21c5f4242a50590e161b3720077a9dfe5e77aa08 |
||
|
Josh Kang
|
c64f682946 |
Fix IoUringBackend crash in ReadIOExecutor worker threads (#15020)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15020 Each read executor worker creates its own `IoUringBackend`, and Folly's I/O thread pool creates its worker threads and EventBases lazily. A successful calling-thread probe or `DB::Open()` therefore does not validate every worker. A later read can start a new worker; previously, an `IoUringBackend` initialization exception on that worker could escape startup and terminate the process. The previous 512-submit setup also allowed Folly's capacity retry to reduce the completion queue to 512 while retaining a 1024-entry submission queue, which the kernel rejects with `EINVAL`. Use a 256-submit limit so the 512-entry retry remains valid. Configure the shared read executor during `DB::Open()` through `DBOptions::read_io_executor_threads`, defaulting to one thread. The backend factory catches per-worker io_uring initialization failures and uses Folly's default backend. Each worker's thread-local availability marker starts false and is set true only after `IoUringBackend` construction succeeds. When construction fails, the marker remains false and `SubmitReadAsync()` performs the read synchronously. The executor's maximum thread capacity can be raised or lowered; `db_stress` requests eight threads for async DB APIs. To expose this degradation, `FSRandomAccessFile::SubmitReadAsync()` now reports whether the asynchronous path was used. `RandomAccessFileReader` increments the `FILE_SUBMIT_ASYNC_READ_FALLBACK` ticker for every submission that falls back to synchronous I/O, making the fallback visible through the DB's `Statistics`. Reviewed By: xingbowang Differential Revision: D114031018 fbshipit-source-id: 1b7f6c1cea693e5dd743ea1ada0fca1cf757b3f9 |
||
|
Josh Kang
|
359d57be96 |
Fix blob file path misidentification in SstFileManager during compaction (#15021)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15021 The warm storage crash failure in T282626155 exposed a RocksDB bookkeeping bug, not a transient WS error. The regression was introduced by D91480994 / upstream RocksDB PR https://github.com/facebook/rocksdb/pull/14227 (“Support abort background compaction jobs”). That change routed `BlobFileBuilder` output tracking through `CompactionOutputs::output_file_paths_`, which contains both blob and SST paths, while blob completion still assumed `.back()` identified the current blob. Failure timeline: 1. Compaction job 225 opens `000455.blob`, producing `[000455.blob]`. 2. It opens `000456.sst`, producing `[000455.blob, 000456.sst]`. 3. Blob 455 finishes, but `OnBlobFileCompleted()` receives `.back()`, which is `000456.sst`. 4. `SstFileManager` stats the still-open SST. WS correctly returns non-retryable `NOT_FOUND` because the output is not pathname-visible until close. 5. Compaction fails before installing its outputs in the MANIFEST. Cleanup then deletes `000455.blob` and `000456.sst`; those deletions are consequences of the failure, not its cause. This change derives callback paths from the blob file number instead of the shared cleanup vector, renames the vector accordingly, and adds a deterministic regression test verifying that `SstFileManager` tracks every blob created during compaction. Reviewed By: pdillinger Differential Revision: D114002145 fbshipit-source-id: 472b563f3fb731753f286a40feece7428928bc20 |
||
generatedunixname1395027625275998
|
ba81144d8d |
Force-inline xxph3 mix helpers on the NPHash64/Hash64 hot path (#14981)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14981 Reviewed By: pdillinger Differential Revision: D112965953 fbshipit-source-id: 9645d9287a9aba987bf1b37752df7f9dcb3a9669 |
||
|
Anand Ananthabhotla
|
4b9070bccb |
Defer obsolete OPTIONS file deletion on open (#15004)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15004 This change reduces DB open latency on remote storage by avoiding foreground deletion of obsolete `OPTIONS-*` files during open when `avoid_unnecessary_blocking_io` is enabled. DB open now avoids foreground deletion of obsolete `OPTIONS-*` files on the pre-open `WriteOptionsFile()` path. `RenameTempFileToOptionsFile()` records the new OPTIONS file and chooses an explicit cleanup mode: skip while open is still in progress, schedule deletion when background work is available, or delete immediately when blocking IO is allowed or background jobs are rejected. `DBImpl::Open()` makes the deferred cleanup decision once final open status is known. A successful open schedules obsolete OPTIONS files on the purge queue, while a failed open deletes them synchronously before tearing down the partially opened DB. The default synchronous behavior remains unchanged when `avoid_unnecessary_blocking_io` is disabled, and the latest-two OPTIONS retention policy is preserved. Reviewed By: pdillinger Differential Revision: D113593680 fbshipit-source-id: 8aee99b3f38a729647d0bcb644d212181fe86895 |
||
|
Josh Kang
|
7247edbf0a |
Add opt-in stats for async RocksDB reads (#15017)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15017 ## Summary RocksDB async `GetAsync` and `MultiGetAsync` callbacks now default to a completion-only path. Callbacks can override `EnableStats()` when they need request-specific `PerfContext` or `IOStatsContext`; otherwise RocksDB avoids creating a coroutine stats context and returns null stats pointers. Callbacks must not initiate another async read from `OnComplete()`. RocksDB stats normally use thread-local storage, which does not compose with interleaved coroutine reads. A stats-enabled request captures only the caller's configuration: perf level, per-level perf enablement, and whether io stats are disabled. It then starts with empty request contexts. `onSet()` loads that request's contexts into TLS, while `onUnset()` saves the accumulated contexts back into request data. On resume, the same contexts are loaded again. This isolates request counters without preserving or restoring pre-existing executor TLS state. The synchronous fallback similarly resets TLS before executing a stats-enabled read. `PerfContext` and `IOStatsContext` gain `Merge()` support for caller-side aggregation. `PerfContext` move construction and assignment now transfer ownership of per-level state rather than deep-copying it, avoiding allocations during coroutine context handoffs. A fbcode-wide audit found no caller that reads a moved-from `PerfContext`. `db_bench` opts in only for coroutine jobs that print per-job perf context and merges every completed request's callback stats into the job aggregate. ## Example: Two Async Gets on One Thread Both callbacks enable stats. Each request starts with empty counters and carries its contexts across suspensions. ```text Shared executor thread | | onSet(A): load A's empty contexts into TLS | A records 3 reads | onUnset(A): save TLS contexts back into A | | onSet(B): load B's empty contexts into TLS | B records 4 reads | onUnset(B): save TLS contexts back into B | | onSet(A): load A's contexts containing 3 reads | A records 2 more reads | callback A receives block_read_count = 5 | | onSet(B): load B's contexts containing 4 reads | callback B receives block_read_count = 4 ``` Isolation comes from replacing TLS with the active request's saved contexts on every `onSet()`. ### Timers Existing wall-clock timers remain in coroutine frames, so timers spanning `co_await` include suspended time and measure end-to-end latency. Request-level `get_cpu_nanos` is suspension-aware: it starts in `onSet()` and stops in `onUnset()` or `Finalize()`, charging only time while the request is active. Nested CPU timers remain disabled because normal RAII CPU timers would also charge executor work performed while their coroutine is suspended. `MultiGetAsync` fanout additionally requires per-child timer state to avoid double counting. ## Benchmarks Benchmarked to confirm no performance regressions when perf is disabled, and also to see the overhead of enabling perf. ```bash db_bench --benchmarks=<readrandomcoroutine|multireadrandomcoroutine> --db=<db_path> --coro_jobs_per_thread=10 --use_existing_db=true --num=2000000 --value_size=1024 --compression_type=none --threads=8 --batch_size=16 --multiread_batched=true --use_direct_reads=true --cache_size=1428076626 --reads=320000 --perf_level=<1|3> --seed=<seed> ``` | Workload | Before change | Perf level 1 | Before -> 1 | Perf level 3 | 1 -> 3 | | -------- | ------------- | ------------ | ----------- | ------------ | ------ | | Async Get | 475.6K ops/s | 476.7K ops/s | +0.2% | 470.5K ops/s | -1.3% | | Async MultiGet | 567.3K ops/s | 567.4K ops/s | +0.0% | 552.2K ops/s | -2.7% | Reviewed By: xingbowang Differential Revision: D112634874 fbshipit-source-id: 94e240aeb1496fa12dac995b5aa5a9c3543cc5fc |
||
|
Ranjan Banerjee
|
f5c0c3e274 |
Record CheckpointEngine copy bytes under checkpoint tickers (#14997)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14997 `CheckpointEngine`'s shared `CopyEngine` recorded copied bytes under the `BACKUP_READ_BYTES` / `BACKUP_WRITE_BYTES` tickers (the `CopyEngineOptions` defaults), conflating checkpoint I/O with backup I/O in statistics. This adds dedicated `CHECKPOINT_READ_BYTES` / `CHECKPOINT_WRITE_BYTES` tickers and points `CheckpointEngineImpl`'s `CopyEngineOptions` at them, so checkpoint copies report under their own metric names. `BackupEngine` is unchanged (still uses the backup-ticker defaults). Addresses review feedback that the shared `CopyEngine` should not hardcode backup metric names for non-backup callers. The new tickers are intentionally not mirrored in the Java bindings (`TickerType.java` / `portal.h`): the signed-`jbyte` id space there is exhausted, so they remain C++-only, documented inline in `statistics.h`. Reviewed By: xingbowang Differential Revision: D113469742 fbshipit-source-id: 2a6a7ef04c939a1240f1c48c7bcaba8b2e7a1c10 |
||
|
Xingbo Wang
|
2fc58a678b |
Fix MultiScanPrepare fallback after io_uring init failure (#15016)
Summary: - Gate the `MultiScanPrepare` test's async I/O mode on the filesystem's reported `kAsyncIO` support. - Use the same gated async-I/O flag for `MultiScanArgs::use_async_io` and read-count expectations. - Match production DB-level MultiScan behavior so the test falls back consistently after io_uring initialization fails. ## Background - Recent Sandcastle failures in T265255062 all hit the MultiScanPrepare read-count assertion after PosixFileSystem::SupportedOps() logged "CreateIOUring failed: Cannot allocate memory (errno=12)" and returned without kAsyncIO support. - The test already probed SupportedOps(), but it only used that result when computing expected reads. It still copied the raw use_async_io test parameter into MultiScanArgs. That let the lower-level table-reader path call ReadAsync() and retry CreateIOUring() after the support probe had failed. If the retry succeeded, Prepare queued async reads and recorded zero sync reads while the assertion expected the sync fallback count. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15016 Test Plan: - `make format-auto` - `AUTO_CLEAN=1 make -j$(nproc) block_based_table_reader_test` - `timeout 60s ./block_based_table_reader_test --gtest_filter="*MultiScanPrepare*"` - `make check-sources` Reviewed By: archang19 Differential Revision: D113830233 Pulled By: xingbowang fbshipit-source-id: 38460f24fd44b33eb139a8f59bd87c637983e503 |
||
|
Josh Kang
|
a4645e864d |
Implement Get/MultiGet coroutine (#15014)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15014 Implements coroutine-backed execution behind the callback-based `DB::GetAsync` and `DB::MultiGetAsync` APIs that issue SST block reads asynchronously, so a single application thread can keep many point-lookup reads in flight at once instead of paying one blocking read per thread. The motivation is point-lookup throughput on disk-bound workloads (working set larger than the block cache), where the win comes from queue depth rather than per-read latency. The async capability rests on a small, generic FileSystem API that a `FileSystem` must implement to participate. Every method is virtual with a working synchronous default, so existing and custom `FileSystem` implementations compile and run unchanged (they simply stay synchronous): - `FSRandomAccessFile::SubmitReadAsync(req, opts, cb, dbg)` -- a fire-and-forget read: it starts the read and returns without blocking, then invokes `cb` once `req.result`/`req.status` are filled in (the callback may run on a different thread). The default implementation is synchronous -- a blocking `Read` followed immediately by `cb`. - `FileSystem::GetReadExecutor()` -- returns the `folly::Executor*` that async queries and read completions run on. The default returns `nullptr`, which the query layer treats as "async reads unavailable" and falls back to the synchronous path. It is exposed to users, so they may add additional application logic to run on the same thread pool, thus avoiding expensive context switches. - `FileSystem::SetReadIOExecutorThreads(int)` -- sizes that executor. The default is a no-op. `RandomAccessFileReader::Read`/`MultiRead` are `DEFINE_SYNC_AND_ASYNC` coroutines (new `file/random_access_file_reader_sync_and_async.h`), and `DBImpl::GetAsync`/`MultiGetAsync` bind the query to `GetReadExecutor()` with `co_withExecutor`. For a MultiRead, `SubmitMultiReadAsync` issues all of the block reads through `SubmitReadAsync` back-to-back and then `co_await`s a `folly::coro::Baton`: awaiting the baton suspends the query and releases the worker thread while the reads are in flight, and the callback for the last outstanding read posts the baton to resume it. A MultiGet's per-file `MultiGetFromSST` reads additionally fan out across the current eventbase so overlapping-level lookups run in parallel. This layer works against any executor and any `SubmitReadAsync` implementation. The Posix `FileSystem` is the concrete async implementation, and is where io_uring enters. `GetReadExecutor()` is a `folly::IOThreadPoolExecutor` whose workers are EventBases that each drive an io_uring ring (sized by `SetReadIOExecutorThreads`). `PosixRandomAccessFile::SubmitReadAsync` -- which must be invoked on one of those EventBase workers -- places the read on that worker's submission queue via `IoUringBackend::queueRead` and returns immediately: no blocking `pread`, no parked thread. When the kernel posts the completion the backend runs the callback on that same worker, and a short read re-queues its remainder from inside the completion callback, so even multi-syscall reads stay on the non-blocking path. The net effect is that one worker thread keeps many reads outstanding at once, bounded by io_uring queue depth rather than thread count. ## Thread-local state handling A coroutine suspends at every `co_await` and can resume on a different read-executor worker, and another coroutine can run on that worker in between. Any thread-local state held across a suspension point is therefore unsafe -- it can be observed or mutated by an interleaved coroutine, restored onto the wrong thread, or trip an in-use assertion. The read path handles each thread-local it touches explicitly, and introduces no new thread-local storage of its own: - SuperVersion cache: the thread-local SuperVersion slot is marked `kSVInUse` for the duration of a read, so a second coroutine resuming on the same worker would trip that assertion. The coroutine path instead takes an independent heap reference that releases the slot before any suspension -- `cfd->GetReferencedSuperVersion()` for single Get, and `kExtraSvRef = true` threaded through `MultiCFSnapshot` for MultiGet -- and releases it with `CleanupSuperVersion` (via a `Defer` for single Get). The synchronous path is unchanged and keeps using the thread-local slot (`GetAndRefSuperVersion` / `ReturnAndCleanupSuperVersion`). - perf_context / iostats_context / perf level: the timing guards that save-and-restore or accumulate into these thread-locals across an IO -- `GetPerfLevel`/`SetPerfLevel`, `IOSTATS_TIMER_GUARD(read_nanos)`, `IOSTATS_CPU_TIMER_GUARD(cpu_read_nanos)` -- are compiled out on the coroutine path (`#if defined(WITHOUT_COROUTINES)`), so none of that thread-local timing state spans a `co_await`. The tradeoff is that these perf/IO counters are not populated for coroutine reads (a known limitation of this path). - TLS-free helpers are left as-is: `StopWatch`, for instance, holds only clock/statistics pointers and a start time, so it is safe across a suspension (it can over-count wall time under oversubscription, which is a metrics-accuracy issue, not corruption). ## Benchmarks Benchmarked using 8 threads for both multiget (with/without async_io enabled), and single get on both direct-io enabled and disabled on a ~11 GB DB. Non direct-io benchmarks are not a good representation because my dev machine is capable of paging the entire DB into memory. But, it is still useful to compare against async_io=true, since that feature only benefits non direct-io. Jobs-per-thread (jpt) is the multiplier for the number of tasks being submitted to the thread pool. E.g. a threadpool of 8 threads with jpt = 4 has 32 running tasks on the thread pool. It is important to oversubscribe so that when a task suspends, another one can resume on the thread pool. MultiGet (batch 16): | IO | cache hit % | sync | async_io | coro jpt=1 | coro jpt=2 | coro jpt=5 | coro jpt=10 | | -------- | --- | ---- | -------- | ---------- | ---------- | ---------- | ----------- | | direct | 10% | 65K | 64K | 145K | 151K | 161K | 166K | | direct | 50% | 88K | 80K | 239K | 287K | 327K | 340K | | direct | 90% | 435K | 399K | 732K | 991K | 1,093K | 1,119K | | buffered | 10% | 449K | 561K | 555K | 593K | 572K | 566K | | buffered | 50% | 495K | 682K | 656K | 724K | 700K | 701K | | buffered | 90% | 600K | 878K | 828K | 1,021K | 1,048K | 1,060K | single Get: | IO | cache hit % | sync | coro jpt=1 | coro jpt=2 | coro jpt=5 | coro jpt=10 | | -------- | --- | ---- | ---------- | ---------- | ---------- | ----------- | | direct | 10% | 41K | 37K | 69K | 125K | 144K | | direct | 50% | 74K | 66K | 127K | 238K | 281K | | direct | 90% | 390K | 330K | 540K | 767K | 849K | | buffered | 10% | 403K | 291K | 369K | 447K | 447K | | buffered | 50% | 441K | 337K | 443K | 554K | 527K | | buffered | 90% | 544K | 433K | 606K | 753K | 810K | Example command (MultiGet coroutine, ~50% hit): ``` db_bench -benchmarks=multireadrandomcoroutine -coro_jobs_per_thread=10 -use_existing_db=true -num=2000000 -value_size=1024 -compression_type=none -threads=8 -batch_size=16 -use_direct_reads=true -cache_size=402653184 -reads=320000 -statistics ``` ### Coroutine vs context switch overhead Coroutines are not zero overhead, so we also compare it against the context switching of using many sync threads. Both variants were pinned to the same 8 CPUs and ran 80 concurrent MultiGet operations. | variant | throughput | | ---------------------------------------- | ------------ | | sync, 80 threads | 330.6K ops/s | | coroutine, 8 executor threads / 80 jobs | 446.0K ops/s | Reviewed By: xingbowang Differential Revision: D111116635 fbshipit-source-id: bd0ac052efe79261a552e9e66137e5e70f7cd8a1 |
||
|
Josh Kang
|
039c45075e |
Add db_stress coroutine read flag (#15013)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15013 Add `--use_async_db_api` to db_stress and route direct `DB::Get` and value `DB::MultiGet` reads through the callback async APIs (`DB::GetAsync` / `DB::MultiGetAsync`) when coroutine support is available. The db_stress helpers block on callback completion and notify waiters while holding the callback lock, so existing stress-test control flow and callback lifetime are preserved. Builds without coroutine support silently keep the existing synchronous path. Centralize the dispatch in db_stress helpers and use them for workload, verification, secondary, and consistency/debug reads while leaving transaction and entity APIs on their existing paths. Randomize `use_async_db_api` in `tools/db_crashtest.py` one run in six. Reviewed By: jaykorean Differential Revision: D111132477 fbshipit-source-id: 2aeac267867d8501131161c9fdbaf1da2b0c51d5 |
||
|
Xingbo Wang
|
fbd446cebb |
Disable open_read_only_one_in when mmap_read is enabled (#15011)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15011 Root cause: `open_read_only_one_in` can create a read-only DB instance on the same live directory as the primary DB. When the stress run also enables `mmap_read=1`, that read-only instance can hold mmap-backed slices into SST files while the primary continues compacting and deleting obsolete files. If `SstFileManager` uses chunked truncation (`sst_file_manager_bytes_per_truncate > 0`), Linux can raise SIGBUS when the read-only path later touches a page whose backing SST was truncated or removed. The range tombstone fragmenter/comparator is where the invalid mmap-backed key slice was touched, not necessarily the root cause. Evidence from T281873950 / Sandcastle job `63050395314610170`: the crashing command had `--mmap_read=1`, `--open_read_only_one_in=16`, and `--sst_file_manager_bytes_per_truncate=1048576`. The crash stack was `BlockBasedTable::ReadRangeDelBlock()` -> `FragmentedRangeTombstoneList::FragmentTombstones()` -> `std::set<ParsedInternalKey>::emplace()` -> comparator -> `memcmp`, ending in signal 7/SIGBUS. The crash DB LOG shows `allow_mmap_reads: 1`; table `104662.sst` was created cleanly at size `6257069` with range tombstones and was later deleted by the primary. The artifact does not show persistent SST corruption or checksum failure, and injected read faults would normally surface as RocksDB statuses rather than an OS SIGBUS. Why this started surfacing: D111995911 / `rFBS0f7a2ef93e63cfa6677b6fa32c1bd34eb818c978` added randomized read-only-on-primary coverage via `open_read_only_one_in` on 2026-07-20. Before that change, blackbox crash test did not randomly keep a separate read-only DB open on the primary live directory, so the existing `mmap_read + SstFileManager truncation` hazard lacked the second reader needed to trigger SIGBUS. The failure is therefore a probabilistic exposure of the new stress mode when it combines with mmap reads and chunked SST truncation. Fix: disable `open_read_only_one_in` in the existing `mmap_read=1` sanitizer path. This preserves read-only-on-primary stress coverage for non-mmap runs while avoiding an OS-level mmap/truncation hazard that RocksDB cannot convert into a normal `Status`. Reviewed By: jaykorean Differential Revision: D113774892 fbshipit-source-id: f82ec2924c12f4512aeeb7a913659dc9a72a4c2f |