mirror of
https://github.com/facebook/rocksdb.git
synced 2026-10-24 19:59:35 +03:00
fix_codex_review_command
3182 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Xingbo Wang
|
cbd61a3165 |
Add parser for raw table iterator keys (#14726)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14726 Add SstFileReader::ParseTableIteratorKey() so callers of NewTableIterator() have a public way to decode raw table keys without duplicating RocksDB internal-key layout. The implementation delegates to the existing public ParsedEntryInfo parser using the reader comparator. Reviewed By: pdillinger Differential Revision: D104584393 fbshipit-source-id: 98e21c4d6676fbba69e533376b3da67539dd8fad |
||
Peter Dillinger
|
795f3bd61f |
Fix check-sources.sh non-ASCII check and remove non-ASCII from sources (#14729)
Summary: The non-ASCII character check in check-sources.sh used git grep -P (Perl regex), which requires git compiled with PCRE support. On systems without it, the command fails with exit code 128, which is != 1 (no match), so the check always reported a violation -- effectively dead. Even in CI where git has PCRE2 support, the check was silently broken: git grep -P uses PCRE2 in UTF mode by default, which interprets [\x80-\xFF] as a Unicode codepoint range (U+0080 to U+00FF). Characters like em-dash (U+2014), arrows (U+2192), and math symbols (U+2248, etc.) fall outside that range and were not detected. Only Latin-1 Supplement characters (U+0080-U+00FF) would have been caught. Replace with LC_ALL=C git grep using bash $'[\x80-\xff]' literal byte range, which works with basic regex in the C locale, and replace all non-ASCII characters in non-excluded source files: - em-dash to -- - arrow to -> - math symbols to ASCII equivalents (~=, <=, >=) - box-drawing characters to ASCII art Also exclude .github/ from the check, as scripts there can use non-ascii without disrupting RocksDB builds on non-UTF-8 systems. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14729 Test Plan: manual / CI (make check-sources passes clean) Reviewed By: hx235 Differential Revision: D104692574 Pulled By: pdillinger fbshipit-source-id: 1d884c21056dcd83558b825a04b867f1c08e3f45 |
||
Josh Kang
|
e07ccc3528 |
block GetCreationTimeOfOldestFile on async file open completion (#14723)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14723 ### Context `GetCreationTimeOfOldestFile()` assumed `max_open_files` = -1 meant every live SST had a pinned table reader. That is not true with `open_files_async`: recovery intentionally skips loading table files, `DB::Open()` returns, and `BGWorkAsyncFileOpen()` pins readers later. Any caller — e.g. fb_rocksdb's daily report at `FbRocksDb.cpp:1025` — invoking the API in the window between `DB::Open` returning and the background opener completing trips a debug assert. Reported in https://fb.workplace.com/groups/rocksdb/permalink/31668956482726231/. Removing the assert alone is insufficient. For legacy DBs whose manifest does not carry `file_creation_time`, `FileMetaData::TryGetFileCreationTime()` falls back to the pinned reader; with no reader, it returns `kUnknownFileCreationTime` and the function silently returns 0 (the "info unavailable" sentinel). The caller cannot distinguish "no info" from "raced with async open." ### Changes - Remove the invalid debug assert in `Version::GetCreationTimeOfOldestFile`. - Add a private helper `DBImpl::WaitForAsyncFileOpen()` that blocks on `bg_cv_` while `bg_async_file_open_state_ == kScheduled`. The synchronization machinery (`bg_async_file_open_state_` + `bg_cv_`) already exists — the destructor wait loop in `db_impl.cc:674-687` uses the same pattern. The helper is a no-op when `open_files_async = false`, and bails on `shutting_down_` so `DB::Close()` is not blocked by an in-flight caller. - Call `WaitForAsyncFileOpen()` at the top of `DBImpl::GetCreationTimeOfOldestFile()` (inside the `max_open_files == -1` branch). - Document the blocking behavior in `include/rocksdb/db.h`. - Replace the regression test with one that uses a `DBImpl::WaitForAsyncFileOpen::BeforeWait` sync point: spawn a thread that calls `GetCreationTimeOfOldestFile`, deterministically confirm it blocks inside the wait, release async open, confirm the caller wakes with the real value. ### Potential Followups (not included here) - Apply the same wait to `GetLiveFilesMetaData` and `GetColumnFamilyMetaData` — both zero out `oldest_ancester_time` / `file_creation_time` in the SST metadata they return during the async-open window (`db/version_set.cc:7877-7878`, `2090-2091`, `2172-2173`). - Address compaction-picker effects: TTL/periodic file selection (`db/version_set.cc:4039`, `4092-4094`), bottommost over-marking (`db/version_set.cc:4700-4701`), FIFO TTL/temperature pickers (`db/compaction/compaction_picker_fifo.cc:105-107`, `167-170`, `401-411`), and tiered-compaction output time inheritance (`db/compaction/compaction.cc:981`, `1000`). - Harden `FbRocksDb.cpp:1025` to check `status.ok()` instead of `status.code() != kNotSupported`. Reviewed By: mszeszko-meta Differential Revision: D104285992 fbshipit-source-id: ea46375ea1b3ba77fe6b548071aee1101ac0da77 |
||
Anand Ananthabhotla
|
c734b7cc60 |
Add reuse_manifest_on_open DBOption (#14704)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14704 Add an immutable DBOption `reuse_manifest_on_open` (default false). When enabled, `DB::Open` can keep using the recovered MANIFEST for the first post-open metadata update instead of rebuilding a fresh MANIFEST, which can reduce warm-open latency for DBs whose MANIFEST is expensive to regenerate. Reuse is still best-effort. If RocksDB cannot safely resume appending to the recovered MANIFEST, it falls back to the existing fresh-MANIFEST path. The option is also disabled under `best_efforts_recovery`. This diff also teaches the reopened MANIFEST writer to adopt the existing file size before appending, documents the small-`max_manifest_file_size` caveat for the reused path, and keeps the full warm-reopen composition working with `optimize_manifest_for_recovery`. Reviewed By: hx235, pdillinger Differential Revision: D103568447 fbshipit-source-id: f4f5c35ea3ef0b80a0d52d94be40c6bd11505999 |
||
|
Anand Ananthabhotla
|
02a2b3501d |
Add optimize_manifest_for_recovery DBOption (#14702)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14702 Add a mutable DBOption `optimize_manifest_for_recovery` (default false) as a temporary rollout / kill switch for warm-reopen MANIFEST optimizations. In this diff, enabling the option lets recovery skip MANIFEST updates during `DB::Open` when the recovered state is already reflected on disk, which reduces metadata appends after a clean shutdown and can lower warm-reopen latency on storage where MANIFEST appends are expensive. If the option is disabled, RocksDB follows the existing recovery path unchanged. The optimization is disabled under `best_efforts_recovery`, where recovery intentionally rewrites metadata as part of salvage, and the option is mutable so later diffs in this stack can share the same rollout knob. Reviewed By: pdillinger, hx235 Differential Revision: D103568448 fbshipit-source-id: 9ec930343e434f1bee6130bcdbd7738dddd92b6d |
||
Maciej Szeszko
|
c25dcbccba |
Fix GCC 16 warning in CacheItemHelper constructor (#14713)
Summary:
This fixes a GCC 16 unity-build failure in `Cache::CacheItemHelper`. The no-secondary-cache constructor delegated to the full constructor while passing this as `without_secondary_compat`. GCC 16 reports that pattern as `-Wmaybe-uninitialized` because the delegated constructor receives a pointer to the object under construction and its debug assertions dereference it. Since RocksDB builds with `-Werror`, the warning is promoted to a build error. We see it now because the unity job uses `gcc:latest`, which now has `GCC 16.1.0`. The code pattern itself dates back to PR https://github.com/facebook/rocksdb/issues/11299 /
|
||
|
Josh Kang
|
b874d2546e |
Preserve published-boundary visibility during flush/compaction (#14681) (#14681)
Summary: See SC crash test failure 2729181374202803470. Flush/compaction retention was protecting only explicit snapshots, while readers without an explicit snapshot were still bounded by GetLastPublishedSequence(). That mismatch becomes observable in WRITE_COMMITTED when commit_bypass_memtable runs: a newer version can already be installed into WBWI-backed immutable memtables before SetLastSequence() publishes it. This can happen with two-write-queue enabled OR disabled. Example setup: - K@P = "old_value" is the current published version of key K. - K@U = "new_value" is written later with U > P. - commit_bypass_memtable ingests K@U into immutable memtables, but publication is still at P. - flush starts in that window and builds its snapshot context. Before this change, if there was no explicit snapshot at P, flush/compaction was free to collapse K@P under K@U. A reader at the published boundary P then saw neither version: K@P had been discarded, while K@U was still too new, so Get(K) returned NotFound. A simple solution is to add a "fake" snapshot boundary for pessimistic write committed txns. NOTE: A behavioral side effect is that this will prevent a merge operand at snapshot seqno from being filtered via compaction filter because of the new managed snapshot. But this keeps it aligned with the behavior that write prepared and write unprepared have. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14681 Test Plan: Regression test that fails without fix, specifically K@P gets deleted and a snapshot read returns NotFound, when it should exist. Reviewed By: hx235 Differential Revision: D102856943 Pulled By: joshkang97 fbshipit-source-id: 46cb3936e9d4e771966d39fd0d15436335bf6ccf |
||
|
Josh Kang
|
a12bd7d641 |
ExternalTable support FS (#14629)
Summary: Extends the `ExternalTable` plug-in interface so external table implementations have access to the same `FileSystem` RocksDB itself uses. Previously only `ExternalTableOptions` (read path) had `fs`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14629 Test Plan: - Existing tests pass Reviewed By: anand1976 Differential Revision: D101384103 Pulled By: joshkang97 fbshipit-source-id: cade6f4699337e07bdd4875d070f629058fd6795 |
||
Danny Chen
|
2e7cf42cda |
Add MANIFEST_VALIDATION_FAILURE_COUNT statistic (#14657)
Summary: CONTEXT: The manifest validation on close feature (verify_manifest_content_on_close) detects corruption but does not increment any statistics counter, making it harder to monitor in production. WHAT: Add a new ticker MANIFEST_VALIDATION_FAILURE_COUNT that is incremented each time content validation detects manifest corruption during DB::Close(). The counter fires per corruption detection, so it can increment up to 2 times per close (once on initial check, once after rewrite attempt). Updated all existing manifest validation tests to verify the counter value. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14657 Test Plan: - All 7 manifest validation tests pass with new stat assertions - 5x repeat with COERCE_CONTEXT_SWITCH=1 shows no flakiness - Full version_set_test suite (212 tests) passes Reviewed By: anand1976 Differential Revision: D102404260 Pulled By: dannyhchen fbshipit-source-id: 21a0aa1ad8de12a935caf5642e41ccf2a47b46d9 |
||
Yoshinori Matsunobu
|
3cdf942192 |
Make ParseCompressionNameForDisplay manager-aware (#14658)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14658 Follow up to D101463511. Add a hook and a manager-aware overload of so custom CompressionManagers can provide human-readable names for custom compression types while preserving the existing generic fallback when no compatible manager is available. Reviewed By: pdillinger Differential Revision: D102201365 fbshipit-source-id: 0c7456bb9db2e54927a4349d12c035fc8b5ad562 |
||
anand76
|
ff16ff23ed |
Update version to 11.3.0 for 11.2 release (#14638)
Summary: - Bump version.h from 11.2.0 to 11.3.0 - Add `11.2.fb` to `check_format_compatible.sh` - Cherry-pick HISTORY.md from `11.2.fb` - Update folly hash in `folly.mk` - Delete `unreleased_history/` note files Part of 11.2 release workflow. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14638 Reviewed By: xingbowang Differential Revision: D101893027 Pulled By: anand1976 fbshipit-source-id: a4d0d355a9dca6199228f91ba08665524e6c8f5b |
||
|
Josh Kang
|
03e5a0b9a8 |
Fix range tombstone conversion + ingest sst and re-enable crash tests (#14654)
Summary:
Fixes a correctness bug in read-path range-tombstone synthesis when it races with `IngestExternalFile`. The synthesis path could insert a tombstone into the active memtable at a snapshot's sequence number, while ingestion installed an L0 SST at `LastSequence + 1` — a higher seqno than the synthesized tombstone. This breaks the main assumption of range tombstone reads that all lower levels have lower seqno.
The fix introduces a per-CF `port::RWMutex` (`ColumnFamilyData::ingest_sst_lock_`) plus a per-memtable `ingest_seqno_barrier_`. Ingestion takes the read lock and range tombstone synthesis **tries** to take a write lock.
If iterator lock is successful, then we have a new updated barrier seqno that we can validate the iterator seqno against. An added benefit is we no longer need to gate against empty memtable. This was originally added as an easy fix to prevent memtables from being inserted into while ingestion was happening.
## The bug, by example
`ReadPathRangeTombstoneTest.NewerPointInOlderFileStillVisible` (`db/db_iterator_test.cc:6929`):
1. L0 file `b@1, c@2, d@3`, then L0 file `Delete(b)4, Delete(c)5`.
2. Active memtable: `Put(z)6`. Snapshot taken at seq 6.
3. `IngestExternalFile({c → "vc_live"})` → installed at L0 with seq 7.
4. Iterator at snap 6 walks the deletion run and synthesizes `[b, d) @ seq 6` into the active memtable via `MemTable::AddLogicallyRedundantRangeTombstone`.
5. `Get("c")` at the latest snapshot: memtable returns covering tombstone (seq 6), `Version::Get` short-circuits, **never reads `c@7`** — returns `NotFound` instead of `"vc_live"`.
The invariant `Version::Get` relies on (memtable seqs ≥ any L0 seq for the same key) is broken because synthesis writes at the *snapshot's* seq while ingestion writes at `LastSequence + 1`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14654
Test Plan:
- Updated regression test
- Re-enable crashtests and manually run
| Flavor | Jobs |
|---|---:|
| `fbcode_blackbox_crash_test` | 200 |
| `fbcode_whitebox_crash_test` | 30 |
| `fbcode_asan_blackbox_crash_test` | 30 |
| `fbcode_tsan_blackbox_crash_test` | 30 |
| `fbcode_crash_test_with_atomic_flush` | 30 |
| `fbcode_crash_test_with_wc_txn` | 30 |
| `fbcode_crash_test_with_ts` | 30 |
| **Total** | **380** |
Reviewed By: xingbowang
Differential Revision: D102044512
Pulled By: joshkang97
fbshipit-source-id: 0c69187595edc5a5fa80be24bffdba710a92e56e
|
||
|
Yoshinori Matsunobu
|
763401b595 |
Add ParseCompressionNameForDisplay API (#14637)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14637 Add public API ParseCompressionNameForDisplay() to render TableProperties::compression_name in human-readable form for both legacy and format_version >= 7 SST metadata, including BuiltinV2/compression-manager encodings and filtered no-compression markers for display. Reviewed By: xingbowang Differential Revision: D101463511 fbshipit-source-id: 51c44850183cb9757644a323376844adffc7a8c2 |
||
|
Xingbo Wang
|
6ff96384dd |
db_iter: eagerly resolve blob-backed iterator columns (#14632)
Summary:
Restore the iterator contract for blob-backed wide-column entities by eagerly materializing all blob-backed columns before a `DBIter` entry is exposed as valid. Also capture similar error inside compaction filter V4, so that the error is captured a surfaced up like older compaction filter V3.
## Problem
Lazy wide-column blob resolution let `DBIter` report `Valid() == true` before all columns were actually prepared. Callers could read `value()` successfully, then hit a blob-read failure in `columns()`, which flipped the iterator to invalid after the entry had already been observed as valid.
## Solution
- Restore back to old behavior.
- Resolve and materialize all blob-backed wide columns during iterator positioning.
- Keep `columns()` as a pure accessor once `Valid()` is true.
- Add coverage for eager blob resolution on iterator scans.
- Add coverage for blob resolution failures invalidating the iterator before the entry is exposed.
- Capture the same error in compaction filter and set db to bg error status if the error is not retriable.
## Next step
If we want lazy iterator resolution in the future, we will need an API that can surface lazy resolution failures explicitly instead of hiding I/O and status transitions inside value() or columns().
## Testing
- `make -j14 db_wide_blob_direct_write_test`
- `timeout 60s ./db_wide_blob_direct_write_test --gtest_filter='*DirectWriteIteratorValueScanEagerlyResolvesBlobColumns:*DirectWriteIteratorBlobResolutionErrorInvalidatesEntry'`
## Task
T265294130
# Bonus fix
## Non-compaction blob filtering fixes
### 1. Flush-time wide-entity lazy blob resolution now works correctly
Blob direct-write can leave wide-entity blob references in memtables before flush. Flush already runs through `CompactionIterator`, but outside a real compaction it did not have blob read support. As a result, `FilterV4` lazy resolution on flush could fail immediately with
`NotSupported("Blob fetcher not available")` instead of either:
- resolving the blob successfully, or
- surfacing the real blob read error and failing the flush
This stack fixes that by plumbing blob read support into non-compaction table-file creation. `BuildTable()` now gives `CompactionIterator` enough context to construct a `BlobFetcher` for flush/recovery table building, including write-path fallback for direct-write blob files
that are not yet manifest-visible.
With that in place:
- flush-time `FilterV4` can lazily resolve blob-backed wide columns
- if resolution fails, the failure is latched and flush fails after `FilterV4()` returns
- `bg_error` is set instead of silently preserving the entry
### 2. Plain blob fallback outside compaction now uses the resolved user value
For plain blob-backed values, `FilterBlobByKey()` is allowed to return `kUndetermined`, which is supposed to fall back to the normal value-based filter path. That fallback worked in compaction, but the non-compaction `BuildTable()` path still treated plain blob indexes as
unsupported/corrupt because it could not read the blob.
This stack fixes that behavior for flush/recovery table-file creation:
- when `FilterBlobByKey()` returns `kUndetermined`, RocksDB now eagerly resolves the plain blob value
- `FilterV2`/`FilterV3`/`FilterV4` then see the actual user value as `ValueType::kValue`, not the `BlobIndex` encoding
- legacy filters therefore behave the same way outside compaction as they already do inside compaction
Wide-column entities still use the `FilterV4` lazy resolver path. The plain-blob fix is specifically about restoring the documented fallback behavior for blob-backed `kValue` records outside compaction.
## Why this matters
Without these last two commits, flush-time filtering still had two inconsistent behaviors:
- wide-entity lazy resolution could not actually read blob-backed columns in the direct-write case
- plain blob-backed values could not fall back to normal value-based filtering outside compaction
So even after fixing iterator validity and compaction error propagation, non-compaction table-file creation still had remaining contract holes. These commits make flush/recovery behave consistently with compaction.
## Coverage added
The tests added in these commits cover:
- flush-time plain-blob `FilterV3` fallback using the resolved user value
- flush-time plain-blob `FilterV4` fallback using the resolved user value
- flush-time direct-write wide-entity lazy resolver success
- flush-time direct-write wide-entity lazy resolver failure on missing blob file, including flush failure and `bg_error` latching
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14632
Reviewed By: anand1976
Differential Revision: D101425233
Pulled By: xingbowang
fbshipit-source-id: 695b2df5189033d30309b35849815e31fd965664
|
||
|
anand76
|
4bc0bff0e4 |
Mark IODispatcher as experimental (#14630)
Summary: As titled Pull Request resolved: https://github.com/facebook/rocksdb/pull/14630 Reviewed By: xingbowang Differential Revision: D101383049 Pulled By: anand1976 fbshipit-source-id: 19fb1c27bee465e691412a30681f9a21c70672f7 |
||
|
Josh Kang
|
790294f3ad |
Disallow table_filters entirely when range tombstone conversion is enabled (#14618)
Summary: The prior fix (https://github.com/facebook/rocksdb/issues/14586) disabled read-path range tombstone synthesis at the `DBIter` level when `table_filter` was set. However, synthesized tombstones persist in the memtable beyond the lifetime of the iterator that created them. A prior unfiltered iterator can synthesize a range tombstone that then silently affects a subsequent filtered iterator's results — the filtered scan sees the tombstone but not the SSTs it was derived from, allowing hidden SST state to corrupt the filtered view. See failed crash test in T264151327. This PR takes the stronger approach of rejecting iterator creation outright (`InvalidArgument`) when `ReadOptions::table_filter` is used on a column family with `min_tombstones_for_range_conversion > 0`. This eliminates the entire class of interaction bugs between the two features rather than trying to suppress conversion in individual code paths. ## Example - `min_tombstones_for_range_conversion = 2` - L1 SST_keep: `Put(a), Put(b), Put(c)` (`c` is the live boundary) - L0 SST_dels: `Delete(a), Delete(b)` (2 contiguous tombstones, newer) ### Step 1 — Iterator A (no filter) A walks `Del(a) Del(b)` (2 contig) then `c` (live). Synthesizes range tombstone `[a, c)` into the active memtable. ### Step 2 — Iterator B (`table_filter` skips SST_dels) B's filter excludes SST_dels, so `a, b` from SST_keep should appear live. But the memtable now holds range tombstone `[a, c)` from step 1, which applies to B regardless of `table_filter`. B sees only `c` — **a, b are silently hidden**. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14618 Test Plan: - `TableFilterNotAllowed` test validates the rejection path end-to-end: unfiltered scan synthesizes the tombstone, filtered iterator is rejected with `InvalidArgument`, and the full-DB view remains correct via `VerifyIteration`. - Crash test sanitization ensures `db_stress` won't hit the incompatible configuration. - Run crash test sanitization unit test: `python3 tools/db_crashtest_test.py` - Run iterator tests: `make db_iterator_test && ./db_iterator_test --gtest_filter="*TableFilterNotAllowed*"` Reviewed By: xingbowang Differential Revision: D100873156 Pulled By: joshkang97 fbshipit-source-id: dd0d123d94b1bfda2a4a5040681fcc7cf14f760e |
||
|
Josh Kang
|
9886d52fc4 |
Disable range tomb conversion when using legacy prefix iterator (#14625)
Summary:
There is an assumption in the code that within a given prefix all keys can be seen after a seek to that prefix. The problem occurs when there is a Next() followed by a Prev() back into the same prefix. The merging iterator actually uses SeekForPrev during Prev for non-child iterators, and as a result changes the prefix being used. Now that we are back to our original prefix, the view is incomplete.
The fix is to simply disable range tombstone conversion in this legacy prefix mode.
### Example
Use a 1-byte prefix extractor, so the prefix is just the first character.
Think of the merged iterator as combining two children:
- Child A: `b8`
- Child B: `b7, c1`
Global sorted order is:
```text
b7, b8, c1
```
So:
- predecessor of `c1` should be `b8`
- predecessor of `b8` should be `b7`
## Sequence
Start in legacy prefix mode:
- `total_order_seek = false`
- `prefix_same_as_start = false`
Now do:
1. `Seek("b8")`
2. `Next()`
3. `Prev()`
## What should happen
```text
Seek("b8") -> b8
Next() -> c1
Prev() -> b8
```
Because in the global order, `b8` is immediately before `c1`.
## What the buggy iterator did
```text
Seek("b8") -> b8
Next() -> c1
Prev() -> b7
```
So it skipped `b8`.
## Why it happens
When `Prev()` switches direction, the merging iterator tells all non-current
children to `SeekForPrev(current_key)`.
At that moment:
- current key is `c1`
- so the other children get `SeekForPrev("c1")`
But in legacy prefix mode, those child seeks are allowed to use prefix
filtering. So a child that only contains `b*` keys can effectively say:
```text
"I was asked for prefix c; I do not have prefix c."
```
That child drops out instead of returning its real global predecessor `b8`.
Then the current child simply moves back from `c1` to `b7`, and the merged
result becomes `b7`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14625
Test Plan: Update unit tests to disable feature.
Reviewed By: xingbowang
Differential Revision: D101259748
Pulled By: joshkang97
fbshipit-source-id: 23e19c470f931406f96a34a7baebca89e91c7e39
|
||
|
Anand Ananthabhotla
|
1cc216a45b |
Fast file open: retrieve, persist, and pass file open metadata (#14596)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14596 When DBOptions::fast_sst_open is enabled, RocksDB retrieves opaque file system metadata for SST files after flush, compaction, and external file ingestion via FSRandomAccessFile::GetFileOpenMetadata(). This metadata is persisted in the MANIFEST using a new forward-compatible NewFileCustomTag (kFileOpenMetadata = 17), and passed back to the file system via FileOptions::file_metadata on subsequent file opens. This accelerates DB open time on remote storage systems by allowing the file system to skip expensive metadata RPCs. The feature is gated by DBOptions::fast_sst_open (default false). Everything works seamlessly regardless of the option setting, file metadata support, or presence/absence of the metadata in the MANIFEST. The MANIFEST change is backward compatible - older RocksDB versions safely ignore the new tag. Reviewed By: xingbowang Differential Revision: D100220973 fbshipit-source-id: f52de9dd853a50653b3297ab4a37a868fe41cc04 |
||
|
Peter Dillinger
|
f3f7feba92 |
Document Flush(), Sync(), and Fsync() contracts; remove dead enum (#14590)
Summary: Flush(), Sync(), and Fsync() on WritableFile and FSWritableFile were essentially undocumented. Added comprehensive API contracts covering durability guarantees (process crash vs. power failure), data readability after Flush(), the implication relationship (Sync implies Flush), and thread-safety (referencing IsSyncThreadSafe()). The contracts are written to be implementation-agnostic, avoiding assumptions about OS-level mechanisms, so they apply equally to local, remote, and virtual/wrapper filesystem implementations. Several such implementations have been audited for adherence. Note that WritableFileWriter currently calls FSWritableFile::Flush() UNNECESSARILY for cases like writing SST files. This should be optimized away in follow-up assuming this interpretation of the contract is agreed upon. Also removed the dead FileSystem::WriteLifeTimeHint enum, which was never referenced anywhere. The actively used enum is Env::WriteLifeTimeHint. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14590 Test Plan: Documentation-only change to public headers, plus removal of an unused enum with no references. No functional changes. Verified no compilation errors with existing tests. Reviewed By: xingbowang Differential Revision: D100183288 Pulled By: pdillinger fbshipit-source-id: 7f1982c41e09fe39896dbc6cc328316be559ec4a |
||
|
Peter Dillinger
|
b7395b3fa4 |
Allow Compressor to override parallel_threads for SST building (#14580)
Summary:
Add Compressor::GetRecommendedParallelThreads() virtual method so that the compression subsystem can influence the parallel compression thread count used when building SST files. The base Compressor returns 0 (no opinion), while built-in compressors (via CompressorBase) return the parallel_threads value from their CompressionOptions. This gives CompressionManager a clean mechanism to override parallel_threads by customizing the CompressionOptions passed to GetCompressor() in its GetCompressorForSST() implementation.
The table builder now reads the thread count from the compressor after creation, rather than only from CompressionOptions directly. Hard structural constraints (partition_filters without decoupled mode, user_defined_index_factory) still force single-threaded compression regardless of the compressor's recommendation.
Also adds a sync point in MaybeStartParallelCompression for test observability, and compression_test coverage for the new functionality.
Bonus: extends CompressionManagerCustomCompression test to cover re-opening a DB at format_version=7 with a non-default compression manager that uses the built-in CompatibilityName ("BuiltinV2"). Verifies that data is readable after re-opening without the original manager, and that neither GetId() nor CompatibilityName() resolves to the custom manager via CreateFromString.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14580
Test Plan:
New unit tests in compression_test:
- GetRecommendedParallelThreads: verifies built-in compressors return the parallel_threads from their CompressionOptions for all supported compression types.
- CompressionManagerOverridesParallelThreads: end-to-end test with a custom CompressionManager that overrides parallel_threads from 1 to 4 in GetCompressorForSST, verified via sync point that parallel compression actually activates with the overridden thread count, plus data readback verification.
Existing parallel compression tests (DBCompressionTestMaybeParallel) continue to pass.
Reviewed By: xingbowang
Differential Revision: D99896343
Pulled By: pdillinger
fbshipit-source-id: 6b3a30856a78641714d33ec7ba2099f33533d3af
|
||
zaidoon
|
3b38fde293 |
Support UDI as primary index (#14547)
Summary: Add `use_udi_as_primary_index` option to `BlockBasedTableOptions`. When enabled, the UDI becomes the primary index — all reads (including internal operations like compaction and VerifyChecksum) automatically route through the UDI without needing `ReadOptions::table_index_factory`. Both the standard binary search index and the UDI are always fully built. The standard index serves as a safety fallback (e.g., for backup/restore or rollback to a non-UDI configuration). A future refactor will extract the index abstraction to allow skipping the standard index build when the UDI is primary (see discussion below). ## Write path - `UserDefinedIndexBuilderWrapper` always forwards `AddIndexEntry` and `OnKeyAdded` to both the internal standard builder and the UDI builder - New `udi_is_primary_index` table property marks primary-mode SSTs - Validates incompatible options at `DB::Open` and builder creation: partitioned index, partitioned filters, missing `user_defined_index_factory` ## Read path - `UserDefinedIndexReaderWrapper` defaults to UDI when `udi_is_primary_`, even when `ReadOptions::table_index_factory` is null — this handles the 15+ internal call sites that don't set `table_index_factory` - `use_udi_as_primary_index` automatically enforces `fail_if_no_udi_on_open` to prevent silent data loss if SSTs are opened without UDI support ## Rollback Since the standard index is always fully populated, rollback from primary mode is straightforward: set `use_udi_as_primary_index=false`. No compaction required — SSTs written in primary mode are immediately readable through the standard index. ## Public API - `BlockBasedTableOptions::use_udi_as_primary_index` (default: false) - `UserDefinedIndexBuilder::EstimatedSize()` — pure virtual, O(1) via running counter in the trie implementation ## Bug fixes (issues https://github.com/facebook/rocksdb/issues/14560, https://github.com/facebook/rocksdb/issues/14561, https://github.com/facebook/rocksdb/issues/14562) Fixed trie index correctness bugs that caused crash test failures: - **Always-on seqno encoding**: `must_use_separator_with_seq_` is now unconditionally true. Non-boundary separators store tag=0 (sentinel), same-user-key boundaries store the real tag, and the last block stores its real last-key tag. This fixes the `NonBoundaryTag` bug where non-boundary separators with `kMaxSequenceNumber` caused the post-seek correction to incorrectly advance past the correct block. - **Standard index always built in primary mode**: An empty (stub) standard index block caused behavioral divergence in `BlockBasedTableIterator` under concurrent flush/compaction, leading to `test_batches_snapshots` prefix scan inconsistencies. ## Stress test - `use_udi_as_primary_index` flag randomized by `db_crashtest.py` - Both primary and secondary UDI modes exercised in crash tests - Trie index probability halved (~6%) per reviewer request - Re-enables trie crash tests (disabled by https://github.com/facebook/rocksdb/issues/14559) ## Tests - Parameterized `TrieIndexDBTest` on UDI mode (secondary vs primary) - New factory-level tests: non-boundary separator seek, Prev within overflow, SeekToLast with overflow, empty trie, overflow exhaustion, all-scans-exhausted - New DB-level tests: multi-CF coalescing iterator, GetEntity with explicit snapshot, reverse iteration across same-user-key blocks, non-boundary separator seek correctness, rollback from primary Pull Request resolved: https://github.com/facebook/rocksdb/pull/14547 Reviewed By: anand1976 Differential Revision: D99494181 Pulled By: xingbowang fbshipit-source-id: ca52a0d5c0e523770c80e1fe2b9b5d50406b67bc |
||
|
Xingbo Wang
|
3070f73e97 |
Wide-column blob separation: lazy resolution through read, compaction, and write paths (#14386)
Summary: Wide-column blob separation: lazy resolution through read, compaction, and write paths Extend blob direct write to support wide-column entities (PutEntity), and add lazy blob resolution for wide-column values across all read and compaction paths. **Write path -- PutEntity blob separation:** - BlobWriteBatchTransformer::PutEntityCF now extracts large column values (>= min_blob_size) to blob files and serializes V2 entities with BlobIndex references, matching the existing Put behavior. - Add MaybePreprocessWideColumns() static helper to share blob extraction logic between the WriteBatch transformer and the new PutEntity fast path. - Add PutEntityFastPath() in DBImpl that preprocesses columns (sort, blob extract, serialize) before calling WriteImpl, skipping the redundant WriteBatch transformation pass. Trace batch preserves the original columns. **Read path -- blob resolution for Get/MultiGet/Iterator:** - GetContext::SaveValue resolves V2 entity blob columns eagerly: for value (Get), resolves the default column's blob reference; for columns (GetEntity), resolves all blob columns and re-serializes as V1. - DBIter::SetValueAndColumnsFromEntity detects V2 entities, deserializes with DeserializeV2, and eagerly resolves all blob columns via a new ReadPathBlobResolver. Resolved values are cached in the resolver and wide_columns_ Slices point into the cache, avoiding copies. - Add ReadPathBlobResolver (new file) -- on-demand blob fetcher for the read path with per-column caching, used by both DBIter and GetContext. - BlobFetcher gains allow_write_path_fallback to read from in-flight direct-write blob files not yet visible through Version (pre-flush reads). - Memtable lookups for Get(key) on V2 entities with a blob default column now return the blob index with is_blob_index=true, triggering the existing BDW resolution in MaybeResolveWritePathValue. - MaybeResolveWritePathValue (renamed from MaybeResolveDirectWriteBlobIndex) now also resolves V2 entity blob columns for GetEntity/MultiGetEntity, re-serializing as V1 after resolution. **Compaction path -- filter, GC, and extraction:** - CompactionIterator::InvokeFilterIfNeeded handles V2 entities: FilterV3 gets eagerly-resolved column values for backward compatibility; FilterV4 gets a CompactionBlobResolver for lazy on-demand resolution. - Add CompactionFilter::FilterV4 with WideColumnBlobResolver* parameter and SupportsFilterV4() opt-in. Default delegates to FilterV3. - CompactionBlobResolver (new class) implements WideColumnBlobResolver for the compaction path with stats tracking. - ExtractLargeColumnValuesIfNeeded extracts inline columns to blob files during compaction (entities without existing blob columns only). - GarbageCollectEntityBlobsIfNeeded relocates blob values from old blob files to new ones during compaction GC, with helpers FetchBlobsNeedingGC, RelocateBlobValues, and SerializeEntityAfterGC. - PrepareOutput unified entity deserialization: single DeserializeV2 call reused by both filter and GC/extraction paths via entity_deserialized_ flag, avoiding redundant parsing. **Merge path -- V2 entity base value resolution:** - MergeHelper::MergeUntil, GetContext::MergeWithWideColumnBaseValue, and DBIter::MergeWithWideColumnBaseValue resolve V2 blob columns before calling TimedFullMerge, using ResolveEntityForMerge. **Blob garbage accounting:** - BlobGarbageMeter tracks blob file in/out flow for V2 entity blob columns via ForEachBlobFileNumber, used for accurate GC decisions. - FileMetaData::UpdateBoundaries tracks oldest_blob_file_number for V2 entities, ensuring blob files referenced by entities are not prematurely deleted. **Serialization improvements:** - WideColumnSerialization::SerializeV2Impl allocates serialized_blob_indices only for actual blob columns (not all columns) and uses autovector for name/value sizes. - Add ForEachBlobFileNumber for lightweight blob file number extraction without full deserialization. - Add ResolveEntityForMerge helper for merge-path resolution. - Add section-size validation in DeserializeV2Impl. - Add empty blob index and column type validation. - blob_column_resolver_util.h -- shared helpers (FindBlobColumn, FindInCache, CacheInlinedBlob) used by both ReadPathBlobResolver and CompactionBlobResolver. **Testing:** - db_blob_direct_write_test: end-to-end PutEntity with BDW before/after flush, verifying Get, GetEntity, MultiGetEntity, and Iterator. - db_blob_index_test: ~1550 lines covering V2 entity blob resolution through Get, GetEntity, MultiGet, Iterator, compaction filter (V3 compat and V4 lazy), merge with blob base, and compaction GC/extraction. - compaction_iterator_test: ~950 lines testing entity blob GC, extraction, filter interaction, and combined GC+filter scenarios. - db_wide_basic_test: ~1200 lines for wide-column lazy blob resolution through all read paths plus compaction round-trips. - db_open_with_config_test: ~450 lines for BDW entity config validation. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14386 Reviewed By: anand1976 Differential Revision: D99739701 Pulled By: xingbowang fbshipit-source-id: 6badd89b577f3054802eaaa654738468efb9dbdb |
||
|
Josh Kang
|
3f51c0a185 |
Convert sequential single deletes into range tombstones (#14448)
Summary:
Add a read-path optimization that converts contiguous point tombstones into range tombstones during forward/reverse iteration. When a configurable threshold of consecutive point deletions (kTypeDeletion, kTypeDeletionWithTimestamp, kTypeSingleDeletion — with no live keys between them) is detected, a range tombstone covering `[first_tombstone_key, next_live_key)` is inserted into the active mutable memtable. This benefits future iterators by enabling efficient skipping via range tombstone fragmentation.
If there is a memtable switch during the read iteration, then the range deletion entry is discarded.
The inserted range tombstones are logically redundant (they don't delete anything that isn't already deleted by point tombstones), skip WAL (they're a derived optimization regenerated by future reads on crash), and use the max tombstone sequence number so they don't interfere with newer writes.
## Key changes
- **New option `min_tombstones_for_range_conversion`** (`AdvancedColumnFamilyOptions`): Threshold of contiguous point tombstones before converting to a range tombstone. Default 0 (disabled). Dynamically changeable via `SetOptions()`.
- **`DBIter` tracking logic** (`db/db_iter.cc`): Tracks contiguous tombstones during `FindNextUserEntryInternal()` (forward) and `PrevInternal()` (reverse). When a live key terminates a run that meets the threshold, `MaybeInsertRangeTombstone()` inserts `[first_tombstone, live_key)` into the active memtable.
- **`FindValueForCurrentKey` `found_visible` output** (`db/db_iter.cc`): Distinguishes "key deleted at this snapshot" from "no visible entries" so reverse tracking doesn't treat post-snapshot keys as tombstones.
- **`IterKey::Swap()`** (`db/dbformat.h`): Efficiently tracks reverse tombstone run end keys without extra allocations.
- **`MemTable::AddLogicallyRedundantRangeTombstone()`** (`db/memtable.cc`): Concurrent-safe range tombstone insertion into the active memtable. Range tombstone skiplist always uses concurrent inserts.
- **`ConstructFragmentedRangeTombstones` race fix** (`db/db_impl/db_impl_write.cc`): Moved after `MarkImmutable()` to prevent lost entries.
- **`MarkImmutable` ordering fix** (`db/memtable_list.cc`): Called before `current_->Add()` to close a race window.
- **Prefix filter awareness** (`db/db_iter.cc`): Tombstone tracking scoped to the seek prefix when prefix filtering is active. See dedicated section below.
- **Transaction awareness** (`db/db_iter.cc`): Tombstones with `seq > snapshot` excluded from tracking. `min_uncommitted` guard uses `insert_seq` (which may be bumped to `earliest_seq`) instead of `range_tomb_max_seq_`. See dedicated section below.
- **Duplicate range check**: Skips insertion if the memtable already covers `[start, end)`.
- **New statistics**: `READ_PATH_RANGE_TOMBSTONES_INSERTED` and `READ_PATH_RANGE_TOMBSTONES_DISCARDED`.
- **Memtable MultiGet batch lookup** (`memtable/inlineskiplist.h`, `db/memtable.cc`): `InlineSkipList::MultiGet()` with cached search path ("finger") for sorted key lookups.
- **New option `memtable_batch_lookup_optimization`** (`AdvancedColumnFamilyOptions`): Enables batch lookup for memtable MultiGet. Default false. Immutable.
## Deciding Range Tombstone Seqno
- The range tombstone is inserted with `insert_seq = max(range_tomb_max_seq_, earliest_seq)` where `range_tomb_max_seq_` is the maximum sequence number across all point tombstones in the contiguous run, and `earliest_seq` is the memtable's earliest sequence number. This preserves the memtable's `earliest_seqno_` invariant.
- If the iterator's snapshot sequence (`sequence_`) predates the memtable's `earliest_seq`, insertion is skipped entirely to avoid unintentionally covering entries between `sequence_` and `earliest_seq`.
## ConstructFragmentedRangeTombstones Race Fix
- `MarkImmutable()` and `ConstructFragmentedRangeTombstones()` are now called before `mutex_.Lock()` in `SwitchMemtable`, keeping this work outside the DB mutex. `MarkImmutable()` blocks concurrent `AddLogicallyRedundantRangeTombstone()` calls via `immutable_mutex_`, ensuring no range tombstones are inserted after the fragmented list is built. `MarkImmutable()` is idempotent, so `MemTableList::Add()` calling it again inside the mutex is harmless.
## Prefix Filter Safety
- When prefix filtering is active, the BBTI bloom filter may reject SST files outside the seek prefix, but the memtable (no bloom filter) returns keys across prefix boundaries. Tombstone tracking is scoped to the seek prefix so that converted range tombstones cannot cover live keys hidden in filtered files.
- `total_order_seek=true` disables prefix filtering — all files are visible, so tombstones safely span prefix boundaries.
- **Behavior change**: Seeking to an out-of-domain key with `total_order_seek=false` now treats it as total-order (prefix_ not set). When `prefix_same_as_start=true`, iterating past an out-of-domain key cleanly invalidates the iterator instead of calling `Transform()` on it (which was UB in release builds with `FixedPrefixTransform`). This is a requirement because an incorrect iterator scan could lead to a range tombstone covering a live key.
## Transaction Support
- Tombstones written by the transaction's own uncommitted writes (sequence > snapshot) are now excluded from contiguous tombstone tracking entirely at the tracking site in `FindNextUserEntryInternal()` and `PrevInternal()`. Previously, tracking relied on a `min_uncommitted` check at insertion time, but this was insufficient — a transaction's own Delete with `seq > snapshot` could extend a run of committed tombstones, and the resulting range tombstone would cover data visible to other snapshots.
- The fix skips any tombstone with `ikey_.sequence > sequence_` during tracking. If a transaction-owned tombstone appears mid-run, it flushes the accumulated committed run first, then resets tracking. This ensures only tombstones visible to the current snapshot are ever converted.
- Both WritePrepared and WriteUnprepared transactions are supported with dedicated test coverage:
- **WritePrepared**: When tombstones are committed before `Prepare()`, their seqnos are below `min_uncommitted` and insertion proceeds safely. When `Prepare()` happens first, tombstone seqnos exceed `min_uncommitted` and insertion is blocked.
- **WriteUnprepared**: Multiple unprepared batches with different seqno ranges are handled correctly. Own transaction Deletes that extend a committed tombstone run block insertion of the entire run. After rollback, data correctness is verified.
## UDT support
- When user-defined timestamps (UDT) are enabled, keys include an 8-byte timestamp suffix. The comparator, Put/Delete APIs, and ReadOptions all require timestamps.
- Forward exhaustion with UDT: `iterate_upper_bound_` is a plain user key without a timestamp suffix. It is padded with min timestamp via `AppendKeyWithMinTimestamp()` so it sorts after all entries with this user key, preserving the exclusive bound semantics.
- Reverse exhaustion with UDT: The end key comes from either the previous live key (which already has a proper timestamp suffix) or the seek target set by `SetSavedKeyToSeekForPrevTarget()` (which appends a timestamp via `SetInternalKey(..., timestamp_ub_)` and `UpdateInternalKey(..., ts)`). In both cases, the end key is already properly timestamped, so no additional padding is needed unlike forward exhaustion.
- Contiguous tombstone detection works correctly with UDT because the underlying `kTypeDeletionWithTimestamp` entries are tracked the same way as `kTypeDeletion`.
## Concurrent Iterators
- Should concurrent iterators happen to read the range at the same time, both will produce the same range and seqno entry. Only one will be accepted by the skip list and the others will be rejected. Future iterators will read the range and not even attempt to insert the range.
- There is nothing preventing similar ranges from being inserted however. Two iterators can produce overlapping ranges, but this protection would be complicated to implement and there is no evidence that it is a likely scenario yet.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14448
Test Plan:
- **Unit tests** (`db/db_iterator_test.cc`): `ReadPathRangeTombstoneTest` parameterized by forward/reverse with cases for basic insertion, non-contiguous (below threshold), memtable switch, exhausted iterator with/without bounds, direction change, mixed Delete/SingleDelete, single-delete-only runs, snapshot predating memtable, block cache tier incomplete, skip when covered by existing range, UDT basic scan, UDT exhaustion, prefix filter cross-prefix scan (`PrefixFilterCrossPrefixScanCoversLiveKey` with default/total_order_seek/prefix_same_as_start variants), stale ikey from forward-then-reverse scan (`StaleIkeyFromForwardThenReverse`), and reseek stale ikey (`ReseekStaleIkey`).
- **Concurrency test** (`db/db_test2.cc`): `DBTestConcurrentRangeTombstoneConversions` parameterized by `(allow_concurrent_memtable_write, min_tombstones_for_range_conversion)` with mixed writers, deleters, range deleters, and concurrent forward/reverse readers.
- **Transaction tests** (`utilities/transactions/write_prepared_transaction_test.cc`, `write_unprepared_transaction_test.cc`): Tests for WritePrepared (insertion allowed when tombstones committed before prepare, blocked when after; seqno bump shadowing prepared writes `RangeTombstoneSeqnoBumpShadowsPreparedWrite`) and WriteUnprepared (multiple batches, extended visibility with CalcMaxVisibleSeq, own deletions with rollback).
- **IterKey::Swap tests** (`db/dbformat_test.cc`): `IterKeySwapTest` parameterized over `(key_len, copy, use_secondary)` × 2 covering all inline/heap/pinned/secondary combinations.
- **InlineSkipList MultiGet tests** (`memtable/inlineskiplist_test.cc`): Basic, exact matches, empty, single key, randomized validation against `std::set::lower_bound`, duplicate keys with callback walk, and concurrent MultiGet with read-after-write consistency.
- **Memtable MultiGet tests** (`db/db_basic_test.cc`): Batch lookup, overwrite, flush, merge, disabled by default, paranoid checks, and snapshot tests.
- **Stress test coverage**: `min_tombstones_for_range_conversion` and `memtable_batch_lookup_optimization` options added to `db_crashtest.py` and `db_stress` flags.
- `make check` passes all tests.
### Benchmark results
Tombstones scattered randomly in clusters via `seek_nexts_to_delete` for realistic workloads.
**DB Setup A (scattered deletes, 100 per seek)**:
```
# Step 1: Create and compact
./db_bench --benchmarks=fillseq,compact --seed=1 --compression_type=none --num=1000000 --db=<DB>
# Step 2: Scatter tombstones (5000 seeks × 100 deletes ≈ 500k tombstones)
./db_bench --benchmarks=seekrandom,flush --seed=1 --compression_type=none --num=2000 \
--seek_nexts=0 --seek_nexts_to_delete=100 --use_existing_db=1 --threads=1 --db=<DB>
```
**DB Setup A2 (scattered deletes, 8 per seek)**:
```
# Step 1: Create and compact
./db_bench --benchmarks=fillseq,compact --seed=1 --compression_type=none --num=1000000 --db=<DB>
# Step 2: Scatter tombstones (5000 seeks × 8 deletes ≈ 40k tombstones)
./db_bench --benchmarks=seekrandom,flush --seed=1 --compression_type=none --num=2000 \
--seek_nexts=0 --seek_nexts_to_delete=8 --use_existing_db=1 --threads=1 --db=<DB>
```
**DB Setup B (no deletes)**:
```
./db_bench --benchmarks=fillseq,compact --seed=1 --compression_type=none --num=1000000 \
[--key_size=100] --db=<DB>
```
**Read workload** (same for all):
```
./db_bench --benchmarks=seekrandom --seek_nexts=100 --threads=8 \
--reverse_iterator={true,false} --seed=1 --use_existing_db=1 \
--compression_type=none --num=1000000 --duration=10 \
--disable_auto_compactions \
[--key_size=100] [--min_tombstones_for_range_conversion=X] --db=<DB_COPY>
```
Each workload averaged over 3 runs.
**Table 1: seekrandom forward, scattered deletes (2000 seeks × 100 deletes/seek)**
| Variant | avg ops/s | % vs main |
|---------|-----------|-----------|
| main | 2,895 | - |
| threshold=0 | 2,869 | -0.9% |
| threshold=8 | 287,334 | +9,824% |
**Table 2: seekrandom reverse, scattered deletes (2000 seeks × 100 deletes/seek)**
| Variant | avg ops/s | % vs main |
|---------|-----------|-----------|
| main | 544 | - |
| threshold=0 | 548 | +0.7% |
| threshold=8 | 206,491 | +37,860% |
**Table 3: seekrandom forward, scattered deletes (2000 seeks × 8 deletes/seek)**
| Variant | avg ops/s | % vs main |
|---------|-----------|-----------|
| main | 194,049 | - |
| threshold=0 | 195,703 | +0.9% |
| threshold=8 | 310,740 | +60.1% |
**Table 4: seekrandom reverse, scattered deletes (2000 seeks × 8 deletes/seek)**
| Variant | avg ops/s | % vs main |
|---------|-----------|-----------|
| main | 63,854 | - |
| threshold=0 | 69,266 | +8.5% |
| threshold=8 | 218,101 | +241.6% |
**Table 5: seekrandom forward, no deletes (regression check)**
| Variant | key=16B avg ops/s | % vs main | key=100B avg ops/s | % vs main |
|---------|-------------------|-----------|---------------------|-----------|
| main | 330,901 | - | 236,048 | - |
| threshold=0 | 328,398 | -0.8% | 238,055 | +0.9% |
| threshold=8 | 332,539 | +0.5% | 233,776 | -1.0% |
**Table 6: seekrandom reverse, no deletes (regression check)**
| Variant | key=16B avg ops/s | % vs main | key=100B avg ops/s | % vs main |
|---------|-------------------|-----------|---------------------|-----------|
| main | 261,445 | - | 192,177 | - |
| threshold=0 | 265,020 | +1.4% | 191,616 | -0.3% |
| threshold=8 | 250,881 | -4.0% | 189,239 | -1.5% |
Reviewed By: xingbowang
Differential Revision: D96203950
Pulled By: joshkang97
fbshipit-source-id: 06ba66ebde3c355f04671d1e681f1b1586e8751d
|
||
Jialiang Tan
|
65e77282ce |
Add block_decompress_count to PerfContext (#14557)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14557 RocksDB's PerfContext tracks block_decompress_time but has no counter for the number of block decompressions. The global Statistics ticker NUMBER_BLOCK_DECOMPRESSED exists but is not accessible through PerfContext, which is the thread-local, per-operation metric system used by benchmarks. Add block_decompress_count as a new PerfContext counter (gated at kEnableCount level) incremented in DecompressBlockData alongside the existing NUMBER_BLOCK_DECOMPRESSED ticker. This enables benchmarks and applications to observe how many blocks required decompression per operation, complementing the existing block_read_count. Reviewed By: anand1976 Differential Revision: D99233514 fbshipit-source-id: 40a68c1d9321f560cebdb7c30a544a0c62ae64f0 |
||
|
Jialiang Tan
|
a0ff5b143f |
Add num_data_blocks_compression_rejected/bypassed to TableProperties (#14551)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14551 Add two new per-SST TableProperties fields that track how many data blocks were stored uncompressed: - `num_data_blocks_compression_rejected`: blocks where compression was attempted but the compressed output exceeded the ratio limit set by `CompressionOptions::max_compressed_bytes_per_kb`. - `num_data_blocks_compression_bypassed`: blocks where compression was never attempted (e.g., kNoCompression type, no compressor available). Together with `num_data_blocks`, these give a full decomposition: num_data_blocks = compressed + rejected + bypassed Previously this information was only available via global Statistics tickers (NUMBER_BLOCK_COMPRESSION_REJECTED / BYPASSED) which aggregate across all SSTs. Now it is persisted per-SST in the properties block and visible via sst_dump --show_properties. Reviewed By: anand1976 Differential Revision: D99229339 fbshipit-source-id: 8333f51acff30a759d725e5e94bd80f4fcb18b57 |
||
Rajat Jain
|
64f8d5765b |
Add blob_compression_opts to ColumnFamilyOptions (#14568)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14568 Port of D98326754 to internal_repo_rocksdb (GitHub-synced repo). BlobDB blob file compression currently uses hardcoded default CompressionOptions{} in BlobFileBuilder, ignoring any user-specified compression options like level, window_bits, or strategy. This means ZSTD always uses level 3 (doubleFast strategy with two hash tables), even when a lower level would significantly reduce CPU usage. This diff adds a new `blob_compression_opts` field to AdvancedColumnFamilyOptions (and MutableCFOptions) and plumbs it into BlobFileBuilder::GetCompressor(). The option is registered as mutable and dynamically changeable via SetOptions(). For example, setting level=1 with ZSTD switches from "doubleFast" (two hash tables per thread) to "fast" (single hash table), reducing L3 cache pressure on flush-heavy workloads. There was an existing TODO in blob_file_builder.cc requesting exactly this feature — this diff fulfills it. This change is safe as the default value is always dFast. So this will not impact the current code. Reviewed By: xingbowang Differential Revision: D99478562 fbshipit-source-id: e79b847a854f60dea552442b18fd3f1384efac70 |
||
|
Xingbo Wang
|
4cf28450a5 |
Add customizable blob partition strategy for blob direct write (#14565)
Summary:
This PR adds a `BlobFilePartitionStrategy` interface that allows users to plug in custom partition selection logic for blob direct writes. The default behavior (round-robin across partitions) is preserved as the built-in `RoundRobinBlobFilePartitionStrategy`.
### Motivation
The existing blob direct write implementation distributes writes across partitions using a fixed round-robin strategy (atomic counter mod num_partitions). While this works well for uniform workloads, some use cases benefit from custom routing:
- **Key-based routing**: co-locate related keys in the same blob partition for locality-aware reads.
- **CF-based routing**: direct writes for different column families to dedicated partitions.
- **Value-size routing**: send large vs. small values to different partitions.
- **Application-defined affinity**: any domain-specific grouping that the default round-robin cannot express.
### Design
A new pure-virtual interface `BlobFilePartitionStrategy` is added to `include/rocksdb/advanced_options.h`:
```cpp
class BlobFilePartitionStrategy {
public:
virtual ~BlobFilePartitionStrategy() = default;
// Select a partition for the given blob direct write.
// The return value can be any uint32_t; the caller applies
// modulo num_partitions internally.
// Implementations must be thread-safe.
virtual uint32_t SelectPartition(uint32_t num_partitions,
uint32_t column_family_id,
const Slice& key,
const Slice& value) = 0;
};
```
The strategy receives:
- `num_partitions`: the configured partition count (for implementations that want to be partition-count-aware).
- `column_family_id`: useful for CF-based routing.
- `key` / `value`: the key and value being written.
The return value is taken modulo `num_partitions` internally, so implementations can return any `uint32_t` without worrying about bounds.
A new column family option `blob_direct_write_partition_strategy` (type `std::shared_ptr<BlobFilePartitionStrategy>`, default `nullptr`) wires the strategy into `BlobFilePartitionManager`. When `nullptr`, the built-in `RoundRobinBlobFilePartitionStrategy` is used, preserving existing behavior exactly.
### Changes
| File | Change |
|---|---|
| `include/rocksdb/advanced_options.h` | New `BlobFilePartitionStrategy` interface; new `blob_direct_write_partition_strategy` option |
| `db/blob/blob_file_partition_manager.h/.cc` | Accept strategy in constructor; replace atomic counter with strategy call; move `RoundRobinBlobFilePartitionStrategy` here as default |
| `options/cf_options.h/.cc` | Thread strategy through `ImmutableCFOptions` |
| `options/options_helper.cc` | Propagate strategy in `UpdateColumnFamilyOptions` |
| `options/options_settable_test.cc` | Register new option field in options settability test |
| `db/db_impl/db_impl.cc` | Pass strategy when constructing `BlobFilePartitionManager` |
| `db/blob/db_blob_direct_write_test.cc` | New test with `FixedBlobDirectWritePartitionStrategy` |
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14565
Test Plan:
New test `DirectWriteCustomPartitionStrategyRoutesWritesToOneBlobFile` verifies that a `FixedBlobDirectWritePartitionStrategy` that always returns partition 3 correctly routes all writes to a single blob file across 4 configured partitions.
```
make -j128 db_blob_direct_write_test && ./db_blob_direct_write_test --gtest_filter='*CustomPartition*'
make -j128 db_blob_direct_write_test && ./db_blob_direct_write_test
```
Reviewed By: joshkang97
Differential Revision: D99458813
Pulled By: xingbowang
fbshipit-source-id: 54cfbb0f75e24b58a8db61ad8755204d0db6ece7
|
||
|
Xingbo Wang
|
af4e32945b |
Blob direct write v1: write-path blob separation with partitioned files (reduced scope) (#14535)
Summary: This PR introduces **blob direct write v1**, a reduced-scope write-path optimization where large values (>= `min_blob_size`) are written directly to blob files during `Put()` and replaced in the memtable with compact `BlobIndex` references. This avoids holding full values in memory until flush time. ### Motivation In the existing BlobDB architecture, values are written to the WAL and memtable in their full form and separated into blob files only at flush time. This means: - Large values are held in memory twice (raw in memtable + blob file at flush) - Blob I/O is serialized through a single flush thread per column family Blob direct write addresses both: values leave the write path as small `BlobIndex` references, and multiple **partitions** (configurable via `blob_direct_write_partitions`) allow concurrent blob writes with independent locks. ### Design (v1 — single-writer, WAL-disabled, reduced scope) The v1 design intentionally keeps scope narrow for correctness and reviewability: - **Single writer thread assumption**: no concurrent writes to the same partition file. One logical writer serializes the batch. - **WAL-disabled**: direct-write blob files are only registered in MANIFEST at flush time. WAL replay cannot recover unregistered blob references, so WAL is disabled for this v1. - **Flush-on-write**: each `AddRecord` call flushes to the OS immediately. - **FIFO generation batching**: each memtable switch creates one generation batch. Direct-write files for that memtable are sealed and registered atomically when the batch is flushed to MANIFEST. - **Round-robin partitions**: blob writes are distributed across `blob_direct_write_partitions` files using an atomic counter. ### New components | Component | Description | |---|---| | `BlobFilePartitionManager` | Owns N partition files per CF. Manages open/seal/register lifecycle tied to memtable generations. | | `BlobWriteBatchTransformer` | A `WriteBatch::Handler` that rewrites qualifying `Put` values as `BlobIndex` entries before the batch enters the write group. | ### Write path integration 1. `DBImpl::WriteImpl` calls `BlobWriteBatchTransformer::TransformBatch` before entering the writer group (for default write path), or before joining the batch group (for pipelined/unordered write). 2. Values >= `min_blob_size` are written to a partition file; the key is stored with a `BlobIndex` in the transformed batch. A rollback guard marks blob bytes as initial garbage if the write fails. 3. On `SwitchMemtable`, `RotateCurrentGeneration` moves active partitions into the next immutable batch. 4. `FlushMemTableToOutputFile` / `AtomicFlushMemTablesToOutputFiles` call `PrepareFlushAdditions` to seal partition files and collect `BlobFileAddition` + `BlobFileGarbage` entries registered to MANIFEST alongside the flush. 5. Shutdown paths (`CancelAllBackgroundWork`, `WaitForCompact` with `close_db=true`) force-flush all CFs with active direct-write managers to ensure blob files are registered before close. ### Read path - **Get/MultiGet**: `MaybeResolveBlobForWritePath` resolves `BlobIndex` references found in memtable or immutable memtable via `BlobFilePartitionManager::ResolveBlobDirectWriteIndex`, which first checks manifest-visible state and falls back to direct blob-file reads via `BlobFileCache`. - **Iterator**: `DBIter::BlobReader` is extended with a `BlobFilePartitionManager*` to resolve direct-write blob indexes during iteration. The unified `ResolveBlobDirectWriteIndex` path handles both manifest-visible and not-yet-flushed files. ### New options | Option | Default | Description | |---|---|---| | `enable_blob_direct_write` | `false` | Enable write-path blob separation for this CF. Requires `enable_blob_files = true`. Not dynamically changeable. | | `blob_direct_write_partitions` | `1` | Number of parallel partition files per CF. Not dynamically changeable. | ### Feature incompatibilities (reduced v1 scope) The following features are *not supported* when `enable_blob_direct_write = true`, and are enforced both in `db_stress_tool` validation and `db_crashtest.py` sanitization: **Write model constraints:** - `threads` must be 1 (single writer assumption) - `allow_concurrent_memtable_write` = 0 - `enable_pipelined_write` = 0 (transformation done before batch group, but pipelined path supported with pre-transform) - `two_write_queues` = 0 - `unordered_write` = 0 (transformation done before batch group, but unordered path supported with pre-transform) **WAL and recovery:** - `disable_wal` = 1 (required — WAL replay of unregistered blob files is out of v1 scope) - `best_efforts_recovery` = 0 - `reopen` = 0 (no crash-restart with WAL replay) - All WAL-related stress features disabled: `manual_wal_flush_one_in`, `sync_wal_one_in`, `lock_wal_one_in`, `get_sorted_wal_files_one_in`, `get_current_wal_file_one_in`, `track_and_verify_wals`, `rate_limit_auto_wal_flush`, `recycle_log_file_num` **Blob GC and dynamic options:** - `use_blob_db` = 0 (stacked BlobDB not supported) - `allow_setting_blob_options_dynamically` = 0 - `enable_blob_garbage_collection` = 0 - `blob_compaction_readahead_size` = 0 - `blob_file_starting_level` = 0 **Unsupported value types and APIs:** - Merge (`use_merge`, `use_full_merge_v1`) — merge values pass through untransformed - Entity APIs (`use_put_entity_one_in`, `use_get_entity`, `use_multi_get_entity`, `use_attribute_group`) - `use_timed_put_one_in` - User-defined timestamps (`user_timestamp_size`, `persist_user_defined_timestamps`, `create_timestamped_snapshot_one_in`) - Transactions (`use_txn`, `use_optimistic_txn`, `test_multi_ops_txns`, `commit_bypass_memtable_one_in`) — though `WriteCommittedTxn::CommitInternal` falls back from bypass-memtable to normal path when BDW is active - `IngestWriteBatchWithIndex` returns `NotSupported` - `inplace_update_support` = 0 **Fault injection:** - All write/read/metadata fault injection disabled (`sync_fault_injection`, `write_fault_one_in`, `metadata_write_fault_one_in`, `read_fault_one_in`, `metadata_read_fault_one_in`, `open_*_fault_one_in`) **Infrastructure/snapshot APIs:** - `remote_compaction_worker_threads` = 0 - `test_secondary` = 0 - `backup_one_in` = 0 - `checkpoint_one_in` = 0 - `get_live_files_apis_one_in` = 0 - `ingest_external_file_one_in` = 0 - `ingest_wbwi_one_in` = 0 ### Tests - `db/blob/db_blob_basic_test.cc`: ~660 lines of new direct-write unit tests covering basic put/get, multi-partition, flush/compaction, recovery, and error injection. - `db/blob/blob_file_cache_test.cc`: ~96 lines of new tests for direct-write blob file cache behavior. - `db/write_batch_test.cc`: ~96 lines of tests for WriteBatch with blob index entries. - `utilities/transactions/transaction_test.cc`: verifies transaction commit path falls back correctly with direct write enabled. - `db_stress_tool/`: full stress test support with `--enable_blob_direct_write` and `--blob_direct_write_partitions` flags, integrated into `db_crashtest.py` with 10% random selection alongside regular blob params. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14535 Test Plan: ``` make -j128 db_blob_basic_test && ./db_blob_basic_test make -j128 blob_file_cache_test && ./blob_file_cache_test make -j128 write_batch_test && ./write_batch_test make -j128 transaction_test && ./transaction_test make -j128 check ``` Stress test: ``` python3 tools/db_crashtest.py blackbox --enable_blob_direct_write=1 \ --enable_blob_files=1 --blob_direct_write_partitions=4 \ --disable_wal=1 --threads=1 ``` Reviewed By: pdillinger Differential Revision: D98766843 Pulled By: xingbowang fbshipit-source-id: 1577653826913a59d05680a87bce5534ac5a5e69 |
||
Hemal Shah
|
b611aa7309 |
Export header to add log data on a transaction (#14543)
Summary: Transactions in rockdb supported adding log data using the `put_log_data` api, but this was not exported for other language bindings. Exported this binding allows other languages like rust, go, etc add log data on an transaction Pull Request resolved: https://github.com/facebook/rocksdb/pull/14543 Reviewed By: joshkang97 Differential Revision: D99171663 Pulled By: xingbowang fbshipit-source-id: 24c94d71668a41cc7b2913972427255ab67d0863 |
||
|
Danny Chen
|
ae322e0a73 |
Add SortedRunBuilder: use RocksDB as an external… (#14499)
Summary: CONTEXT: MyRocks and other third-party users currently need to understand deep RocksDB internals to sort unsorted data into SST files. The existing pattern requires: opening a full DB instance, configuring VectorRepFactory with universal compaction, disabling auto-compaction, writing unsorted data, manually triggering CompactRange with kForceOptimized, collecting output via GetColumnFamilyMetaData, and finally ingesting via IngestExternalFile with allow_db_generated_files. This is error-prone and requires ~30 lines of RocksDB configuration knowledge. WHAT: This adds a new utility class, SortedRunBuilder, that wraps all of the above into a simple Create/Add/Finish API. Callers feed in unsorted key-value pairs (in any order, from any number of threads) and receive sorted SST files with seqno=0 ready for ingestion -- or can iterate sorted output directly. The name SortedRunBuilder was chosen over alternatives like UnsortedSstFileWriter because this utility targets a broad audience: anyone wanting to use RocksDB as an external sort engine, not just users migrating from SstFileWriter. That said, this explicitly removes the sorted-input requirement that SstFileWriter enforces -- callers no longer need to pre-sort their data before writing SST files. KEY DESIGN DECISIONS: - Zero changes to core RocksDB internals. This is a pure utility layer that exclusively calls existing public APIs: * DB::Open(), DB::Put(), DB::Write(), DB::Flush(), DB::CompactRange(), DB::GetColumnFamilyMetaData(), DB::NewIterator(), DestroyDB() * VectorRepFactory (sort-on-flush memtable) * BottommostLevelCompaction::kForceOptimized (zero seqnos) - No modifications to: compaction logic, memtable implementation, SST file format, ingestion logic, or DB open/close paths - All new code lives in utilities/sorted_run_builder/ and the public header include/rocksdb/utilities/sorted_run_builder.h - Build file changes are limited to registering the new source/test files API SURFACE: SortedRunBuilderOptions opts; opts.temp_dir = "/tmp/sort_work"; std::unique_ptr<SortedRunBuilder> builder; SortedRunBuilder::Create(opts, &builder); builder->Add(key, value); // any order, thread-safe builder->AddBatch(&batch); // WriteBatch for throughput builder->Finish(); // flush + compact + collect builder->GetOutputFiles(); // sorted SSTs for ingestion builder->NewIterator(ro); // iterate sorted output FILES CHANGED: New: include/rocksdb/utilities/sorted_run_builder.h (public header) New: utilities/sorted_run_builder/sorted_run_builder.cc (implementation) New: utilities/sorted_run_builder/sorted_run_builder_test.cc (12 tests) New: docs/plans/sorted_run_builder_plan.md (design plan) New: docs/plans/sorted_run_builder_usage_guide.md (usage guide) Modified: src.mk, CMakeLists.txt, Makefile (register new files only) Pull Request resolved: https://github.com/facebook/rocksdb/pull/14499 Test Plan: $ make clean && make -j$(nproc) sorted_run_builder_test $ ./sorted_run_builder_test [==========] Running 12 tests from 1 test case. [ PASSED ] 12 tests. (688 ms total) Tests cover: basic sort correctness, empty builder, WriteBatch path, concurrent multi-threaded writes, ingestion into a target DB, large random dataset (10K keys), entry/size counters, error cases (Add after Finish, iterator before Finish, empty temp_dir), cleanup verification, and duplicate key handling. Reviewed By: xingbowang Differential Revision: D98935972 Pulled By: dannyhchen fbshipit-source-id: e3bb7f1ea5f6004aab697e4da667fa21292ca250 |
||
|
zaidoon
|
41e5eb1a4d |
Support reverse iteration in UDI/trie index and optimize hot paths (#14466)
Summary: The following performance optimizations are included in this PR: - Inline NextSetBit/PrevSetBit into header (called on every trie level) - Unroll Rank1 popcount loop (the single hottest function) - Advance/Retreat: replace path stack top in-place instead of pop+push - std::swap for prev_key_scratch_ instead of O(n) string copy - assign() instead of ToString() to reuse buffer capacity - Cache IndexValue in wrapper to avoid repeated virtual dispatch - Mark TrieIndexIterator/TrieIndexBuilder as final for devirtualization Part of https://github.com/facebook/rocksdb/issues/12396 Pull Request resolved: https://github.com/facebook/rocksdb/pull/14466 Reviewed By: anand1976 Differential Revision: D97979699 Pulled By: xingbowang fbshipit-source-id: 7ec56ecc8b6d68548a336dd1aaccfb215189eff0 |
||
|
Josh Kang
|
f897c1b2ef |
Add uniform block tracking stats (#14513)
Summary: Add tracking of uniform index blocks as a table property (`num_uniform_blocks`). When `uniform_cv_threshold` is set, the block builder detects uniformly distributed keys via coefficient of variation of key gaps. This information is now surfaced end-to-end: from block building through index builders to SST table properties. ## Key changes - **BlockBuilder**: Persist the uniformity result from `ScanForUniformity()` as a member (`is_uniform_`) exposed via `IsUniform()`, rather than a local variable discarded after `Finish()`. - **Index builders**: All three index builder types (`ShortenedIndexBuilder`, `HashIndexBuilder`, `PartitionedIndexBuilder`) implement `NumUniformIndexBlocks()`. For partitioned indexes, the count accumulates across partition `Finish()` calls. - **Table property serialization**: Added `TablePropertiesNames::kNumUniformBlocks` (`"rocksdb.num.uniform.blocks"`) with full serialization/deserialization support. Without this, the property was computed in memory but never persisted to SST files. - **Table property aggregation**: `num_uniform_blocks` included in `Add()` and `GetAggregatablePropertiesAsMap()` for correct cross-SST aggregation. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14513 Test Plan: - **Unit tests**: `block_test` (7504 passed), `table_test` (6917 passed), `table_properties_collector_test` (8 passed). - **End-to-end verification via db_bench + sst_dump**: - Positive: `./db_bench --benchmarks=fillseq,compact --num=10000 --uniform_cv_threshold=0.2 --index_shortening_mode=0 --compression_type=none` produces SST with `# uniform blocks: 1`. - Negative: Same with `--uniform_cv_threshold=-1` (disabled) produces `# uniform blocks: 0`. Reviewed By: xingbowang Differential Revision: D98343170 Pulled By: joshkang97 fbshipit-source-id: ed08e83b2dcfb70f074bfb0f7bb5c31d75dc6da9 |
||
|
Josh Kang
|
5db0603613 |
Read-triggered compactions (#14426)
Summary: Add read-triggered compaction, a new feature that reduces read amplification by compacting SST files that receive high read traffic. When an SST file's read frequency (`num_reads_sampled / file_size`) exceeds a configurable threshold, it is marked for compaction to a lower level. The feature introduces two new options: a CF option `read_triggered_compaction_threshold` (default 0, disabled) and a DB option `max_periodic_compaction_trigger_seconds` (default 43200s) that controls how often the background thread re-evaluates compaction scores on quiet databases. Both options are dynamically changeable. Lowering `max_periodic_compaction_trigger_seconds` does add some overhead, but generally is minimal, so running this every couple of minutes in a production environment seems fairly reasonable. ## Key changes - **New CF option `read_triggered_compaction_threshold`** (`advanced_options.h`): When positive, files with `reads_per_byte > threshold` are marked for compaction. Files at the last non-empty level are skipped (bottommost compaction handles those separately). Marked files are sorted by hotness (reads_per_byte descending). - **New DB option `max_periodic_compaction_trigger_seconds`** (`options.h`): Replaces the hardcoded 12-hour ceiling in `ComputeTriggerCompactionPeriod()`. Essential for read-triggered compaction on quiet DBs since there are no writes to trigger score re-evaluation. - **Leveled compaction picker** (`compaction_picker_level.cc`): Adds read-triggered as the lowest-priority compaction reason in `SetupInitialFiles()`, using the existing `PickFileToCompact` helper. - **Universal compaction picker** (`compaction_picker_universal.cc`): Adds `PickReadTriggeredCompaction` as lowest priority. Refactors shared "find output level + compute overlapping inputs + create Compaction" logic from both `PickDeleteTriggeredCompaction` and `PickReadTriggeredCompaction` into `BuildCompactionToNextLevel`, handling both single-level and multi-level universal cases. - **Periodic trigger integration** (`db_impl.cc`): `TriggerPeriodicCompaction` now also fires for CFs with `read_triggered_compaction_threshold > 0`, even without time-based compaction configured. - **Stress test & db_bench support**: Both `db_stress` and `db_bench` support the new options. `db_crashtest.py` randomly enables read-triggered compaction and sets a short periodic trigger interval when enabled. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14426 Test Plan: **Unit tests**: - `compaction_picker_test` — 7 new tests: `ReadTriggeredCompactionDisabled`, `ReadTriggeredCompactionBelowThreshold`, `ReadTriggeredCompactionAboveThreshold`, `NeedsCompactionReadTriggered`, `ReadTriggeredPicksFile`, `UniversalReadTriggeredCompaction`, `ReadTriggeredSkipsLastLevel`, `UniversalReadTriggeredNoPickWhenNotMarked` - `db_compaction_test` — `ReadTriggeredCompaction` integration test verifying end-to-end behavior with sync points - Stress test coverage **Stress test**: ``` make V=1 -j "CRASH_TEST_EXT_ARGS=--duration=600 --max_key=2500000 --max_compaction_trigger_wakeup_seconds=10 --read_triggered_compaction_threshold=0.0001 --interval=600" blackbox_crash_test ``` - confirmed read triggered compactions from LOGS **Benchmark** (`db_bench`): Setup: 5M keys (100B values, 16B keys), leveled compaction, 5 levels, 4MB target file size. DB fully compacted, then 2M overlapping keys written without compaction to create L0/L1 overlap (82 files, ~294MB). LSM shape change during readrandom with read-triggered compaction: ``` BEFORE: L0=9 files (15MB), L1=4 (16MB), L2=20 (69MB), L3=49 (194MB) — 82 files, 294MB AFTER: L3=66 files (223MB) ``` | Benchmark | Config | avg ops/s | % change | |-----------|--------|-----------|----------| | readrandom (8 threads, 5M reads) | baseline (threshold=0) | 1,086,965 | — | | readrandom (8 threads, 5M reads) | threshold=0.000001, trigger=5s | 1,453,697 | **+33.7%** | Reviewed By: xingbowang Differential Revision: D97838716 Pulled By: joshkang97 fbshipit-source-id: a21fcb270c7fadd4f78d98b9c821982f220dd3f0 |
||
|
Xingbo Wang
|
1a4b1e42cc |
Add include_blob_files option to GetApproximateSizes (#14501)
Summary: **Summary:** Add a new boolean flag `include_blob_files` (default: `false`) to `SizeApproximationOptions` and a corresponding `INCLUDE_BLOB_FILES` enum value to `SizeApproximationFlags`. When set to `true`, the returned size includes an approximation of blob file data in the queried key range. **Algorithm:** The blob file size contribution is prorated using the SST size ratio: ``` blob_size_in_range ≈ total_blob_size * (sst_size_in_range / total_sst_size) ``` The blob-to-SST ratio (`total_blob_size / total_sst_size`) is computed once before the per-range loop, so iterating levels and blob files only happens once per `GetApproximateSizes` call regardless of how many ranges are queried. The per-range SST size (`ApproximateSize`) is computed once and shared between `include_files` and `include_blob_files`. **Limitations:** - Assumes blob data is distributed proportionally to SST data across the key space. May be inaccurate if blob value sizes vary significantly across different key ranges (e.g., one range has large blobs while another has small ones). - If there are no SST files (all data in memtables), the blob size contribution will be 0 even if blob files exist on disk. **Changes:** - `include/rocksdb/options.h`: New `include_blob_files` field in `SizeApproximationOptions`; updated doc comments for `include_memtables`/`include_files` - `include/rocksdb/db.h`: New `INCLUDE_BLOB_FILES` in `SizeApproximationFlags` enum, updated flags-to-options mapping - `include/rocksdb/c.h`: New `rocksdb_size_approximation_flags_include_blob_files` C API enum value - `java/`: Added `INCLUDE_BLOB_FILES` to `SizeApproximationFlag.java` and JNI flag mapping in `rocksjni.cc` - `db/db_impl/db_impl.cc`: Blob-to-SST ratio computed once before loop, SST range size computed once per range and shared - `db_stress_tool/db_stress_test_base.cc`: Randomized `include_blob_files` in stress test Pull Request resolved: https://github.com/facebook/rocksdb/pull/14501 Test Plan: - New `DBBlobBasicTest.GetApproximateSizesIncludingBlobFiles` — verifies: - Size with blobs > without (full range) - Non-overlapping range returns 0 - Partial range returns proportionally less than full range - `SizeApproximationFlags` API works - Multi-range query: two sub-ranges sum approximately to the full-range result - Stress test now exercises the new option randomly Reviewed By: hx235 Differential Revision: D97984211 Pulled By: xingbowang fbshipit-source-id: e9127eac3308687fd4f0b17a771fd61fba6a8380 |
||
Andrew Chang
|
24dc8306f8 |
Add atomic_flush to CreateBackupOptions (#14511)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14511 Add `atomic_flush` option to `CreateBackupOptions` that plumbs through `CheckpointImpl::CreateCustomCheckpoint` to `LiveFilesStorageInfoOptions`. When `flush_before_backup=true` and `atomic_flush=true`, the backup engine atomically flushes all column families before creating the backup. This ensures cross-CF consistency without needing WAL files. Combined with `BackupEngineOptions::backup_log_files=false`, this allows safely skipping WAL backup for multi-CF databases, reducing backup size and complexity. Changes: - `backup_engine.h`: Add `bool atomic_flush = false` to `CreateBackupOptions` - `checkpoint_impl.h/.cc`: Add `bool atomic_flush` parameter to `CreateCustomCheckpoint`, plumb to `LiveFilesStorageInfoOptions::atomic_flush` - `backup_engine.cc`: Pass `options.atomic_flush` through to `CreateCustomCheckpoint` - `checkpoint_test.cc`: Add `BackupWithAtomicFlushSkipsWAL` test verifying backup with atomic flush + no WAL creates a valid restorable multi-CF backup Reviewed By: xingbowang Differential Revision: D98208696 fbshipit-source-id: e818ba8669ac52a206e30e9dd56f1d7573cc0175 |
||
|
Josh Kang
|
f620a6d039 |
Support Pinnable Reads In SstFileReader (#14500)
Summary: Add `SstFileReader::Get` (single-key) and `SstFileReader::MultiGet` (PinnableSlice) overloads to enable zero-copy point lookups directly from SST files. The existing `MultiGet(std::string*)` is refactored to delegate to the new `MultiGet(PinnableSlice*)`, which writes results directly into caller-provided `PinnableSlice` values instead of copying through an intermediate buffer. The single-key `Get` uses `TableReader::Get` with a `GetContext` for efficient single-key lookups without the overhead of MultiGet's sorting and batching machinery. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14500 Test Plan: - New unit tests Reviewed By: xingbowang Differential Revision: D97825648 Pulled By: joshkang97 fbshipit-source-id: 17f3edd59bbf4747d17309c44ef12f0d952ea4eb |
||
|
anand76
|
c78144e452 |
Update version to 11.2.0 and add 11.1.fb to format compat (#14509)
Summary: - Bump version.h from 11.1.0 to 11.2.0 - Add `11.1.fb` to `check_format_compatible.sh` ## Remaining manual steps - [x] Cherry-pick HISTORY.md update from release branch - [ ] Update folly hash in Makefile to latest - Follow up in another PR Part of 11.1 release workflow. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14509 Reviewed By: archang19 Differential Revision: D98173862 Pulled By: anand1976 fbshipit-source-id: 020dbb30126d054788ed979f42fabd1be8d97abb |
||
|
Xingbo Wang
|
056a0f9664 |
Add atomic_flush option for checkpoint (#14477)
Summary: Add `force_atomic_flush` to `FlushOptions` so any flush caller (Checkpoint, `DB::Flush()`, etc.) can force atomic flush of all column families even when `DBOptions::atomic_flush` is disabled. ## Changes ### Public API - Added `bool force_atomic_flush = false` to `FlushOptions` - Added `bool atomic_flush = false` to `LiveFilesStorageInfoOptions` ### Flush dispatch - `FlushAllColumnFamilies()` — uses `immutable_db_options_.atomic_flush || flush_options.force_atomic_flush` - `DB::Flush()` (single-CF and multi-CF) — respects `force_atomic_flush` - `FlushForGetLiveFiles()` — constructs `FlushOptions` with `force_atomic_flush` from `LiveFilesStorageInfoOptions::atomic_flush` ### Per-request atomic flush in pipeline - `FlushRequest.atomic_flush` and `BGFlushArg.atomic_flush_` carry the per-request flag - `GenerateFlushRequest`, `EnqueuePendingFlush`, `PopFirstFromFlushQueue`, `BackgroundFlush`, `FlushMemTablesToOutputFiles` all use the per-request flag ### Stress test - Added `--checkpoint_atomic_flush` flag to db_stress, randomly enabled in db_crashtest.py ### Unit tests (6 new) - End-to-end checkpoint with atomic flush override - Negative test (default = no atomic flush) - Single-CF edge case - Interaction with `DBOptions::atomic_flush=true` - Mixed non-atomic then atomic flushes in queue - Mixed atomic then non-atomic flushes in queue Pull Request resolved: https://github.com/facebook/rocksdb/pull/14477 Reviewed By: joshkang97 Differential Revision: D97639233 Pulled By: xingbowang fbshipit-source-id: b924c044d6179e68b38c6604eb46c9140516d42a |
||
|
Josh Kang
|
304ae7190a |
Support ExternalTable PinnableSlice Get (#14497)
Summary: The ExternalTableReader `Get` API has been modified to use PinnableSlice instead of std::string, this will allow implementations to utilize zero-copy Gets (e.g. reading from mmap or a cache). This is not a compatible change, but the API is marked as experimental, so should be allowed. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14497 Test Plan: - New unit tests Reviewed By: xingbowang Differential Revision: D97782011 Pulled By: joshkang97 fbshipit-source-id: 8a9e8c5bc5ff5e8dee6c0f2ee745521f09042cef |
||
Hui Xiao
|
63c86160cd |
Add OnBackgroundJobPressureChanged listener callback (#14474)
Summary: **Summary:** Add a new EventListener callback `OnBackgroundJobPressureChanged` that fires after every flush or compaction background job completes. The callback delivers a `BackgroundJobPressure` snapshot containing: - Compaction scheduling counters (scheduled/running, combined and per-priority LOW/BOTTOM breakdown) - Flush scheduling counters (scheduled/running) - Write stall proximity percentage (0=healthy, 100=at stall threshold, can exceed 100 when stalling) - Whether compaction speedup is active `CaptureBackgroundJobPressure()` reads scheduling counters and computes write stall proximity from L0 sorted run count and pending compaction bytes (same inputs as `RecalculateWriteStallConditions()`). TODO: add memory-related write stall triggers later. Introduces `num_running_bottom_compactions_` counter to track BOTTOM- priority compactions separately from LOW, enabling per-pool breakdown in the pressure snapshot. The callback fires on the background thread after counter decrements and `MaybeScheduleFlushOrCompaction()`, so the snapshot reflects post- completion state. Uses the same mutex unlock/lock pattern as `NotifyOnFlushCompleted`. A `bg_pressure_callback_in_progress_` counter ensures destructor safety since the callback fires after `bg_flush_scheduled_`/`bg_compaction_scheduled_` are decremented. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14474 Test Plan: - listener_test BackgroundJobPressure: 3-phase test verifying no pressure, pressure build-up (speedup, scheduling, proximity), and pressure relief after compaction completes - db_compaction_test CompactRangeBottomPri: verifies num_running_bottom_compactions_ via sync point - db_stress_tool exercises the callback with RandomSleep() Reviewed By: xingbowang Differential Revision: D97423623 Pulled By: hx235 fbshipit-source-id: 07003c8de226ec29d32b8a88e2d86e5de85cd2cc |
||
|
Yoshinori Matsunobu
|
b23fc77aca |
Add per-block-type block read byte perf counters (#14473)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14473 Add separate PerfContext byte counters for data, index, filter, compression-dictionary, and metadata block reads while preserving block_read_byte as the aggregate. Wire the new counters through the block fetch and multi-read data block paths, expose them via the C perfcontext API, and extend table tests to verify byte attribution and that the classified counters sum back to block_read_byte. Reviewed By: xingbowang Differential Revision: D97333746 fbshipit-source-id: 3411844d7fa9c76c9ff28af477b3a72a5d6e5d9b |
||
|
Danny Chen
|
b6f498b2c9 |
Add verify_manifest_content_on_close option (#14451)
Summary: Add a new mutable DB option `verify_manifest_content_on_close` (default: false). When enabled, on DB close the MANIFEST file is read back and all records are validated (CRC checksums via log::Reader and logical content via VersionEdit::DecodeFrom). If corruption is detected, a fresh MANIFEST is written from in-memory state using the existing LogAndApply recovery path. This complements the existing size validation in VersionSet::Close() with content validation, reusing the same manifest reading pattern as VersionSet::Recover(). Implementation plan: ## Part 1: New DB Option — verify_manifest_content_on_close - A new mutable bool DB option (default: false) that can be dynamically toggled via SetDBOptions() at runtime, following the pattern of other mutable manifest options like max_manifest_file_size. - Propagation: SetDBOptions() -> DBImpl::mutable_db_options_ -> versions_->UpdatedMutableDbOptions() -> VersionSet::verify_manifest_content_on_close_ ## Part 2: Core Implementation — Content Validation in VersionSet::Close() - Inserted after existing size check, before closed_ = true - Opens manifest as SequentialFileReader, creates log::Reader with checksum=true - Loops ReadRecord with WALRecoveryMode::kAbsoluteConsistency, decodes each record as VersionEdit - On corruption: fires OnIOError listeners, logs error, calls LogAndApply with empty edit to trigger manifest rewrite from in-memory state - If manifest can't be opened for reading: logs warning, doesn't fail close ## Part 3: Unit Tests (in version_set_test.cc) - ManifestContentValidationOnClose_Clean: enable option, normal close, verify no manifest rotation - ManifestContentValidationOnClose_CorruptRecord: enable option, corrupt manifest via SyncPoint, verify rotation occurs and DB reopens cleanly - ManifestContentValidationOnClose_Disabled: default off, verify content validation does not run - ManifestContentValidationOnClose_SizeCheckFails: truncate manifest so size check fails first, verify recovery via size-check path ## What Happens If a Corruption is Detected If corruption was detected, four things happen: 1. **Notify listeners** — Fires `OnIOError` on all registered event listeners (from db_options_->listeners) so monitoring/alerting systems can observe the corruption event. Uses `FileOperationType::kVerify` to categorize it. 2. **Permit unchecked errors** — `PermitUncheckedError()` silences RocksDB's debug-mode assertion that every `IOStatus` must be inspected. These statuses are informational-only here; the real recovery is via `LogAndApply`. 3. **Log the error** — Writes a `ROCKS_LOG_ERROR` message with the filename for operational visibility (grep-able in production logs). 4. **Rewrite the manifest via `LogAndApply`** — This is the actual recovery. `LogAndApply` is called with an empty `VersionEdit` (no changes). Internally, `LogAndApply` detects that the current `descriptor_log_` is null (it was reset at line 5551, or by the previous `LogAndApply` in the size-check path) and creates a brand-new MANIFEST file. It serializes the entire current in-memory LSM state — all column families, all levels, all file metadata, sequence numbers, etc. — into this new file. It then atomically updates the `CURRENT` file pointer to reference the new MANIFEST. This works because the in-memory state was built from the original manifest during `DB::Open()` and has been kept fully up to date through all subsequent operations (flushes, compactions, etc.) during the DB's lifetime. The on-disk manifest is essentially a journal of changes; `LogAndApply` with an empty edit produces a fresh, compacted snapshot of that state. ## Flow Diagram of Manifest Content Validation VersionSet::Close() │ ├─ Close descriptor_log_ and check size │ └─ Size mismatch? → LogAndApply (rewrite manifest) │ ├─ Content validation (if s.ok() && option enabled) │ ├─ Open manifest for sequential reading │ │ └─ Can't open? → WARN log, continue │ │ │ ├─ For each record: │ │ ├─ ReadRecord (CRC32 check, kAbsoluteConsistency) │ │ └─ DecodeFrom (VersionEdit logical check) │ │ │ └─ Corruption detected? │ ├─ Notify OnIOError listeners │ ├─ LOG_ERROR │ └─ LogAndApply (rewrite manifest from in-memory state) │ └─ closed_ = true; return s; ## How This Relates to the Existing Size Check The existing size check (lines 5556-5582) and the new content validation are complementary: | Check | What it catches | How it checks | |----------------|-----------------------------------------|----------------------------| | Size check | Truncation, partial writes, extra bytes | Compare expected vs actual file size | | Content check | Bit-rot, silent corruption, bad records | CRC32 + VersionEdit decode | The size check catches gross corruption (file too short or too long). The content check catches subtle corruption where the file is the right size but individual bytes have been flipped (e.g., storage media bit-rot, buggy filesystem, incomplete block write). Both recovery paths use the same mechanism: `LogAndApply` with an empty `VersionEdit` to rewrite the manifest from in-memory state. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14451 Reviewed By: xingbowang Differential Revision: D96004906 Pulled By: dannyhchen fbshipit-source-id: 0b0ecdada3a74e97d2cadbba2091b8b577f1d684 |
||
|
zaidoon
|
ec22903914 |
Support all operation types in User Defined Index (UDI) interface (#14399)
Summary: Remove the restriction that limited UDI to ingest-only, Puts-only use cases. This enables UDI plugins (including the trie index from https://github.com/facebook/rocksdb/issues/14310) to work with all operation types: Put, Delete, Merge, SingleDelete, PutEntity, etc. Part of https://github.com/facebook/rocksdb/issues/12396 Pull Request resolved: https://github.com/facebook/rocksdb/pull/14399 Reviewed By: pdillinger Differential Revision: D96392636 Pulled By: xingbowang fbshipit-source-id: 0f1e6c38531fa72539a0e2c6a3dffff333392b4c |
||
|
anand76
|
c66c14258f |
Add memtable MultiGet finger search optimization (#14428)
Summary: Add memtable batch lookup optimization with finger search Optimize memtable MultiGet by using a finger search on the skip list. After finding key[i], the search path (Splice) is retained as a "finger" for key[i+1]. The next search walks up the finger until the forward pointer overshoots, then descends -- costing O(log d) where d is the distance between consecutive sorted keys, rather than O(log N) from the head each time. Controlled by the new `memtable_batch_lookup_optimization` column family option (default: false). ## Changes - Add `FindGreaterOrEqualWithFinger()` and `MultiGet()` to `InlineSkipList` with optional paranoid validation support (key ordering checks and per-key checksum verification via default parameters) - Add virtual `MultiGet()` to `MemTableRep`, override in `SkipListRep` - Add `memtable_batch_lookup_optimization` CF option - Integrate finger search into `MemTable::MultiGet` -- sorts keys, performs batched finger search, then unscrambles results - Add `db_bench`, `db_stress`, and crash test support - Add unit tests for `InlineSkipList::MultiGet` (5 tests) and integration tests at the DB level (25 `BatchLookup` tests), including paranoid validation tests ## Benchmark Results Setup: 2M keys in memtable, release build (`DEBUG_LEVEL=0`). ### Single-threaded `multireadrandom` | Batch Size | Baseline | BatchOpt | vs Base | |:---:|---:|---:|---:| | 2 | 363,593 | 376,562 | **+3.6%** | | 8 | 385,204 | 386,082 | **+0.2%** | | 32 | 360,339 | 375,105 | **+4.1%** | | 64 | 352,696 | 378,497 | **+7.3%** | ### Multithreaded `multireadrandom` (batch_size=64) | Threads | Baseline | BatchOpt | vs Base | |:---:|---:|---:|---:| | 4 | 171,356 | 185,633 | **+8.3%** | | 8 | 161,478 | 171,392 | **+6.1%** | ### `multireadwhilewriting` (batch_size=64) | Threads | Baseline | BatchOpt | vs Base | |:---:|---:|---:|---:| | 1 | 163,938 | 175,068 | **+6.8%** | | 4 | 109,698 | 120,790 | **+10.1%** | | 8 | 116,623 | 123,658 | **+6.0%** | No regression at any batch size or thread count. The finger is stack-allocated per `MultiGet` call, so there is no shared state or cache-line contention between threads. Concurrent writes don't invalidate the finger's bracket since it only reads via acquire-load `Next()` pointers. Small Batch Size Regression Check: memtable_batch_lookup_optimization ===================================================================== All values are ops/sec. Measured with db_bench (release build). 2M keys in memtable, value_size=100, duration=30s per run. Multithreaded multireadrandom — small batch sizes -------------------------------------------------- Threads | Batch Size | Baseline | BatchOpt | Change 4 | 2 | 532,302 | 540,682 | +1.6% 8 | 2 | 1,044,046 | 1,046,920 | +0.3% 4 | 8 | 633,733 | 630,039 | -0.6% 8 | 8 | 1,260,818 | 1,241,792 | -1.5% multireadwhilewriting — small batch sizes ----------------------------------------- Threads | Batch Size | Baseline | BatchOpt | Change 1 | 2 | 107,284 | 105,217 | -1.9% 4 | 2 | 428,508 | 423,747 | -1.1% 8 | 2 | 854,654 | 864,923 | +1.2% 1 | 8 | 114,788 | 118,496 | +3.2% 4 | 8 | 467,995 | 473,437 | +1.2% 8 | 8 | 935,636 | 960,791 | +2.7% No regression at small batch sizes. Variations at batch_size=2 are within noise (~1-2%). At batch_size=8, modest positive trend (+1-3% in read-while-writing). Pull Request resolved: https://github.com/facebook/rocksdb/pull/14428 Test Plan: - `make check` -- all tests pass - `ASSERT_STATUS_CHECKED=1 make check` -- all tests pass - 2-hour `db_crashtest.py blackbox` with `--memtable_batch_lookup_optimization=1` -- passed - 2-hour `db_crashtest.py blackbox` with `--memtable_batch_lookup_optimization=1 --paranoid_memory_checks=1` -- passed Reviewed By: pdillinger Differential Revision: D95813786 Pulled By: anand1976 fbshipit-source-id: b182a023e1026021f1b9682d1cc024c7729326c7 |
||
Jay Huh
|
5494bc1d76 |
Add option to verify file checksum of output files (#14433)
Summary: One of the follow ups from https://github.com/facebook/rocksdb/pull/14103. Users will have the option to verify file checksums for all compaction output files before they are installed. This feature helps prevent corrupted SST files from being added to the LSM tree. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14433 Test Plan: Unit test added Reviewed By: archang19 Differential Revision: D95648000 Pulled By: jaykorean fbshipit-source-id: 512f3c1a7449b96a660865531f3537624c89a9cc |
||
|
Maciej Szeszko
|
1ed5052486 |
Prepopulate block cache during compaction (#14445)
Summary:
When RocksDB operates with tiered or remote storage (e.g., Warm Storage, HDFS, S3), reading recently compacted data incurs high-latency remote reads because compaction output files are not present in the block cache. The existing `prepopulate_block_cache = kFlushOnly` avoids this for flush output but leaves compaction output cold until first access.
Add a new `PrepopulateBlockCache::kFlushAndCompaction` enum value that warms all block types (data, index, filter, compression dict) into the block cache during both flush and compaction. Flush-warmed blocks use `LOW` priority (unchanged from kFlushOnly behavior), while compaction-warmed blocks use `BOTTOM` priority — compaction data is less temporally local than freshly flushed data, so it should be the first to be evicted when the cache is full. This gives the remote-read avoidance benefit without risking cache thrashing.
The enum uses `kFlushAndCompaction` rather than separate `kCompactionOnly` + `kFlushAndCompaction` values because there is no practical use case for warming compaction output without also warming flush output. Flush output is by definition the hottest data (just written by the user), so if a workload benefits from warming the colder compaction output, it would always benefit from warming flush output too.
The implementation reuses the existing `InsertBlockInCacheHelper` / `WarmInCache` infrastructure in `BlockBasedTableBuilder`. The only internal change is adding a `warm_cache_priority` field to `Rep` alongside the existing `warm_cache` bool, and plumbing it through to the `WarmInCache` call instead of the previously hardcoded `Cache::Priority::LOW`.
### Key changes
- New `PrepopulateBlockCache::kFlushAndCompaction` enum value in table.h
- `Rep::warm_cache_priority` field in BlockBasedTableBuilder for per-reason priority control
- Serialization support ("kFlushAndCompaction" in string map)
- db_bench support (--prepopulate_block_cache=2)
- Crash test coverage (random choice includes new value)
**NOTE:** Unlike flush output (which is inherently hot — just written by the user), it is hard to distinguish hot from cold blocks in compaction output. Warming all compaction output therefore risks polluting the block cache and evicting genuinely hot entries. The kFlushAndCompaction mode is recommended only for use cases where most or all of the database is expected to reside in cache (e.g., the working set fits in cache). For workloads where only a fraction of the data is hot, kFlushOnly remains the safer choice.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14445
Test Plan:
- New `WarmCacheWithDataBlocksDuringCompaction` test: verifies data blocks from compaction output are present in the block cache and served without misses
- Extended `DynamicOptions` test: verifies dynamic switching through kDisable -> kFlushAndCompaction -> kFlushOnly -> kDisable via SetOptions
- Existing `WarmCacheWithDataBlocksDuringFlush` and parameterized `WarmCacheWithBlocksDuringFlush` tests continue to pass (kFlushOnly behavior unchanged)
- db_block_cache_test: 81/81 passed
- options_test: 74/74 passed
- table_test: 6910/6910 passed
Reviewed By: xingbowang
Differential Revision: D95997952
Pulled By: mszeszko-meta
fbshipit-source-id: 4ad568264992532053947df298e63e343821ddb5
|
||
|
Josh Kang
|
3ad23b2d94 |
Support automated interpolation search (#14383)
Summary: Add automatic per-block interpolation search selection (`kAuto` mode) for index blocks. During SST construction, each index block's key distribution is analyzed using the coefficient of variation (CV) of gaps between restart-point keys. Blocks with uniformly distributed keys are flagged via a new bit in the data block footer, and at read time, `kAuto` resolves to interpolation search for uniform blocks and binary search otherwise. ## Key changes - **New `BlockSearchType::kAuto` enum value**: Resolves per-block at read time to either `kInterpolation` or `kBinary` based on the block's uniformity flag. Falls back to `kBinary` on older versions that don't recognize it. - **Write-path uniformity analysis**: `BlockBuilder::ScanForUniformity()` uses Welford's online algorithm to incrementally compute the CV of key gaps at restart points. The result is stored in a new bit (bit 30) of the data block footer's packed restart count. - **New table option `uniform_cv_threshold`** (default: -1 `disabled`): Controls how strict the uniformity check is. Set to negative to disable. Exposed in C++, Java (JNI), and `db_bench`. - **Code reorganization**: Block entry decode helpers (`DecodeEntry`, `DecodeKey`, `DecodeKeyV4`, `ReadBe64FromKey`) moved from `block.cc` to a new shared header `block_util.h` so they can be reused by `BlockBuilder` on the write path. - **New histogram `BLOCK_KEY_DISTRIBUTION_CV`**: Records the CV (scaled by 10000) of each index block's key distribution for observability. - **Java bindings**: `IndexSearchType.kAuto`, `uniformCvThreshold` getter/setter, JNI portal constructor signature updated, and `HistogramType.BLOCK_KEY_DISTRIBUTION_CV` added. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14383 Test Plan: - `IndexBlockTest.IndexValueEncodingTest` parameterized to include `kAuto` search type alongside `kBinary` and `kInterpolation`, verifying correct seek/iteration behavior across all combinations of key distributions, restart intervals, and key lengths. - Uniformity detection validated: blocks with uniform key distribution correctly set `is_uniform = true`, blocks with clustered/non-uniform keys set `is_uniform = false`. - Stress test coverage - Updated check_format_compatible to also include a "uniform" dataset. By default using uniform_cv_threshold=-1 does not result in an incompatibility issues. When manually changing the threshold (e.g. `uniform_cv_threshold=1000`), I see `bad block contents`, which is expected ## Benchmark readrandom with `fillrandom,compact -seed=1 --statistics`: | Benchmark | Branch | Params | avg ops/s | % change vs main | CV P50 | |-----------|--------|--------|-----------|------------------|--------| | readrandom | main | `binary_search, shortening=1` | 335,791 | baseline | N/A | | readrandom | feature | `binary_search, shortening=1` (default) | 335,749 | -0.0% | 1,500 | | readrandom | feature | `auto_search, shortening=1` (kAuto) | 366,832 | **+9.2%** | 1,500 | | readrandom | feature | `interpolation_search, shortening=1` | 366,598 | **+9.2%** | 1,500 | | readrandom | feature | `auto_search, shortening=2` (kAuto) | 344,631 | **+2.6%** | 1,030,000 | | readrandom | feature | `interpolation_search, shortening=2` | 201,178 | **-40.1%** | 1,030,000 | As seen with shortening=2, a non-uniform distribution produces a high CV, which does not use interpolation search. ## Write benchmark There is a write overhead which scans each restart entry for a block upon Finish. In practice this is very low because currently it is only applied to index blocks. See cpu profile (https://fburl.com/strobelight/io5hwj9h) here of `-benchmarks=fillseq,compact -compression_type=none -disable_wal=1`. Only 0.08% attributed to `ScanForUniformity`. Reviewed By: pdillinger Differential Revision: D94738890 Pulled By: joshkang97 fbshipit-source-id: 9661ac593c5fef89d49f3a8a027f1338a0c96766 |
||
|
Jay Huh
|
3aa706c2bf |
Enforce WriteBufferManager during WAL recovery (#14305)
Summary: Add a new immutable DB option `enforce_write_buffer_manager_during_recovery` to control whether WriteBufferManager buffer_size is enforced during WAL recovery. When multiple RocksDB instances share a WriteBufferManager, a recovering instance could exceed the global memory limit by replaying large amounts of WAL data into memtables. This can cause OOM, especially when other instances are actively using the shared memory budget. When this option is enabled (and a WriteBufferManager is configured), RocksDB will check `WriteBufferManager::ShouldFlush()` after each batch insertion during WAL recovery and schedule flushes when needed to keep memory bounded. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14305 Test Plan: Two unit tests added - `WriteBufferManagerLimitDuringWALRecoverySingleDB` and `WriteBufferManagerLimitDuringWALRecoveryMultipleDBs` ``` ./db_write_buffer_manager_test --gtest_filter="*WriteBufferManagerLimitDuringWALRecovery*" ``` Basic stress test added with options toggled ## To follow up Multiple DB scenario in stress test Reviewed By: hx235 Differential Revision: D92533792 Pulled By: jaykorean fbshipit-source-id: 35ca70e2300a8bfd6b81a6646a65ef284fb90a9a |
||
|
zaidoon
|
16c81a27e9 |
Extend UDI trie index with seqno side-table for same-user-key block boundaries (#14412)
Summary: When the same user key spans adjacent data blocks with different sequence numbers, the trie index cannot distinguish them because it only stores user keys. This adds a side-table that stores per-leaf sequence numbers and overflow block metadata, enabling correct post-seek correction using seqno comparison. Design: - Trie always stores user-key-only separators (unchanged) - Duplicate separators from same-key boundaries are de-duplicated - Side-table stores leaf_seqnos[], leaf_block_counts[], and overflow arrays (offsets, sizes, seqnos) serialized after the trie data - Post-seek correction: after trie Seek lands on a leaf, compare target_seq vs leaf_seqno to decide whether to advance through overflow blocks or to the next trie leaf - Zero overhead when no same-user-key boundaries exist (no side-table serialized, no seqno checks at seek time) Public API changes (user_defined_index.h): - AddIndexEntry: single pure virtual with IndexEntryContext parameter (replaces old 4-arg version). Context carries last_key_seq and first_key_seq for block boundaries. - SeekAndGetResult: single pure virtual with SeekContext parameter (replaces old 2-arg version). Context carries target_seq. - Fix trailing semicolons on NewBuilder/NewReader default overrides. Wrapper layer (user_defined_index_wrapper.h): - Extract sequence numbers from parsed internal keys and pass via context structs to UDI builder/iterator. - Fix CurrentIndexSizeEstimate() to delegate to internal builder (was returning 0). - Fix ApproximateMemoryUsage() to include UDI reader memory. - Fix typos: lof_err_key -> log_err_key, COuld -> Could, "Bad index name" -> "Bad index name: ". Trie builder (trie_index_factory.cc): - Buffer separator entries during building; at Finish(), detect whether any same-user-key boundary was seen (sticky flag, same strategy as ShortenedIndexBuilder::must_use_separator_with_seq_). - When seqno encoding is needed, re-encode all separators with seqno side-table metadata in the trie. - Default null comparator to BytewiseComparator() to prevent crash. Trie iterator (trie_index_factory.cc): - Post-seek correction: compare target_seq vs leaf_seqno to advance through overflow runs. - Overflow run state tracking: overflow_run_index_, overflow_run_size_, overflow_base_idx_ for O(1) access to overflow block handles. - NextAndGetResult advances within overflow runs before moving to next trie leaf. - Return kOutOfBound instead of kUnknown when Seek/Next finds no blocks. LOUDS trie (louds_trie.h, louds_trie.cc): - Seqno side-table: builder AddKeyWithSeqno/AddOverflowBlock methods, BFS reordering of seqno/block_count arrays, serialization/deserialization of per-leaf seqnos, block counts, overflow handles/seqnos, and overflow_base_ prefix sum for O(1) access. - AppendKeySlot() helper with debug bounds assert on all key-append sites. Dead code removal: - LoudsTrieBuilder::NumKeys() (never called externally) - sparse_leaf_count_ (serialized but never read by the reader) - DenseChildNodeNum(pos), DenseLeafIndex(pos), DenseNodeNum(pos) (superseded by FromRank variants that avoid redundant Rank1 calls) Deserialization hardening (all in InitFromData, cold path only): - Move num_keys_ validation before any dependent arithmetic. - Validate dense bitvector sizes match dense_node_count_ (d_labels must be node_count*256, d_has_child must equal d_labels.NumOnes(), d_is_prefix_key must equal node_count). - Validate sparse bitvector sizes match s_labels_size_ (s_has_child and s_louds must have the same number of bits as the label array). - Validate child position table values within s_labels_size_ bounds. - Validate chain suffix offset+length within suffix data blob. - Validate chain end child indices < num_internal or == UINT32_MAX. - EliasFano: add count_ upper-bound check (<=2^30) to prevent count_*low_bits_ integer overflow. - Bitvector: validate select hint values < num_rank_samples_ to prevent OOB in FindNthOneBit/FindNthZeroBit. - EliasFano: custom move ctor/assignment to re-seat low_words_ after owned_low_data_ move (fixes dangling pointer for SSO strings). https://github.com/facebook/rocksdb/issues/14406 Pull Request resolved: https://github.com/facebook/rocksdb/pull/14412 Reviewed By: anand1976 Differential Revision: D95160933 Pulled By: xingbowang fbshipit-source-id: f2c3681c1059c03d540ce1cb9cc14cc79cd9730c |
||
|
Xingbo Wang
|
15f2f2bf6f |
Relax option sanitization for kv ratio compaction (#14397)
Summary: The kv ratio compaction option sanitization is too strict. Sometimes, it blocks engine start up. Relax the validation and convert it to runtime check. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14397 Test Plan: Unit test Reviewed By: pdillinger Differential Revision: D94709471 Pulled By: xingbowang fbshipit-source-id: cc076c397b3acfa426112063224771a196684798 |