Repository navigation
Conversation
Summary: Pull Request resolved: meta#14934 First diff in the async read stack: pure plumbing, no behavior change. Both the point `Get` path and the batched `MultiGet` path gain synchronous/coroutine twins generated by `DEFINE_SYNC_AND_ASYNC`; `DB::GetAsync` and `DB::MultiGetAsync` become the public callback-based async entry points with synchronous default implementations. For `Get`, the existing `DBImpl::Get` / `GetImpl` bodies are relocated into the sync/async wrapper. For `MultiGet`, the same wiring continues through the DBImpl, version, and table-cache layers. The generated coroutine variants still run the existing synchronous logic. This keeps the refactor mechanical while preserving dispatch semantics: the coroutine side remains virtual at the same `GetImpl` boundary as the synchronous path, read-only and secondary DBs use generated sync/coroutine overrides instead of falling back to base `DBImpl`, and `StackableDB` leaves async `Get` / `MultiGet` to the base wrappers so wrapper-specific `Get` / `MultiGet` behavior is preserved. For ABI compatibility and avoiding MACRO blocks in public headers, the asynchronous APIs are callback based instead of coroutine based. Internally it will be developed using coroutines, and applications can also develop their own coroutine wrappers. This is meant to make the later changes to actually enable coroutine-backed async queries much easier to review. Benchmarks: Compared `db_bench` with and without this diff using: `db_bench --benchmarks=fillseq,readrandom --num=1000000 --reads=5000000 --key_size=16 --value_size=100 --seed=12345 --db=<tmp>` | Benchmark | Without diff avg | With diff avg | Delta | | `fillseq` | 4.076 us/op | 4.165 us/op | +2.2% | | `readrandom` | 3.939 us/op | 3.947 us/op | +0.2% | No meaningful local `db_bench` regression was observed. Reviewed By: xingbowang Differential Revision: D109346179 fbshipit-source-id: 0dee1239f49099b67c11c63426204466e963f903
Summary: Pull Request resolved: meta#14985 The Open Source Automated Checkup (T280493780) flagged the 27 generated `c_api_gen/c_generated_*.inc` fragments for missing the required copyright header ("Meta Platforms, Inc. and affiliates"). The fragments are emitted by the C API code generators, none of which wrote a header, so every regeneration reproduced the violation. Root-cause fix in the generators so regenerated output is compliant: - `tools/c_api_gen/generate_c_api.py` (`HEADER_BANNER`) - `tools/c_api_gen/auto_simple_bindings.py` (`auto_header_banner`) - `tools/c_api_gen/gen_roundtrip_tests.py` Each now emits the standard Meta dual-license block (matching `c_base.h` / `include/rocksdb/c.h`) ahead of the `// generated` marker, and the 27 checked-in `.inc` fragments are regenerated accordingly. The assembled `include/rocksdb/c.h` and `db/c.cc` already carry a top-level header, so `regen_all.py`'s inliner now strips each fragment's license block when inlining to avoid repeating it once per fragment; those two files are therefore unchanged. Also adds the header to `tools/gtest_parallel_repro.py`, the one non-`.inc` source file flagged by the checkup. Reviewed By: pdillinger Differential Revision: D113242908 fbshipit-source-id: 4d81d1355c8d8a35cbc4377dd9b21df901ad3cb7
…meta#14991) Summary: Pull Request resolved: meta#14991 D91705195 ("Replace Prefetch Logic in BlockBasedTableIterator with IODispatcher") moved MultiScan block prefetching out of BlockBasedTableIterator and into the IODispatcher. The IO-level RecordTick/RecordInHistogram calls lived in the deleted prefetch code, and the IODispatcher was not given equivalent instrumentation, so these tickers were still defined but never populated (they published as absent): - MULTISCAN_BLOCKS_PREFETCHED - MULTISCAN_BLOCKS_FROM_CACHE - MULTISCAN_PREFETCH_BYTES - MULTISCAN_IO_REQUESTS - MULTISCAN_IO_COALESCED_NONADJACENT - MULTISCAN_BLOCKS_PER_PREPARE (histogram) The PREPARE_*, SEEK_ERRORS, and PREFETCH_BLOCKS_WASTED tickers were unaffected (recorded on the prepare/teardown path, which was not replaced). Fix: - ReadSet now exposes generic, caller-agnostic prefetch counters (GetNumBlocksPrefetched, GetNumIORequests, GetPrefetchBytes, GetNumCoalescedNonadjacent), incremented once per block in DispatchPrefetch -- the single choke point for both immediate and memory-deferred dispatch. Counting at dispatch time (not completion) keeps async and deferred prefetches accounted for without double counting the async->sync fallback. - PrepareIORequests now outputs prefetch_bytes (sum of request len) and num_coalesced_nonadjacent (within-threshold gap count) while building coalesced requests, per review feedback to avoid the extra pass over read_reqs/coalesced_block_indices at former line 836. DispatchPrefetch now gets stats directly from PrepareIORequests. - The MULTISCAN_* ticker names stay at the caller: MultiScanIndexIterator's destructor records the aggregate counters at end-of-scan (alongside the existing PREFETCH_BLOCKS_WASTED), so async/deferred reads that complete during iteration are included. MULTISCAN_BLOCKS_PER_PREPARE is recorded in BlockBasedTableIterator::Prepare. This keeps the IODispatcher a generic mechanism rather than leaking a caller's metric names into it. - Also plumb the same metrics into PerfContext for per-operation visibility. Reviewed By: xingbowang Differential Revision: D113356436 fbshipit-source-id: 660e9755d6b1b9aecccd112df94eb6838061b298
Summary: Pull Request resolved: meta#14945 First diff in a stack that lets `Checkpoint` and `BackupEngine` share one parallel copy/link infrastructure. Pure options refactor, no behavior change. Adds two shared option bases in `rocksdb/utilities/checkpoint.h`: `CheckpointOrBackupEngineOptions` (engine-level config) and `CreateCheckpointOrBackupOptions` (per-operation options). `CheckpointEngineOptions` / `CreateCheckpointOptions` derive from them for checkpoint; `BackupEngineOptions` / `CreateBackupOptions` derive from them for backup and keep their backup-only fields. Reviewed By: xingbowang Differential Revision: D111387916 fbshipit-source-id: d7efa528ac91fb00090130c9d2a671b13e934a82
meta#14994) Summary: `USE_CLANG=1 make check-c-api-gen` failed with "clang++ not found" because the staleness check and the Python generator detected clang inconsistently: the Makefile inspected the `$(CXX)` make variable (the fbcode clang path) while the generator read the `CXX` environment variable, which make never exports. More broadly, clang++ was detected in several ad hoc places with fragile heuristics (grepping the compiler path, duplicated fallback lists in shell and Python). Centralize clang++ detection in build_detect_platform as a single `CLANG_CXX`: - `CLANG_CXX` honors `$CXX` when it is itself clang (so CI's `CXX=clang++-21` is used), else takes the first clang++ on PATH (bare, versioned, or the Meta-internal clang++.par). `CXX` (when building with clang) and `CLANG_ANALYZER` now derive from `CLANG_CXX` instead of re-detecting. - Infer `USE_CLANG=1` when the compiler is clang, detected via the compiler's own `__clang__` predefined macro rather than matching the binary path (which false-positives, e.g. a home dir containing "clang"), and emit it so clang-specific Makefile logic is consistent. - Emit `CLANG_CXX` to make_config.mk and pass it explicitly to the codegen, so `make check-c-api-gen` works without requiring `USE_CLANG=1` and the generator never guesses. Add a first-class `make regen-c-api` target -- now the preferred regenerate command referenced by the generated banners and user-facing messages. The Python generators no longer detect clang themselves; `--clang` defaults to `clang++` for manual runs and is overridden by the Makefile with `CLANG_CXX`. Regeneration verifies the clang runs before writing any file, so a missing/broken toolchain aborts cleanly instead of leaving a partially regenerated tree. Make field discovery clang-version-independent: older clang (e.g. 15) spells field types fully qualified (`rocksdb::ReadTier`, `rocksdb::BlockBasedTableOptions::IndexType`) while the fbcode/CI clang (21) uses the bare `ReadTier` / `IndexType` the family tables expect. Strip the ROCKSDB namespace and enclosing-struct qualifiers so both agree; this is a no-op on newer clang, so checked-in output and CI are unaffected. Also escape the literal "generated" in the generator .py sources (they emit the marker but are not themselves generated) so tooling stops treating them as read-only generated files. Pull Request resolved: meta#14994 Test Plan: Manual verification (not covered by CI): - `make check-c-api-gen` now runs the staleness sub-check (instead of skipping) and passes in a default gcc build as well as with `USE_CLANG=1` on an fbcode host, since `CLANG_CXX` is always populated. - `ROCKSDB_NO_FBCODE=1 make regen-c-api` using a system clang 15 (clang++.par) produces output byte-identical to the checked-in files (generated with clang 21), and `check-c-api-gen` reports "up to date" -- confirming the namespace/struct stripping is version-robust and a no-op on clang 21. - `regen_all.py --clang /nonexistent` exits non-zero with a clear message and leaves the generated files untouched. - `make regen-c-api` is idempotent (no diff on a clean tree). - Confirmed the escaped `\x40generated` still emits `// generated` byte-for-byte in the regenerated c.h/c.cc/.inc. - Reviewed .github/workflows/pr-jobs.yml: the build-linux-clang-21-no_test_run job's `CXX=clang++-21 make check-c-api-gen` resolves `CLANG_CXX` to clang++-21 (honoring `$CXX`) rather than the container's stray bare clang++. Reviewed By: xingbowang Differential Revision: D113447462 Pulled By: pdillinger fbshipit-source-id: 91ca92d21a71a8b92f9ff57500a8cec91a884078
meta#14995) Summary: db_bench had no way to exercise SST embedded (same-file) blob reads or wide-column entity reads (GetEntity/MultiGetEntity), which are needed to measure the zero-copy embedded-blob wide-column entity read path. This adds: - Two setup benchmarks, fillembeddedentity and fillembeddedblob, that build an SST via SstFileWriter::OpenWithEmbeddedBlobs and ingest it (embedded blobs can only be produced this way, not by fillrandom/flush/compaction). Entity mode writes wide-column entities whose large columns are embedded blobs; blob mode writes whole-value embedded blobs. Both require format_version >= 7. - readrandomentity (GetEntity) and multireadentity (MultiGetEntity) read benchmarks; plain readrandom is reused for the whole-value embedded blob baseline and for entity default-column Get. - Flags embedded_blob_min_size, num_wide_columns, and num_short_wide_columns to control the embedded threshold and the inline/embedded column mix per entity. Pull Request resolved: meta#14995 Test Plan: Manual db_bench runs of the new benchmarks, used to validate D113347622. Reviewed By: xingbowang Differential Revision: D113455538 Pulled By: pdillinger fbshipit-source-id: c6b62f5a2d6a61ae0b7cc32ac1b445b566ea8842
…ta#14990) Summary: This is a follow-up to D112212266 / meta#14943 (multi-buffer PinnableWideColumns), which added the zero-copy read path for separate-blob-file references and explicitly left extending it to SST embedded-blob references as a follow-up. Point GetEntity/MultiGetEntity of a wide-column entity whose columns are stored as embedded (same-file) blob records previously deserialized the entity, copied every column name/value, fetched each embedded blob (copying it), and re-serialized the whole entity before GetContext ever saw it (BlockBasedTable::MaybeResolveEmbeddedValue's wide-column branch). This extends the existing multi-buffer PinnableWideColumns zero-copy machinery (previously only for separate blob files / direct-write) to embedded blob columns. Changes: - New SameFileBlobReader interface (db/blob/same_file_blob_reader.h), a narrow seam for resolving a same-file BlobIndex against the current SST. BlockBasedTable implements it (GetSameFileBlob), delegating to the existing blob-cache-backed / pinned embedded reads. - BlobFetcher is now an abstract interface; the Version-backed implementation moved to VersionBlobFetcher. Added an EmbeddedAwareBlobFetcher decorator that routes same-file references to a SameFileBlobReader and delegates all others to a base fetcher (EffectiveFetcher() selects the decorator only when a same-file reader is present). - The point-Get and MultiGet loops now hand the raw entity to GetContext with the table as SameFileBlobReader, instead of pre-resolving/re-serializing. This is gated on not building a row-cache replay log, so a row-cache entry still captures a fully resolved entity (a later row-cache hit never reopens the SST to resolve same-file references). - GetContext composes the decorator and resolves same-file and separate-file blob columns zero-copy across the columns / default-value / merge consumers. - The iterator (DBIter) path is unchanged (still re-serializes via MaybeResolveEmbeddedValue). - Also fixes latent method hiding in the test-only MockBlobFetcher. Intended follow-ups: - Lazy/deferred embedded resolution: return a LazyWideColumns result whose blob columns stay unresolved and are resolved on demand (whole or by byte range). The SameFileBlobReader seam and the polymorphic BlobFetcher introduced here are the mechanism for embedded support there. - ReadOptions usage cleanup/optimization: have DBIter/BlobReader hold a single ReadOptions instead of decomposed scalar fields (removing the decompose-then-recompose in CreateBlobFetcher and closing the field-loss gap), and split VersionBlobFetcher into a ReadOptions-referencing variant for transient/stack fetchers plus an owning variant for lifetime-independent ones (the lazy resolver and compaction), to avoid copying the (growing) ReadOptions on the hot path. Pull Request resolved: meta#14990 Test Plan: Added embedded wide-column tests to db/blob/db_blob_index_test.cc: - GetEntity/MultiGetEntity of an embedded-blob wide-column entity return the correct columns and resolve them zero-copy (blob columns served from the blob cache: identical data() across reads and in separate backing buffers; pointer-stable across a PinnableWideColumns move). - Plain Get of an entity whose default column is an embedded blob. - Merge base and GetMergeOperands over an embedded-blob entity. - read_tier == kBlockCacheTier yields Incomplete for an unresolved same-file reference. - A corrupt embedded record surfaces Corruption, not a raw BlobIndex. # Performance validation with db_bench Measured with db_bench benchmarks added in meta#14995 / D113455538, temporarily stacked on this diff for A/B testing. ## Setup - **Baseline**: this diff's parent + db_bench benchmark commit. - **Treatment**: this diff, incl. its test/lint fixups + the same db_bench benchmark commit. - Both are release builds (`DEBUG_LEVEL=0 make db_bench`). The diff touches only the read path, so both binaries produce byte-identical SSTs; the db_bench commit is additive (`tools/db_bench_tool.cc` only) and identical in both. - Host: AMD EPYC-Genoa, 166 vCPU, 224 GiB RAM. Single reader thread. ## Method - Datasets are created **once** and read by **both** binaries via `--use_existing_db`, so the A/B compares identical on-disk data. - Each read runs twice in-process (warmup + measured); the blob cache (1 GiB) and block cache (512 MiB) hold the whole ~400 MiB dataset, so the reported (2nd) run is fully warm — isolating the column-resolution cost the diff changes rather than I/O. - 100k keys, `value_size=1024`, `embedded_blob_min_size=256`, `compression_type=none`, `format_version=7`, `--duration=10` per measured run. - Datasets: - **A** — wide-column entity, `num_short_wide_columns=1` + `num_wide_columns=4` (small inline default column + 4 embedded-blob columns). Read via `readrandomentity` (GetEntity) and `multireadentity` (MultiGetEntity, batch 32). - **B** — wide-column entity, `num_short_wide_columns=0` + `num_wide_columns=4` (the default column itself is an embedded blob). Read via plain `readrandom` (Get returns the resolved default column). - **C** — whole-value embedded blobs (`fillembeddedblob`). Read via plain `readrandom`. This path is **not** changed by the diff — a regression check. Example commands: ``` # create (once) db_bench --benchmarks=fillembeddedentity --db=/tmp/perfA --use_existing_db=0 \ --num=100000 --value_size=1024 --embedded_blob_min_size=256 --format_version=7 \ --num_wide_columns=4 --num_short_wide_columns=1 \ --use_blob_cache=true --blob_cache_size=1073741824 --cache_size=536870912 \ --compression_type=none # read (each binary; warmup + measured) db_bench --benchmarks=readrandomentity,readrandomentity --db=/tmp/perfA \ --use_existing_db=1 --threads=1 --duration=10 <same options> ``` ## Results (warm, 2 runs each) | Benchmark (read path) | Baseline µs/op | Treatment µs/op | Latency | Baseline Kops/s | Treatment Kops/s | Throughput | |---|---|---|---|---|---|---| | `readrandomentity` — GetEntity, inline default + 4 embedded cols | 7.16 | 5.00 | **-30%** | 140 | 200 | **1.43×** | | `multireadentity` — MultiGetEntity (batch 32) | 6.46 | 4.54 | **-30%** | 155 | 220 | **1.42×** | | `readrandom` — Get of embedded default column (dataset B) | 6.55 | 2.12 | **-68%** | 153 | 471 | **3.09×** | | `readrandom` — whole-value embedded blob (dataset C, unchanged path) | 1.92 | 1.91 | ~0% | 521 | 524 | neutral | Both runs were within ~1–3% of each other (raw numbers below). ## Interpretation - **GetEntity / MultiGetEntity of embedded-blob wide-column entities improve ~30% in latency (~1.4× throughput)** — the expected win from resolving embedded blob columns zero-copy into the `PinnableWideColumns` backing buffers instead of the old deserialize + per-column copy + re-serialize round trip. - **Plain `Get` of an entity whose default column is an embedded blob improves ~3×** — the largest win, because the old path re-serialized the entire entity even when only the default column is returned, whereas the new path resolves just that column. - **The whole-value embedded blob `Get` path is unchanged (~0% delta), confirming no regression** on the pre-existing zero-copy path. No unexpected regressions observed; all deltas match the diff's intended scope. ## Raw measured numbers (both runs) ``` readrandomentity BASELINE 7.183 / 7.137 µs/op 139225 / 140118 ops/s readrandomentity TREATMENT 4.992 / 5.008 µs/op 200333 / 199681 ops/s multireadentity BASELINE 6.458 / 6.458 µs/op 154837 / 154843 ops/s multireadentity TREATMENT 4.509 / 4.577 µs/op 221798 / 218463 ops/s readrandom (Bdef) BASELINE 6.497 / 6.609 µs/op 153920 / 151307 ops/s readrandom (Bdef) TREATMENT 2.126 / 2.120 µs/op 470317 / 471805 ops/s readrandom (Cblob) BASELINE 1.891 / 1.947 µs/op 528760 / 513554 ops/s readrandom (Cblob) TREATMENT 1.895 / 1.920 µs/op 527767 / 520807 ops/s ``` Reviewed By: xingbowang Differential Revision: D113347622 Pulled By: pdillinger fbshipit-source-id: c51bc187085695d559411d9c0555bb52c8e0a61a
Summary: Pull Request resolved: meta#14952 `DBImpl::ResumeImpl()` constructed its local `ReadOptions`/`WriteOptions` with defaults and carried a `// TODO: plumb Env::IOActivity, Env::IOPriority` comment. Those options flow into `VersionSet::LogAndApply()` (the MANIFEST write during error recovery), so the rate limiter could not attribute or prioritize that I/O correctly. This resolves the TODO for `ResumeImpl` (1 of 91 identical TODOs across the codebase) by accepting an `Env::IOActivity` parameter and constructing the options with it, following the pattern already established in `Get()`, `MultiGet()`, and `NewIterator()`, and in the `kDBOpen` paths in `db_impl_open.cc`. Error recovery is driven by flush operations (as the existing comment in `ErrorHandler::RecoverFromRetryableBGIOError` notes), so both production callers and the `TEST_ResumeImpl` hook pass `Env::IOActivity::kFlush`. Setting `io_activity` on these options is safe: `LogAndApply` does not assert `io_activity == kUnknown`. Reviewed By: pdillinger Differential Revision: D112537885 fbshipit-source-id: 59045fd2f0a6e5bbacef6e0381e03ff95cd28c51
Summary:
Extracts BackupEngine's parallel file-copy/checksum thread pool into a standalone `CopyEngine` (`utilities/copy_engine/copy_engine.{h,cc}`) so `CheckpointEngine` (later in the stack) can reuse it. Behavior-preserving; all `TEST_SYNC_POINT` names are unchanged.
Pull Request resolved: meta#14946
Reviewed By: xingbowang
Differential Revision: D111387950
Pulled By: rban1
fbshipit-source-id: b5046adb98735a606fd1ba471731fd316ce6f036
Summary: Pull Request resolved: meta#14947 Teaches the shared `CopyEngine` to hard-link files through its background pool: adds `WorkItemType::Link` and `CopyEngine::LinkFile(src, dst, dst_env)`, which returns the FileSystem's `NotSupported` when links are unavailable so callers can fall back to copying. This is what CheckpointEngine (next diff) needs for warm storage, where linking is supported but benefits from parallelism. No existing caller links through the engine yet (BackupEngine never hard-links), so no behavior change; the path is exercised by a new `copy_engine_test.cc` (parallel copy, parallel link, single-thread copy, per-item error reporting). Reviewed By: xingbowang Differential Revision: D111387972 fbshipit-source-id: ec0b0e9b80749a3f51e62066ceef2df21a1811d2
Summary: Pull Request resolved: meta#14948 Adds a reusable `CheckpointEngine` (opened once, works for any DB) that creates checkpoints via the shared `CopyEngine` pool, copying and hard-linking data files across `max_background_operations` threads. `CheckpointImpl::CreateCheckpoint` is refactored into a shared `CreateCheckpointImpl`, with the serial/parallel split behind a `CheckpointFileMover`; files the FileSystem cannot hard-link fall back to copies. The legacy `Checkpoint` API stays serial and unchanged. Reviewed By: xingbowang Differential Revision: D111387993 fbshipit-source-id: 036d375da9c9b8f051cfc3da0551b035e29d80c4
…orrupted atomic group The file (meta#15000) Summary: Pull Request resolved: meta#15000 Reviewed By: xingbowang Differential Revision: D113225538 Pulled By: pdillinger fbshipit-source-id: fea9a671e9291ed030c5396264734195b2fb78bf
…ener_wait) (meta#14992) Summary: DB::Flush() with wait=true only waits until the flush result is committed (memtables removed from the immutable list). The EventListener::OnFlushCompleted callbacks run afterward, on the background flush thread, with the DB mutex released, and the waiter is only woken by a shared condition variable that many unrelated events also signal. As a result a Flush(wait=true) caller can return before -- or in the middle of -- its OnFlushCompleted callbacks. This surfaced as a flaky failure in the c_test table_filter_listener_metadata case, which asserts listener_state.flush_count == 3 immediately after Flush() returns (https://github.com/facebook/rocksdb/actions/runs/29976431449). It is also long-standing behavior that other tests have worked around with TEST_WaitForBackgroundWork(). This adds an opt-in FlushOptions::listener_wait (default false). When set together with wait=true, Flush() does not return until the OnFlushCompleted callbacks for the flushed memtables have finished. Default behavior is unchanged. Implementation: MemTableList gains a per-column-family num_pending_flush_notifications_ counter (guarded by the DB mutex). The background flush thread reserves a pending notification at flush-commit time -- before the mutex is released to run the callbacks -- and releases it after they return, in both the single-CF (FlushMemTableToOutputFile) and atomic (AtomicFlushMemTablesToOutputFiles) paths. WaitForFlushMemTables treats a CF as finished only when its normal predicate holds and the counter is zero. Because the memtables are removed and the counter incremented under one continuous mutex hold (and, for atomic flush, all CFs are reserved before the per-CF notify loop starts releasing the mutex), a listener_wait waiter can never observe the flush as finished before the callbacks complete. The counter is per-CF rather than per-flush, so a listener_wait waiter can wait somewhat longer than strictly necessary: under rapid flushing it may block until an unrelated, later flush's OnFlushCompleted on the same CF also drains (e.g. a waiter satisfied by GetEarliestMemTableID() advancing still waits for the whole CF's pending notifications). This can only over-wait, never return early, and is tolerated in favor of implementation simplicity; a per-memtable-id watermark could tighten it if it ever matters. To keep the counter balanced against future edits, the release uses an RAII Defer (runs on every exit path) and ~MemTableList asserts the counter has drained to zero; SubPendingFlushNotifications asserts against underflow. A reservation is taken only when a callback will actually fire (committed flush info non-empty, not a mempurge, CF not dropped, listeners registered). Also adds the C API rocksdb_flushoptions_set/get_listener_wait (regenerated via tools/c_api_gen) and a DBImpl::TEST_FlushMemTableWithListenerWait() helper. Pull Request resolved: meta#14992 Test Plan: New db_flush_test cases ListenerWaitWaitsForOnFlushCompleted and ListenerWaitAtomicFlushWaitsForAllOnFlushCompleted use a listener that blocks inside OnFlushCompleted plus spurious bg_cv_ wakeups to deterministically verify that Flush(wait=true, listener_wait=true) does not return until the callback (all CFs, for atomic flush) has finished; the atomic case guards the per-CF mutex-release gap in the notify loop. Converted existing racy/worked-around cases to the new option: c_test table_filter_listener_metadata, listener_test OnSingleDBFlushTest and MultiDBMultiListeners, and db_flush_test FireOnFlushCompletedAfterCommittedResult (the last previously relied on TEST_WaitForBackgroundWork()). The auto-generated C API roundtrip test covers the new flushoptions field. db_stress StressTest::TestFlush now sets listener_wait on a hard-coded 1 in 4 of its manual flushes (both the atomic and non-atomic branches), so the crash test exercises the new path with the DbStressListener registered. Reviewed By: jaykorean Differential Revision: D113425407 Pulled By: pdillinger fbshipit-source-id: 6fd0f5bc93fabaccd8010c2fead1d93c4c44ccfb
…dex (meta#14993) Summary: Pull Request resolved: meta#14993 A corrupted RocksDB SST data block whose footer sets the hash-index bit (`kDataBlockBinaryAndHash`) could drive an out-of-bounds heap read while the block is being parsed. `DataBlockHashIndex::Initialize` guarded its `num_buckets` / `map_offset` arithmetic only with `assert()`, which is compiled out in release and ASAN builds. When the encoded `num_buckets` exceeds the block, the `uint16_t` `map_offset` computation underflows; that value then flows into `Block`'s ctor at `input.remove_suffix(input.size() - map_offset)`, inflating `restart_offset_` far past the real allocation. If `size` is later zeroed as an error marker (e.g. a corrupted separated-KV footer), `Block::GetCorruptionStatus` builds a `Slice(data(), restart_offset_)` and `DataBlockFooter::DecodeFrom` reads `data() + restart_offset_ - 4`, over-reading the block's heap buffer (reported by agentic fuzzing, T279933950). Fix: - `DataBlockHashIndex::Initialize` now returns `bool` and rejects a `num_buckets` of 0 or one whose bucket array plus the `NUM_BUCKETS` field does not fit within `size`, instead of relying on debug-only asserts. - `Block`'s ctor checks that return value and also rejects blocks too large for the 64KiB `uint16_t`-offset hash-index format, marking the block corrupt (`size = 0`) rather than under-flowing `map_offset` and over-reading. Reviewed By: pdillinger Differential Revision: D113429299 fbshipit-source-id: 3167106e793252c09fd6a558e3ac96de7a2169c0
Summary: Pull Request resolved: meta#15001 Add IODispatcher-specific histograms for async read observed completion latency, `FileSystem::Poll` wait time, and submit-to-consumer-poll prefetch lead time. Record submit timestamps on async read state, report completion latency from the callback, and report poll wait/prefetch lead when the block consumer polls the handle. Keep Java histogram byte mappings stable by assigning the new histogram IDs after the existing range. Add a controlled async filesystem test that verifies one deferred async read records all three histograms. Address Infer feedback by iterating `async_state->block_indices` directly when removing async IO map entries. Keep `PollAndProcessAsyncIO()` holding an owning `shared_ptr<AsyncIOState>` while erasing those entries so the coalesced request state cannot be destroyed mid-loop. Format `util/io_dispatcher_imp.cc` and `util/io_dispatcher_test.cc` with clang-format 21.1.2 to fix the format signal. Reviewed By: pdillinger, anand1976 Differential Revision: D113549401 fbshipit-source-id: dafc272c9252e1baccf404f681d33f76890ce0b2
Summary: Pull Request resolved: meta#14998 T280925646 reported an ASAN double-free in db_stress' batched prefix scan. The visible free was in VerifyWideColumns(), but WideColumn values are non-owning Slice views, so that destructor should only free the vector element storage. The stack points to prior ownership corruption rather than an intentional verifier free. The concrete bug is in PinnableSlice's move constructor: it delegates to move assignment without first initializing the target object. When moving from a pinned slice, move assignment does not set buf_ because the target is still pinned. If that moved-to slice is later Reset() and reused with PinSelf(), it writes through an uninitialized buf_ pointer. This became reachable after D112212266 changed PinnableWideColumns to keep backing buffers in std::forward_list<PinnableSlice> and emplace moved PinnableSlice values into backing nodes. Fix the move constructor by delegating to the default constructor first, so buf_ is initialized before move assignment. Add a regression that poisons placement-new storage, move-constructs from a pinned slice, resets it, and reuses it with PinSelf(); this crashed before the fix. Reviewed By: pdillinger Differential Revision: D113480016 fbshipit-source-id: 38ce9030f392ff9f0438d38d53d19e356e2d2fbf
Summary: Follow-up cleanup to the embedded (same-file) blob wide-column work (PR meta#14990): the "ReadOptions usage cleanup/optimization" item. Split the Version-backed blob fetcher into two variants sharing a common VersionBlobFetcherBase (which implements FetchBlob via a virtual read_options()): - VersionBlobFetcher now holds a `const ReadOptions&` for transient/stack fetchers (the common case: point Get/MultiGet, DBIter, version_set), so the growing ReadOptions is no longer copied per fetcher on the hot path. A deleted rvalue-ref constructor rejects binding to a temporary. - OwningVersionBlobFetcher takes `ReadOptions&&` and owns a copy, for fetchers whose lifetime is independent of the caller (the read-path lazy resolver and compaction), making ownership transfer explicit. DBIter::BlobReader now holds a single blob fetcher instead of decomposed scalar fields (read_tier/verify_checksums/fill_cache/io_activity). This removes the decompose-then-recompose that the old CreateBlobFetcher and RetrieveAndSetBlobValue performed, and closes the field-loss gap: all ReadOptions fields now flow through to blob reads rather than just those four. CreateBlobFetcher is gone; the merge path uses the reader's owned fetcher directly. A CanResolve() predicate on the fetcher preserves the prior "unexpected blob index" Corruption instead of dereferencing a null Version, and the now-redundant allow_blob_write_path_fallback_ member/ parameter (equal to the fetcher's own flag) was dropped. ReadPathBlobResolver keeps owning its fetcher (now OwningVersionBlobFetcher) so it stays self-contained for the planned lazy entity read path, where the result outlives the read call. A TODO in db_iter.h records the remaining duplication (BlobReader and ReadPathBlobResolver each own a fetcher with the same {version, read_options, blob_file_cache, allow_write_path_fallback}), deferred to that work. Pull Request resolved: meta#15003 Test Plan: Pure refactoring with no intended behavior change; Version::GetBlob's Slice overload just decodes and calls the BlobIndex overload, so routing whole-value blob reads through the fetcher is equivalent, and the direct-write fallback flag/read options are unchanged. Covered by existing tests: - db/blob/db_blob_index_test.cc: embedded/entity blob GetEntity/MultiGetEntity zero-copy, plain Get of an embedded default column, merge over an embedded-blob entity, read_tier == kBlockCacheTier Incomplete, corrupt record Corruption. - db/blob/db_blob_basic_test.cc and db/wide/db_wide_blob_direct_write_test.cc: separate-file and direct-write blob resolution (the fallback branch). - db/db_iterator_test.cc (blob) and db/compaction/compaction_iterator_test.cc: DBIter blob/merge paths and the compaction (owning) fetcher. Reviewed By: xingbowang Differential Revision: D113582247 Pulled By: pdillinger fbshipit-source-id: 231c9364e266b826d12a8479cbb7f026a0d26bf6
Summary: Pull Request resolved: meta#15006 Block::GetCorruptionStatus() uses restart_offset_ in error state to decide whether it can re-decode the data block footer for a more specific corruption status: nonzero means the footer decode failed and restart_offset_ stores the original block size; zero means the corruption was detected after the footer decoded. For separated-KV data blocks, Block::Block() first sets restart_offset_ to the restart-array offset, then validates footer.values_section_offset. If that offset is past the restart array, the block is marked corrupt with size = 0, but restart_offset_ was left nonzero. NewDataIterator() then called GetCorruptionStatus(), which treated the stale restart-array offset as an original block size, re-decoded a syntactically valid footer, and hit DEBUG_FAIL("ok status on presumed bad block contents"). Reset restart_offset_ to 0 on this semantic-corruption path so GetCorruptionStatus() reports generic block corruption instead of trying to re-decode a footer that already decoded successfully. The regression test builds a separated-KV hash-index data block, corrupts only the encoded values_section_offset to point past the block, and verifies NewDataIterator() returns "Corruption: bad block contents". Removing the fix line makes the test reproduce the original assertion. This update also applies the clang-format layout required by check-format for the new regression test. Reviewed By: joshkang97 Differential Revision: D113561303 fbshipit-source-id: 8d989a38761063c62694837976aad98f1868f884
Summary: Pull Request resolved: meta#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
Summary: Pull Request resolved: meta#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
Summary: Pull Request resolved: meta#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
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: meta#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
Summary: Pull Request resolved: meta#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
Summary: Pull Request resolved: meta#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
Summary: Pull Request resolved: meta#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
…14981) Summary: Pull Request resolved: meta#14981 Reviewed By: pdillinger Differential Revision: D112965953 fbshipit-source-id: 9645d9287a9aba987bf1b37752df7f9dcb3a9669
…ion (meta#15021) Summary: Pull Request resolved: meta#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 meta#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
Summary: Pull Request resolved: meta#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
Summary: Pull Request resolved: meta#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
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: meta#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
Summary: Pull Request resolved: meta#15484 Four `TrieIndexDBTest` mode combinations are permanently inapplicable: `MigrationFullPath`, `RollbackFromPrimaryWithoutCompactSucceeds`, and `EstimatedSizeNonZero` in `kCustomOnly`, and `PrefetchWithCustomIndexWrapper` in `kStandardDefault`. Using `ROCKSDB_GTEST_SKIP` for these intentional omissions generated eight GTest/TSan skipping issues in T294331850. Change exactly those four calls to `ROCKSDB_GTEST_BYPASS`, matching the permanent-parameter-omission contract documented in `test_util/testharness.h` and already used elsewhere in this test file. Keep the original guards, `return`s, fixture, names, and all 253 registrations unchanged. BYPASS prints its reason and records GTest success instead of a skip; these inapplicable combinations still do not execute the test assertions. All eight applicable cases and every other byte of the test file are preserved. No derived fixtures or suite splits remain. This changes test reporting only, not database behavior, API/ABI, persisted formats, or performance. Unrelated capability-dependent skips and existing bypasses are untouched. TestX recovery must be audited after landing; passing bypass cases are not evidence of additional functional coverage. Public GitHub export remains a separate prerequisite and is not authorized by this revision request. ___ `Trail`: https://trail.nest.x2p.facebook.net/c/da4fc690-13ac-410e-b1ee-faf2b010e677 ___ landed-with-radar-review Differential Revision: D127646540 fbshipit-source-id: 7159e20c88e358c68b1873008ff4134ceae29162
Summary: Pull Request resolved: meta#15240 Why: Shard splits and merges currently require scanning ranges and rewriting them through the write path. These utilities transfer selected half-open `[begin_key, end_key)` ranges through checkpoints, clipping compactions, and SST ingestion. Boundary-only clipping is deferred; kept keys are still rewritten by compaction. Behavior: - `SplitDB` builds and verifies a staged destination, rewrites its OPTIONS file with the source CF options without reopening it, and publishes it by rename and directory fsync. The destination is not opened with the source options; the caller's first open validates it, with sync or async file opening as the caller chooses. Source data remains available. After switching traffic, callers run retryable `DeleteSplitRanges`; combining those steps would leave a failure ambiguous after source deletion. - `MergeDB` clips a source checkpoint and ingests the latest range contents into empty destination ranges in one crash-atomic multi-CF MANIFEST update. It transfers source sequence numbers rather than snapshot history. - `db_split_merge` provides `split`, `delete-ranges`, and `merge` commands with hex-encoded bounds and strict OPTIONS loading. Safety and compatibility: - Callers must stop writes to affected ranges, reserve the temporary paths, and avoid destination snapshots during merge. Existing destination snapshots are rejected. Merge visibility across CFs is not atomic for concurrent readers. - Clipping disables source filters, callbacks, and the WAL filter; source flushes and subsequent source deletion retain their normal filter behavior. Checkpoint opens use strict recovery, and failed splits remove partial outputs. - Support is limited to plain writable DBs with compatible BlockBasedTable CFs, leveled/universal compaction, and colocated SSTs. Transaction/TTL/blob wrappers, timestamps, ingest-behind, and other incompatible options are rejected or documented as unsupported in `db_split_merge.h`. Merge requires the same FileSystem object and rejects destination tombstone conversion; old iterators created with conversion enabled must first be retired. Review fixes: Live blob detection now reads `kNumBlobFiles` without copying SST metadata. `ClipColumnFamily` now checks surviving file presence instead of treating empty boundary strings as an empty CF: previously, a lone empty-string key in L0 caused split/merge of an excluding range to fail verification. Depends on D123306376 for the separate begin-boundary fix. Validation: See the test plan for regression failures without the fixes, sanitizer runs, stress repetitions, full Make checks, and platform/tooling limitations. Reviewed By: xingbowang Differential Revision: D120757697 fbshipit-source-id: df04ea49100eb102ba04321fc23fdca48fc379e5
…eta#15489) Summary: Pull Request resolved: meta#15489 **Adds `SetAsyncReadCPUTimeEnabled()`, so an async read can report the CPU it used without turning on every perf timer.** - **Problem:** a coroutine read runs on the read executor, not the caller's thread. RocksDB sums its CPU in `PerfContext::get_cpu_nanos`, but only at `kEnableTimeAndCPUTimeExceptForMutex`, which also turns on every wall-time timer. - **Change:** with the new opt-in set and the perf level at least `kEnableCount`, an async read times only its running slices into `get_cpu_nanos`. The setting is thread-local, like the other async-read stats settings, so there are no option or C API changes. It resets after each read. - **Unchanged:** reads without the opt-in, sync reads, and async reads that run synchronously on the caller's thread. Reviewed By: archang19, joshkang97 Differential Revision: D127495339 fbshipit-source-id: 1b0ab016552f216409a1a2c99ed1733269e5dbc3
…eta#15481) Summary: Pull Request resolved: meta#15481 With blob direct write, each memtable switch moves the column family's open blob files into a new generation, queued in `BlobFilePartitionManager::pending_generations_`. A flush then seals and publishes the generations of the memtables it flushes. `PrepareFlushAdditions(n)` always used the first `n` queued generations, and `CommitPreparedGenerations(n)` always popped the first `n`. That is only right if flushes pick, and finish, the oldest memtables in order. With more than one flush running for a column family (`max_background_flushes` > 1, or `max_background_jobs` >= 8), a second flush skips the memtable the first one is still flushing and picks a newer one, but prepares the oldest generation, which belongs to the first flush. Both flush edits then add the same blob file, and installing them fails with `Corruption: VersionBuilder: Blob file #N already added`, which stops the database. The newer memtable's own blob files are never published, and committing the second flush pops a generation it did not own. Atomic flush prepares before it waits to install in order, so it has the same problem. Removing generations by the flush's own memtables is not enough either. A newer flush that finishes first only marks its memtable completed: the older flush installs both, or rolls the newer one back too if it fails. A generation removed by the newer flush is then gone when the retried flush looks for it (`Corruption: Missing blob direct write generation metadata for flush`). Fix: - Tag each generation with the ID of the memtable whose switch created it (`RotateCurrentGeneration(memtable_id)`). `PrepareFlushAdditions()` takes the IDs of the memtables the flush picked and seals exactly those generations, in both flush paths. - Remove generations only once installed: after a successful flush, `CommitFlushedGenerations()` drops the generations of the memtables older than the earliest one still in the immutable list (memtables leave it only by installation, in order), and stops tracking their sealed files as manager-owned. Reviewed By: xingbowang, archang19 Differential Revision: D126852567 fbshipit-source-id: 910bf466674cf8bc89bc3ccbbcf08c684968ad72
Summary: Pull Request resolved: meta#15496 Remote compaction previously transferred only SST inputs and output metadata. A worker could not reliably read blob-backed values for a compaction filter, output SST metadata did not preserve references to existing blob files, and blob garbage collection could derive a different cutoff from the worker view. This change adds BlobDB support to the existing remote-compaction path. Existing `enable_blob_files` and `blob_file_starting_level` settings determine whether eligible output may create blobs; compactions whose input SSTs reference blobs transport those dependencies regardless of new-blob eligibility. Each subcompaction transports the live blob-file suffix referenced by overlapping input SSTs. For blob-eligible output, the primary atomically reserves a half-open blob-file-number range. The worker uses native `CompactionJob`, `BlobFileBuilder`, and `BlobGarbageMeter` machinery and returns `BlobFileAddition`, `BlobFileGarbage`, and SST dependency metadata. When blob GC is enabled, the primary computes the cutoff from its authoritative input Version and serializes it in `CompactionServiceInput`; the worker consumes that cutoff rather than recomputing from its local view. Workers advertise result-side BlobDB and GC-cutoff protocol markers. The primary declines old or non-capable results before installation. It validates reserved additions, transported-input garbage, output dependencies, cross-subcompaction aggregate garbage, staged file existence, optional content checksums, and each staged blob footer count plus the overflow-safe header+record-bytes+footer physical size. Metadata or structural corruption declines the result and reruns locally, while genuine validation I/O errors remain `kFailure`. Blob files move before SSTs and register through `BlobFileCompletionCallback`; rollback physically deletes each installed blob before removing its `SstFileManager` entry, and a cleanup error remains a failure with the tracking entry preserved. Resumable workers checkpoint blob additions, cumulative garbage, and the next reserved number. Checkpoints cut SST and blob outputs before preparing the next key. `CompactionIterator` records when processing has already consumed an earlier input entry of the current user key, and progress is not checkpointed at that unsafe boundary. Resume therefore does not seek back over already-counted input, fixing both the baseline record-count mismatch and blob-garbage double counting. At safe boundaries, the raw pre-filter inflow for the current entry is subtracted from the blob-garbage checkpoint; per-entry inflow metering exists only for resumable jobs. `REMOTE_COMPACT_INPUT_BLOB_FILE_COUNT` counts each distinct transported blob file once per compaction job. `REMOTE_COMPACT_BLOB_RESULT_DECLINED_COUNT` counts primary-side blob-result declines only; service-side `kUseLocal` and pre-dispatch reservation fallback do not increment it. Both tickers are exposed through C++ only; JNI and Java bindings are intentionally omitted because the Java ticker jbyte space is exhausted. Compatibility and failure behavior: New fields deserialize with safe defaults. A new primary declines an old worker when blob inputs, blob-eligible output, or a primary-provided GC cutoff requires the new protocol. A new worker receiving an old input without the cutoff field disables blob GC for that job, avoiding a worker-local cutoff. Primary-side declines rerun locally, so `CompactRange` and `CompactFiles` succeed without a background error. The `CompactionService` contract is unchanged: a service returning `kFailure` preserves the worker status, including `Status::Incomplete`; mapping a worker decline to `kUseLocal` remains the integrator's choice. ZippyDB production additionally maps service failures to local fallback through `HandleFailure(FALLBACK_LOCAL)`. Reviewed By: jaykorean Differential Revision: D122097088 fbshipit-source-id: 7790768fe8a25dd2451a5d69d809d356899cabab
…compaction (meta#15498) Summary: Pull Request resolved: meta#15498 Why: "Support BlobDB in remote compaction" (meta#15496) broke four GitHub CI jobs on `main`: `build-linux-static_lib-alt_namespace-status_checked` and the three `build-windows-msvc` jobs. The `main` run just before it passed all four; every run that includes it, on `main` and on unrelated PRs (including reruns), fails them. Previous behavior: - `ASSERT_STATUS_CHECKED` builds abort with "Failed to check Status" in `compaction_job_test` and `compaction_service_test`: - `CompactionOutputs::AddToOutput()` gained an early `return c_iter.status();` after `PrepareOutputIfNeeded()`. It leaves the local `s` unchecked when nothing assigned it earlier in the call. This is production code; `ResumableCompactionServiceTest.BlobCompactionResumeRangeExhaustionFallsBackLocally` reaches it. - New tests never check `CompactionServiceResult::status`: `CompactionJobTest.ResultSerialization` (`deserialized_old_result`), `CompactionServiceTest.RemoteBlobWorkerDisablesBlobOutputSafely`, a sync point callback in a blob-file rollback test, and every test that reads all results through `MyTestCompactionService::GetResults()` but checks only the one it looks for. - MSVC builds with `/W4 /WX`. Two new lines in `compaction_service_test.cc` pass `'a' + i` (an unbounded `int`) to the `char` parameter of `std::string(size_t, char)`. That narrowing is a warning MSVC reports and `/WX` turns into an error. The GitHub logs are not readable without repo admin rights, so this cause is inferred, not read from the log: clang's `-Wimplicit-int-conversion` flags exactly these two lines and nothing else the commit added, and the existing `'a' + (i % 26)` form, which compilers can prove fits in a `char`, already builds on Windows in `compact_files_test`. New behavior: - `AddToOutput()` assigns `c_iter.status()` to `s` and returns `s`, like its other exits. The returned status is unchanged. - The tests check or explicitly permit each result status. `GetResults()` permits every result's status, the way `GetResult()` and `GetInput()` already permit their read status; tests that care still assert on the result they select. - The two conversions use `static_cast<char>`; `i < 20`, so the values are unchanged. No format, API, or behavior change. ___ landed-with-radar-review Differential Revision: D127938230 fbshipit-source-id: 0c5b05e28b07898067a2038e9376225940e910ed
Summary: Pull Request resolved: meta#15488 `CoroDB::CoGet` queues its child read task onto the selected read EventBase even when the caller already uses that executor. The DB and `SstFileReader` overloads now await the child directly when `co_current_executor` matches that EventBase, avoiding redundant queues on task entry and return. Calls from another executor keep the existing dispatch. This applies to reads that perform I/O as well as cached reads. Asynchronous I/O retains its existing suspension and resumption behavior on the read executor; cancellation and statistics handling are preserved. Reviewed By: pdillinger Differential Revision: D127665852 fbshipit-source-id: 73d969d0abeefc68cb5b34b1164f3a2da943516e
…odes (meta#15332) Summary: Pull Request resolved: meta#15332 Why the change is required: Read-only and secondary DBs use their own `GetImpl` paths, which had drifted from the primary `GetMergeOperands` contract. A caller passing an output array sized by `expected_max_number_of_operands` could get writes past the end of that array. Separately, both paths could copy operands out of memory they had already released. Previous behavior: - Both modes pinned every accumulated operand without checking `expected_max_number_of_operands`. More operands than capacity meant writes beyond the caller's array instead of `Incomplete`. - Output slots were not cleared first, so a reused array could expose stale values. - `MergeContext` never received `GetMergeOperandsOptions`, so `continue_cb` was ignored. - Both modes destroyed the SST lookup's `PinnedIteratorsManager` before copying operands into the caller's slices. SST operands borrow block-cache memory that is freed once unpinned under cache pressure, so the copy in `PinSelf` could read freed memory. - Secondary only: the SuperVersion reference was also released before the copy. Memtable operands are borrowed slices into the memtable, and a `TryCatchUpWithPrimary` that completes in between can retire that memtable, with the same heap use-after-free. New behavior: Both modes now follow the primary contract. They clear every advertised output slot, report the required operand count, and return `Incomplete` with `KMergeOperandsInsufficientCapacity` before writing anything to an undersized array. `continue_cb` stops collection and returns the collected newest suffix. Both modes copy operands while the block pins are still held, and secondary also holds its SuperVersion until the copy is done. Implementation: - Reset the advertised output range up front, forward the options into `MergeContext`, and check capacity before the copy loop. Both match `DBImpl::GetImpl`: the reset is skipped when no options are passed, while the capacity check dereferences them unconditionally. - Both modes: hoist `PinnedIteratorsManager` to function scope, as `DBImpl::GetImpl` does. Secondary also moves the SuperVersion release after the copy. Safety, compatibility, and limitations: Only `GetMergeOperands` results change in read-only and secondary modes, and the primary path is untouched. For `Get` and `GetEntity`, the only change is that the block pins, and on secondary the SuperVersion, are released a few statements later. Read-only needs no SuperVersion change because it never installs a new SuperVersion. There is no API, ABI, or file-format impact. Callers that relied on the old behavior were writing past their declared capacity. Large-result zero-copy pinning for these modes is still a TODO (PR meta#10458) and is handled in the follow-up diff. Reviewed By: jaykorean Differential Revision: D123382954 fbshipit-source-id: 7b0c49b5e96f464a3b8c974940713261a9c0e15e
…lled output slots Summary: X-link: meta#15503 Why the change is required: After D123382954, the primary, read-only, and secondary `GetMergeOperands` paths still each have their own copy of output setup and post-processing, so they can drift apart again. Read-only and secondary also copy every operand, even when the result is large enough for the primary's zero-copy bundle (the TODO for PR meta#10458). Separately, xbw pointed out in review that the up-front output reset scales with the caller's capacity rather than the result. Previous behavior: - Each mode set up `MergeContext` and published operands with its own code. Only the primary used the shared cleanup bundle, which pins a SuperVersion and the iterator pins for large results. Read-only and secondary always called `PinSelf` on each operand. - All three modes called `Reset()` on all `expected_max_number_of_operands` output slots before the lookup, even when the caller had already reset them or only a few operands existed. A caller passing a capacity of 1000 to read 3 operands touched 1000 `PinnableSlice` objects on every call. New behavior: - All three modes publish operands the same way. Read-only and secondary now return large results pinned through the shared bundle, just as the primary does. - `GetMergeOperands` resets only the slots it fills, each one just before writing it, so the cost scales with the result instead of the capacity. Slots after `*number_of_operands`, and the whole array on `Incomplete`, `NotFound`, or an error, are left as the caller passed them. Previously they were cleared. Implementation: - `DBImpl::PrepareGetMergeOperands` now only forwards `GetMergeOperandsOptions` into `MergeContext`. - `DBImpl::FinalizeGetMergeOperands` reports the count, checks capacity before writing anything, then resets and fills each slot. Large results move into the existing `GetMergeOperandsState` bundle, with a new SuperVersion reference and the moved `PinnedIteratorsManager`; small results are copied. The per-slot `Reset()` is still required because a reused slot can hold an earlier pin, and `PinSlice`/`PinSelf` assert the slot is unpinned. - All three `GetImpl` variants call the shared pair. Callers must invoke finalization while their SuperVersion reference and pins are still held; secondary releases its reference afterwards, as in D123382954. The primary keeps the `OkMergeOperandThresholdExceeded` status from `merge_operand_count_threshold` unless finalization fails. Safety, compatibility, and limitations: The reset change is visible through the public API in every mode, including the primary. It still satisfies the documented contract: `db.h` says `merge_operands` is not populated on `Incomplete` and that callers own `Reset()` of the entries. A caller that inspected slots after a failed call, or past `*number_of_operands`, will now see its own earlier contents instead of empty slices. A pin left in a slot this call doesn't touch stays alive until the caller resets it. This diff adds a behavior-change release note. Pinned results from read-only and secondary carry the same requirement as the primary's: the DB must outlive them. There is no ABI or file-format change, and snapshot acquisition and the memtable/SST lookups are unchanged. Reviewed By: jaykorean Differential Revision: D123475366 fbshipit-source-id: 615e09ce0658c2ef86235810ff59dbfc3b54c289
Summary: With `ReadOptions::pin_data`, `LevelIterator` pins every table iterator it leaves and `BlockBasedTableIterator::ResetDataIter()` delegates every data block it leaves to the `PinnedIteratorsManager`, until the `DBIter` is destroyed. For an immortal table (read-only with `max_open_files=-1`) whose uncompressed blocks are read through mmap, the returned slices point into the mapping, which the reader keeps alive for the iterator's Version. The pinned iterators and `Block` objects keep nothing alive and only grow memory with every seek. Add `InternalIterator::IsSafeToDropWhilePinned()` (default false), true for a `BlockBasedTableIterator` whose returned slices all point into an immortal source. `LevelIterator` now deletes such an iterator instead of pinning it, and `ResetDataIter()` frees such a block instead of delegating it. Owned and cached blocks are delegated as before. X-link: meta#15459 Reviewed By: anand1976 Differential Revision: D127012316 Pulled By: xingbowang fbshipit-source-id: e29957fb615915abd98a40f98815205bad14c696
Summary: The RocksDB GitHub repository moved from github.com/facebook/rocksdb to github.com/meta/rocksdb. Nearly all GitHub Actions jobs are gated on `github.repository_owner == 'facebook'` (to keep them from running on forks), so after the move those jobs were skipped on new PRs and pushes, and in the nightly and weekly schedules. This change updates the repository for the new location: * Fix: All `repository_owner` checks in the workflows now compare against 'meta', so the PR, nightly, weekly, and clang-tidy jobs run again. * Change: CI container images now come from ghcr.io/meta/rocksdb_ubuntu, and the Dockerfile build/push instructions match. * Change: Workflow names facebook/rocksdb/* are now meta/rocksdb/*. The Claude review workflow triggers on the pr-jobs workflow by name, so its `workflow_run` trigger is updated to match. (Required status checks are keyed by job name, not workflow name.) * Change: Every github.com/facebook/rocksdb URL in the repo now uses github.com/meta/rocksdb. This covers URLs used by CI and build scripts (third-party-deps release downloads, including DEPS_URL for Java builds; git remotes in setup-upstream and check_format_compatible.sh), the docs site config (`ghrepo`) and navigation, Maven SCM metadata, and links in code comments, docs, HISTORY.md, and blog posts. Historical links are included because GitHub's redirect from the old URL is not dependable for every link: some old issue URLs return 404 while the same issue under meta/rocksdb works. * Change: build_tools/format-diff.sh finds the upstream remote by matching either meta/rocksdb.git or facebook/rocksdb.git, because existing clones still use the old remote URL (which GitHub redirects for git operations). * Remove: The stale CircleCI status badge from README.md. CircleCI is no longer used. * Add: A final step in nightly's build-format-compatible job that fails if https://github.com/facebook/rocksdb stops redirecting to https://github.com/meta/rocksdb (or the old name is taken by another repo). Existing releases (e.g. DEPS_URL in java/Makefile as of v11.12), existing clones, and external links still depend on that redirect, so this alerts the RocksDB team if it breaks. The step runs even when the compatibility test fails. Not addressed here: runner labels (16-core-ubuntu, etc.) and secrets are configured per GitHub org, outside the repo. Any that are missing in the meta org need to be set up there. Pull Request resolved: meta#15508 Test Plan: CI on this PR exercises the main fix: jobs should run instead of being skipped. Manual checks from an unrestricted network: * https://github.com/facebook/rocksdb returns a single 301 to https://github.com/meta/rocksdb, then 200, and `git ls-remote` via the old URL works. Old pull, wiki, blob, and release-download URLs redirect, but old issue URLs for meta#9275, meta#10080, and meta#13293 returned 404 (meta#12038 redirected) even though all four exist under meta/rocksdb. * A CI log showed ghcr.io/meta/rocksdb_ubuntu:22.1 pulling successfully. The new nightly step won't run before merge. The exact curl command it uses printed the expected `200 https://github.com/meta/rocksdb` when run manually. Separately, its script was extracted from the parsed workflow YAML and run against a stub curl: it passes for a redirect to meta, fails for a 404 and for a 200 without redirect, and reports a network failure distinctly. After merge, a manual workflow_dispatch of the nightly workflow can confirm it in CI. Reviewed By: archang19 Differential Revision: D128195050 Pulled By: pdillinger fbshipit-source-id: 3d42aa271b706eec2ef6c1586643056084fc866f
Summary: X-link: meta#15448 - Move the existing global prefetch-memory policy and pending queue into a dedicated scheduler module. - Make every returned `ReadSet` own a configured scheduler. - Keep I/O submission, buffers, completion, and block construction in `IODispatcher`. - Preserve the existing default scheduling and coalescing behavior. Reviewed By: anand1976, xingbowang Differential Revision: D126191480 fbshipit-source-id: 515bbe66dafc26908d26e0e7c7c772d952e56f3f
Summary: X-link: meta#15408 Add bounded rolling MultiScan prefetch windows to `IODispatcher`. Invariants: - Rolling prefetch is effective only for forward async scans with verified file-offset-ordered handles, a nonzero window that fits every uncached block, and no global dispatcher memory limit; otherwise it disables itself and preserves ordinary iterator and prefetch error semantics. - Each SST partitions uncached data blocks into ordered, variable windows whose physical I/O span, including coalesced gaps, is at most `max_prefetch_size`; a gap beyond `io_coalesce_threshold` ends the current window early. - `Prepare` submits the first window, first use of a block in the current window submits the next window, and reaching a queued window waits for that whole window instead of issuing an individual block read while rolling remains enabled. - Rolling consumption is monotonic: forward jumps discard obsolete queued windows, successful portions of partial dispatches are removed from pending bookkeeping, benign absent or drained pending groups are treated as already handled, and failed speculation falls back without duplicate I/O or a rolling-specific status. Reviewed By: anand1976 Differential Revision: D121817105 fbshipit-source-id: 1af83d23634b20cc2938a4c5ad6137fa700a0c7b
…serve_internal_time_seconds Summary: X-link: meta#15457 **Problem:** a remote compaction (`CompactionService` / `DB::OpenAndCompact`) ignores `preclude_last_level_data_seconds` and `preserve_internal_time_seconds`. - With tiered storage, every remote compaction into the last level writes recent (hot-window) data to the last level, which is the cold tier. A local compaction keeps that data in the proximal level. - With `preserve_internal_time_seconds`, a remote compaction zeroes sequence numbers that a local compaction keeps, so their write-time information is lost. - Remote outputs also get no `seqno_to_time_mapping` table property. Later compactions, local ones included, take write times only from their input files, so that data is treated as recent unless other inputs cover it. Files written by remote compactions before this fix stay that way after upgrading. **Root cause:** the worker opens the primary's DB as a secondary. `SanitizeCfOptions()` treats the secondary as read-only and sets both options to 0 ("ignored in read-only DB"). `DBImplSecondary::CompactWithoutInstallation()` builds the compaction from these sanitized options, so `Compaction::EvaluateProximalLevel()` turns off per-key placement and `CompactionJob::Prepare()` computes no preserve window. The existing `CompactionServiceTest.PrecludeLastLevel` forces per-key placement and the cutoff through sync points, so it did not catch this. **Fix:** - `CompactionServiceInput` carries the primary's `preserve_internal_time_seconds` and `preclude_last_level_data_seconds`. The worker sets them on its copy of the CF options before it picks the compaction. - Restoring the preserve window alone would bring back the kBottommostFiles infinite compaction loop for remote compactions. The worker does not know the compaction reason or the primary's DB-wide seqno->time mapping, so it cannot apply the version-carried preserve boundary that a local bottommost-files compaction applies. `CompactionServiceInput` therefore also carries that boundary (`preserve_time_min_seqno_floor`, set only for kBottommostFiles). The worker takes the max with it, the same as a local compaction does. - The options are sent in `CompactionServiceInput` rather than re-read from the OPTIONS file (which the worker already loads before sanitizing) so that they always arrive together with the boundary. With the OPTIONS-file approach, a new worker paired with an old primary would get a preserve window but no boundary, which brings back the loop. - With per-key placement on, the primary and the worker each decide it, and the worker's MANIFEST view can be newer than the primary's input version. The primary now also sends its proximal level, proximal output range type and `keep_in_last_level_through_seqno`. The worker returns `Status::Incomplete` on any mismatch, so the `CompactionService` can fall back to local compaction (the same pattern as the MANIFEST floor check). As a backstop, the primary falls back to local compaction if a result has proximal-level outputs while its own compaction has no proximal level. - Compatibility: the new fields default to 0, and readers ignore unknown fields. So an old primary or an old worker keeps today's behavior. The fix takes effect when both are upgraded. Reviewed By: pdillinger Differential Revision: D126826333 fbshipit-source-id: cb59f0f75601e1cbb1233cb52c173d05dc127b92
…eta#15505) Summary: `periodic_compaction_phase_recovery_percent` (experimental, since 11.10) de-herds periodic compactions across a fleet of DBs sharing their `periodic_compaction_seconds` settings and writing slowly enough, on average, to actually reach it: instead of letting every (DB, column family) come due together, it steers each one toward a stable "preferred phase" within that period. Phasing (re)anchors at DB open and whenever that option or `periodic_compaction_seconds` changes. The "anchor" is the earliest time phasing may pull a compaction to so that enabling phasing cannot stampede a backlog of already-old files all at once. **The problem.** Enabling the option across a production use case showed that the catch-up right after an anchor was still too bunched up. In one deployment (7-day `periodic_compaction_seconds`, recovery percent 33), enabling phasing in a single region pushed the remote periodic compaction rate to about 5.7x normal in the first hour and kept it 3-4x elevated for about 6 hours. (The intended long-term de-herding did work.) **Root cause.** Before this change, a file whose preferred-phase target landed before the anchor but whose natural deadline landed after it was given a stable per-(DB, CF) trigger time picked uniformly in [anchor, natural deadline]. Any one such file is spread out fine, but every file's window *starts* at the anchor, so the windows stack up there. Summed over a fleet whose file ages are spread across the period, the trigger density at time t after the anchor grows like ln(n/t) -- about 3.8x the steady rate in the first 1% of a period, and elevated for a large fraction of a period. Within a single DB that is minor, but a fleet-wide option change (e.g. a dynamic SetDBOptions from a config push) lines up every DB's anchor and so turns each DB's small front-load into one fleet-wide burst. The original simulator missed this because no scenario enabled phasing on a fleet whose files already had ages spread across the period: the upgrade scenarios start from a lockstep herd, and the staggered scenario enables phasing at DB open, before any file exists. **The fix.** Phasing fires a file early by some `early_pull`, at `natural deadline - early_pull`. The old code computed it two ways: a normal file used early_pull = q * offset (q = recovery_percent / 100) while a near-anchor file (preferred phase before the anchor, deadline after) ignored that and was instead dropped at a uniform point in [anchor, natural deadline] -- the spreading branch that caused the spike. This change deletes the spreading branch and computes every file the same way: early_pull = offset * min(q, k * r / n) with `offset` the gap from the natural deadline back to the preferred phase, `n = periodic_compaction_seconds`, `r = natural deadline - anchor`, and `k = 1/2` (`TriggerTime` carries the full derivation). The `min` is the crux: for a file due well after the anchor (large r) it selects `q`, reproducing the old steady-state pull exactly; for a file due soon after the anchor (small r) it scales the pull down toward 0, so near-anchor files stay close to their own deadlines instead of all jumping toward the anchor. For example, with k = 1/2 a file due only 0.1n after the anchor is pulled by at most 0.05 of its offset (it barely moves), whereas a file due a full period or more out gets the full q pull (for the recommended q <= 1/2). Resulting behavior: - **Steady state is unchanged.** The full q pull applies whenever r >= q*n/k, which for q <= 1/2 is every file written at or after the anchor. - **Never before the anchor, never after the natural deadline.** - **Catch-up density is bounded** at about -ln(1-k)/k = 1.39x the steady rate right after an anchor on a uniform-age fleet, for any q. - **Herds stay spread.** DBs sharing one deadline get pulls proportional to their own (differing) offsets, so they don't re-collapse. Capping the pull at q*r instead would hand a herd identical pulls and clump much of it onto one instant. Files already past due at the anchor are unaffected and keep the existing n/4 grid. `k` is fixed at 1/2 rather than tracking `q`. k = max(q, 1/2) is identical for q <= 1/2 (including the recommended 33), but at q = 100 it gives k = 1, which loses the density bound entirely; k = q was rejected because it noticeably slows the first de-herding cycle of a lockstep fleet. The price of fixing k = 1/2 is that for q > 1/2 the first cycle after an anchor (including DB open) spreads only as fast as q = 1/2. Phasing now applies only to `periodic_compaction_seconds` up to 2^31 (~68 years), which keeps the scaled-pull arithmetic within 64 bits (asserted statically); longer periods use the classic deadline. No effect when phasing is disabled (the default). No API or file-format changes. Pull Request resolved: meta#15505 Test Plan: New/changed unit tests in db_compaction_test.cc, both of which fail against the previous implementation: - PeriodicCompactionPhaseTriggerTime: replaced the subcase that asserted the old spread-to-deadline result with hand-computed results near the anchor for recovery percent 10 to 100, including the threshold where the full pull resumes, plus long-period cases: exact results up to the 2^31 limit (including the largest intermediate product) and the classic deadline beyond it. The geometric-convergence subcase is unchanged. - PeriodicCompactionPhaseTriggerTimeRandomized (new, time-seeded): over random (n, recovery percent, phase, file time, anchor) it checks anchor < trigger <= natural deadline and trigger >= the steady-state trigger, with exactly the steady-state result for files written after the anchor (a full period after, for recovery percent > 50). With many files sharing one anchor and uniform ages, no 1%-of-period bin after the anchor exceeds 1.6x its share (expected 1.39x) for recovery percent 10, 33, 50, 67, 100. A herd sharing one deadline 0.62n after the anchor stays spread over >= 0.3n with no clump. Simulation (out-of-tree harness, not part of this change): 1000 real RocksDB instances per scenario, `periodic_compaction_seconds` N = 100s, a real periodic-task scheduler, every compaction recorded; the previous code, k = max(q, 1/2), and k = 1/2 run concurrently with identical per-DB phases, plus a replicate of the previous code as a noise reference. New `midlife_dynamic_r*` scenarios model the production case: file ages uniformly spread, phasing enabled via SetDBOptions at 2.4N. Full interactive HTML (plots per scenario plus a side-by-side table; **fb-internal URL sanitization may require copy-pasting the URL manually**): https://htmlpreview.github.io/?https://github.com/pdillinger/rocksdb/blob/38ba095d48e12b85c9a0ccc397c68a486c07a63a/pc_phase_switchover.html Periodic compaction load relative to an even rate, in `midlife_dynamic_r*`: | recovery % | old, first 0.05N | old, 0.1N-0.2N | new, 0.05N-0.4N | |---|---|---|---| | 33 | 2.8x | 1.2x | ~1.4x | | 50 | 2.7x | 1.8x | ~1.4x | | 100 | 3.2x | 2.2x | ~1.4x | Peak load there (max periodic compactions per DB per N, in N/40 bins) dropped from 3.5 / 3.3 / 3.7 to 2.0 / 1.7 / 1.8 for recovery percent 33 / 50 / 100. The tradeoffs are all in first-cycle de-herding of a lockstep fleet: peak load rose from 3.3 to 3.8 (`upgrade_restart_r33`), 2.6 to 3.8 (`upgrade_dynamic_r50`), and 3.0 to 3.8 (`upgrade_dynamic_r100`), comparable to the 3.6 to 3.8 of `upgrade_dynamic_r33`; convergence at 50 and 100 finishes somewhat later. Cumulative badness was unchanged within noise, or better, in every scenario. Scenarios the change should not affect (`staggered_phased_r33`, `turnup_r33`, `natural_*`, `burst_r0` through `burst_r50`) matched the old code within run-to-run noise. Reviewed By: jaykorean Differential Revision: D128151011 Pulled By: pdillinger fbshipit-source-id: b1b34d532647cd5e2280b9c613e659efb79da20b
Summary: Pull Request resolved: meta#15398 X-link: meta#15398 Why: Applications that populate a cache from explicit-snapshot wide-column reads (`GetEntity` / `MultiGetEntity`) need to know whether a returned entity has already been superseded by a newer committed write, without issuing a second lookup. `D104182397` added `DB::GetWithMetadata()` and `DB::MultiGetWithMetadata()` for plain values, but left entity reads without a metadata counterpart. Previous behavior: `DB::GetEntity()` and `DB::MultiGetEntity()` returned the snapshot-visible `PinnableWideColumns` or `PinnableAttributeGroups`, but always passed `newer_version_present = nullptr` into `DBImpl::GetImpl` and `DBImpl::MultiGetCommon` and could not report a newer point write or covering range deletion. New behavior: experimental C++ `DB::GetEntityWithMetadata()` (virtual column-family overload plus default-CF helper) and `DB::MultiGetEntityWithMetadata()` (multi-CF `PinnableWideColumns*` virtual, single-CF helper, and `PinnableAttributeGroups*` attribute-group virtual) report newer committed `PutEntity`, `Put`, `Delete`, `Merge`, `SingleDelete`, and covering `DeleteRange` operations after an explicit snapshot. As with `GetWithMetadata()` / `MultiGetWithMetadata()`, ordinary primary-DB writes completed before the read starts are guaranteed to be reported, racing writes remain best-effort, and `true` has no false positives. For the attribute-group overload, `MultiGetOutputMetadata` vectors are sized to the total number of attribute groups across `results[0..num_keys-1]` in `(key, attribute_group)` order. Implementation: newer-version tracking in `DBImpl::GetImpl` and `DBImpl::MultiGetCommon` is already result-type agnostic (gated on the read-callback metadata bounds inside memtable, SST, range-tombstone, and row-cache lookups rather than `value` vs `columns`). `DBImpl::GetEntityWithMetadata` and `DBImpl::MultiGetEntityWithMetadata` validate inputs, initialize `OutputMetadata` / `MultiGetOutputMetadata` via shared helpers in `db/db_impl/db_impl_metadata.h`, and pass the requested `newer_version_present` pointer or vector alongside `columns` into `GetImpl` and `MultiGetCommon` while keeping metadata-only state off passive `PinnableWideColumns` and `PinnableAttributeGroup` layouts. Existing `GetEntity()` and `MultiGetEntity()` paths are unchanged. Safety and unsupported cases: tracking is opt-in (`WantNewerVersionPresent()`) and requires an explicit `read_options.snapshot`; when `snapshot == nullptr` or tracking is not requested, reads behave like `GetEntity()` / `MultiGetEntity()` and any engaged `newer_version_present` output stays `false`. Entity reads do not return user-defined timestamps, so `WantTimestamp()` / `WantTimestamps()` returns `Status::NotSupported` on all DB types. With an explicit snapshot and tracking requested, `ReadOptions::read_tier == kPersistedTier`, `DBImplSecondary`, `WritePreparedTxnDB` / `WriteUnpreparedTxnDB` (custom visibility callbacks), and `DB` subclasses that do not override the methods return `Status::NotSupported`, whereas `DBImplReadOnly` and `CompactedDBImpl` return `false` because their in-process views are static. `StackableDB` forwards to the wrapped `DB`. Tests and limitations: 1,328 Buck tests across `db_wide_basic_test`, `db_basic_test`, `db_secondary_test`, and `write_prepared_transaction_test` passed, all 6 per-guard counterfactual mutations fail at least one targeted test, and `db_stress`, `db_bench`, and `db_test` build cleanly. Reviewed By: xingbowang Differential Revision: D125108470 fbshipit-source-id: 1ef367d5696ee6f8217391f64698229f48675d68
…eta#15518) Summary: CI cost reduction and cleanup in the GitHub Actions configuration: * Move `build-macos-cmake` (four test shards on `macos-15-xlarge`) from pr-jobs to nightly. These jobs are among the most expensive per run and rarely catch anything not caught elsewhere. PRs still run `build-macos` (a build without tests) and the macOS Java jobs. The job is unchanged except for using nightly's owner check (it can no longer be selected through pr-jobs' `ONLY_JOB`; use workflow_dispatch on nightly instead). * Remove the `post-steps` composite action and its 36 call sites. It was invoked without an `if:` condition, so GitHub skipped it whenever an earlier step had failed, meaning it only ever ran after successful jobs. Its failure-oriented steps (copying `t/` test logs, uploading core dumps) therefore never did anything. On success it uploaded the gnu_parallel job log (misleadingly named "db-log-file") and gtest XML results, neither of which is consumed by anything, and the macOS Java jobs that call it produce neither. Failing test output is already printed in the job console by `make check`. The `GTEST_OUTPUT` setting in pre-steps existed only to produce the uploaded XML, so it is removed too. Rather than fixing post-steps to run on failure, this change drops it because what it would collect is redundant with the console output. * Bonus: the pre-steps diagnostics step now logs OS and CPU details alongside the existing ulimits, to help diagnose failures that depend on the runner environment. Pull Request resolved: meta#15518 Test Plan: CI on this PR validates the workflow YAML (`make check-workflow-yaml` in check-format-and-targets) and exercises pre-steps and the remaining jobs without post-steps. The relocated `build-macos-cmake` won't run before merge; parsing the workflows confirmed the nightly job is identical to the removed pr-jobs job apart from its owner-check condition. A manual workflow_dispatch of nightly after merge can confirm it runs. Reviewed By: xingbowang Differential Revision: D128353844 Pulled By: pdillinger fbshipit-source-id: d066e6f60bf433ab6dc70aea7e1cd22d7207c8d1
Summary: X-link: meta#15501 Why the change is required: `Cache::CreateStandalone` documents that with `strict_capacity_limit` and no room, `allow_uncharged=true` creates the entry anyway with `GetCharge() == 0`. The secondary cache adapter relies on this whenever it promotes an entry into a full strict primary cache. Under `kFullChargeCacheMetadata` (the default), such entries violated the public API contract. Previous behavior: `LRUCacheShard::CreateStandalone` sets `total_charge = 0` for an uncharged entry, but `LRUHandle::GetCharge` unconditionally subtracted the metadata charge. Debug builds aborted on `assert(total_charge >= meta_charge)`; release builds reported enormous per-handle charges from unsigned underflow, corrupting block-cache byte statistics through `GetUsage(handle)` in `UpdateCacheHitMetrics`. The trigger requires an LRU cache with `strict_capacity_limit`, full metadata charging, pinned entries preventing sufficient eviction, and `allow_uncharged=true`. `kDontChargeCacheMetadata` and HyperClockCache are unaffected. New behavior: Uncharged standalone handles report `GetCharge() == 0`, preventing the debug assertion failure and incorrect release-build byte statistics. `GetUsage(handle)` and `ApplyToHandle` also receive the correct charge. Total cache usage accounting is unchanged: uncharged handles add no usage on creation and subtract none on release. Implementation: `LRUHandle::GetCharge` returns zero for standalone handles with the named zero-charge sentinel, retaining the existing assertion for other handles. Added a release note and simplified the direct API regression test. `FullCapacityTest` now exercises both metadata charge policies and verifies zero per-handle usage when strict capacity forces secondary-cache promotion to use an uncharged standalone handle. Its 700-byte values leave room for metadata while ensuring two entries still exceed the 1024-byte capacity. Safety, compatibility, and limitations: Only the previously broken standalone, uncharged case changes its returned charge. There are no API signature, ABI, or serialized-format changes. The added check runs on every LRU `GetCharge` call, for every handle, including block-cache hits through `UpdateCacheHitMetrics` and `GetUsage(handle)`. Expected overhead is negligible because `im_flags` is stored near `total_charge` and the branch is predictable; this is an assessment of the code, with no performance benchmark performed. Reviewed By: xingbowang Differential Revision: D126972213 fbshipit-source-id: 595d8771b0e57f20c12314e00c2aa2d7d2965a34
Summary: Pull Request resolved: meta#15512 `OptionTypeInfo` parses an `OptionType::kEncodedString` field by hex-decoding its value, but `ParseOptionHelper()` ignored the result of `Slice::DecodeHex()` and always reported success. A value with an odd number of digits left the field unchanged, which is empty in a freshly constructed struct. A value with a non-hex digit kept only the bytes decoded before that digit. In both cases `Parse()` returned OK. Only serialized formats that RocksDB itself writes as hex use `kEncodedString`: the remote compaction input and result (`CompactionServiceInput::Read()` and `CompactionServiceResult::Read()`), and `TableProperties::Parse()`, which the compaction result and the `PreparedFileInfo` deserialization use. So a corrupted or truncated serialized string was accepted with an empty or shortened value in fields such as the column family name, an input file name, a compaction range key, an output file key, or a file checksum, with no sign that the input was bad. The custom blob-metadata parser in `compaction_service_job.cc` already rejects bad hex with `InvalidArgument`. Fix: decode into a local string and return false when `DecodeHex()` fails, so `OptionTypeInfo::Parse()` returns `InvalidArgument("Error parsing:", <field>)`. The field stays unchanged on failure, as with the `StringMap` parser. Compatibility: `Serialize()` always writes valid hex, and valid hex in either letter case parses as before. Only malformed input changes behavior: it now fails instead of yielding an empty or partial value. `CompactionServiceInput::Read()` and `CompactionServiceResult::Read()` still ignore unknown fields. Reviewed By: jaykorean Differential Revision: D128131523 fbshipit-source-id: 49e2840758b96e24c910355b0d969092e760a222
Summary: as title, after observing bad agent behavior X-link: meta#15476 Reviewed By: xingbowang Differential Revision: D127492952 Pulled By: pdillinger fbshipit-source-id: 94cfe2766433a7e59c05a79e189d53b46facc292
Summary: Pull Request resolved: meta#15499 X-link: meta#15499 ## Summary A secondary can successfully open without installing a new `Version` when the SSTs referenced by its initial MANIFEST snapshot disappear during primary compaction. On a later MANIFEST switch, catch-up selects the original empty `Version` as its replay base. That `Version` is still `current()`, so an overly strict assertion in `BaseReferencedVersionBuilder` aborts assertion-enabled builds. Allow that valid base with one explicit-base `BaseReferencedVersionBuilder` constructor that takes and releases its own reference. Ordinary updates pass `current()`; MANIFEST replay passes the retained empty `Version`. This replaces the overload-specific inequality assertion and moves reference acquisition into the constructor, before the old builder is released. Replay still constructs a separate candidate `Version`; reference counts, release-build replay behavior, public APIs, and persisted formats are unchanged. ## Example In this example, `V0` is the secondary’s initial empty `Version`, and `A`/`B` are SST files. A MANIFEST begins with a live-file snapshot, followed by later edits. The log names and SST layout below are simplified. ```text Primary Secondary MANIFEST-1 snapshot: AddFile(A) opens MANIFEST-1; current = V0 (empty) roll to MANIFEST-2: snapshot: AddFile(A) compact A -> B: append ONE edit to MANIFEST-2: DeleteFile(A), AddFile(B) purge A replay MANIFEST-1 snapshot: A is missing no candidate installed current still V0 Catch-up sees CURRENT -> MANIFEST-2: replay base = V0 (empty); current() = V0 old constructor: assert(version_ != cfd->current()) -> abort With the unified constructor: Ref(V0), then replay MANIFEST-2 from the beginning: snapshot AddFile(A) -> A missing edit {DeleteFile(A), AddFile(B)} -> live file set {B} SaveTo() a separate V1, then install -> current() = V1 {B} ``` The new MANIFEST is rebuilt from the retained empty `V0`. Here `V0` is also current because initial recovery installed no candidate. The builder keeps `V0` referenced while replay saves the reconstructed file set into a separate `V1`, then installs `V1`. ## CI test fix Also reorder `SyncPointAbortHelper::CleanupAndWait()` to call `WaitForAbortCompletion()` before `CleanupSyncPoints()`, fixing a pre-existing race exposed by CI. This keeps the abort dependency active until the worker has observed the abort and exited. Reviewed By: pdillinger Differential Revision: D127928276 fbshipit-source-id: 3e437328ed5afc8c9bf53bb97cbce0dfe2560097
Summary: Pull Request resolved: meta#15521 The fbcode copy of folly is a dirsync mirror of xplat/folly that is being removed; once it is gone, `fbcode//folly` targets will no longer resolve. Hand-written BUCK files are migrated by `migrate_fbcode_folly.py`; generated BUCK files have to be fixed in their generator. Rewrites the 16 `rocksdb_lib` folly deps in `internal_repo_rocksdb/repo/buckifier/buckify_rocksdb.py` to `fbsource//xplat/folly...` and regenerates `internal_repo_rocksdb/repo/BUCK` with `python3 buckifier/buckify_rocksdb.py`; only the 16 labels change. `fbcode/rocksdb/src/BUCK` is a release snapshot updated by the periodic RocksDB release process and is not changed here. ___ landed-with-radar-review Differential Revision: D128219553 fbshipit-source-id: 259fd54bf827a75e389c7d9be8275145f55515a8
Summary: Pull Request resolved: meta#15493 X-link: meta#15493 **What:** with `ReadOptions::async_io`, MultiGet now reads direct-IO files asynchronously too. Before, `RetrieveMultipleBlocks` sent them to the synchronous `MultiRead`, so a MultiGet over direct-IO files got no read overlap at all. Async reads are now also charged to the rate limiter, like sync reads. **How:** - `AsyncFileReader::MultiReadAsyncImpl`: for a direct-IO request without a scratch buffer, `RandomAccessFileReader::ReadAsync` allocates the aligned buffer and hands it back in the request's `fs_scratch`, with `result` pointing into it. That uses the existing `aligned_buf` support in `ReadAsync`, so the file system only sees aligned requests. If `ReadAsync` reports NotSupported, the synchronous fallback reads through an `AlignedBufferAllocationContext` and moves that buffer into `fs_scratch` the same way. - `RandomAccessFileReader::ReadAsync` charges the read rate limiter at `rate_limiter_priority` for every read it submits: the aligned length for direct IO, the request length otherwise. It charges once the file system accepts the read, because every caller falls back to `Read` on NotSupported and `Read` charges for itself. `MultiRead` and `ReadAsync` share the charging loop (`ChargeRateLimiterForRead`). - `RetrieveMultipleBlocks`: direct-IO files now take `MultiReadAsync` like buffered files. The requests (and so their `fs_scratch`) live until the blocks are parsed. **Behavior changes:** - Direct IO: each block request is aligned and read on its own; requests aren't merged. Adjacent blocks of one file can read an overlapping aligned page twice. - Async reads (async_io MultiGet and iterator prefetching), buffered or direct, are now charged to `DBOptions::rate_limiter`. Before, `ReadAsync` charged nothing. Reviewed By: archang19, anand1976 Differential Revision: D127688099 fbshipit-source-id: cfab475b6a27cb2a78b610144f9f8436c3b8a807
chore: sync with upstream meta/rocksdb
x-at-01
force-pushed
the
ctrip-main
branch
2 times, most recently
from
October 31, 2026 10:19
e1d351d to
63a046f
Compare
- LocalTimeR: avoid calling localtime_r to prevent __tz_convert deadlock - Add C API rocksdb_sst_file_metadata_get_file_create_time - Fix GetInfoLogFiles using wrong path for log file prefix - Fix compaction picker covering_the_whole_range dead logic - Support blob list garbage collection (blob_file_set tracking and C APIs) - Update macOS deployment target to 13.4 for C++20 libc++ compatibility
x-at-01
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
facebook/rocksdb的main分支最新代码（a4ee7556e1，RocksDB v11.12+ / C++20）。CompactFilterFactory与TablePropertiesCollectorFactory的 C API 绑定（rocksdb_compactionfilterfactory_create、rocksdb_tablepropertiescollectorfactory_create、rocksdb_options_add_table_properties_collector_factory、rocksdb_property_int_with_cf、rocksdb_HLL_create）GetApproximateSizes的SizeApproximationOptions增强，用于处理极小的边界 SSTDBImpl::SelectLevelToCompact自定义层选择与相关指标WriteOptions::skip_filters/disable_AutoCompactions自定义钩子合并方式要求