Summary:
- Add `tools/gtest_parallel_repro.py` to reproduce gtest flaky tests that depend on process-level CPU contention and scheduler delays.
- Run many fresh gtest processes concurrently, isolate each process with its own `TEST_TMPDIR`, and optionally pin the workload to a smaller CPU set with `taskset`.
- Record per-run logs, write `failures.jsonl`, summarize failure keys, support stopping on first failed batch, and optionally rebuild with `COERCE_CONTEXT_SWITCH=1`.
- Document when to use the runner in `CLAUDE.md` for CI-style flaky tests that single-process `--gtest_repeat` or coerce mode alone does not reproduce.
- Using the new runner, we found and fixed one flaky test and improved c_test harness
- Flaky test: EnvPosixTest.ReadAsyncQueueFull simulated a full io_uring submission queue by overwriting the SQE pointer after io_uring_get_sqe() had already consumed a submission slot. That left stale state in the thread-local ring, so later AbortIO tests could process an unexpected completion and crash. Add a skip_io_uring_get_sqe syncpoint before the io_uring_get_sqe() call and use it from the test so the Busy path is exercised without mutating the ring.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14827
Test Plan: - CI
Reviewed By: pdillinger
Differential Revision: D107758097
Pulled By: xingbowang
fbshipit-source-id: 3256612ae1ae98b66901593371743b3de7e5bb53
Summary:
Add read-scoped block buffers for scan reads. Introduce an experimental read-scoped block buffer provider API, configured through ReadOptions::read_scoped_block_buffer_provider, so supported block-based table iterator scans and MultiScan data-block reads can use caller-provided read-scoped storage for final data-block contents.
When configured, supported provider-backed scan data-block reads bypass the data-block cache while preserving normal index/filter block-cache behavior. Known-uncompressed reads can attach provider cleanup to provider-backed read buffers without copying. Compressed reads decompress directly into provider-backed output, while maybe-compressed reads that turn out to be uncompressed copy once into provider-backed final contents. mmap reads ignore the provider.
Extend AlignedBuffer and RandomAccessFileReader direct-I/O paths to support external aligned allocations, then use that support for read-scoped iterator, async I/O, and MultiRead scratch buffers. Centralize read-scoped I/O policy, keep coalesced async reads safe when blocks are released before completion, and validate provider lease contracts.
Add focused coverage for read-scoped ownership, compressed and uncompressed blocks, direct I/O, data-block cache bypass behavior, invalid provider leases, async release handling, and stress-test provider invariants. Add public API release notes for read-scoped block buffers.
Bonus change: Fixed a flaky test in ReserveThread
## Testing
- CI
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14806
Reviewed By: anand1976
Differential Revision: D106999951
Pulled By: xingbowang
fbshipit-source-id: b1f23d4bab6318b6373ba2ca99a5c4d6a842dc5a
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14739
Adds public path-level file sync APIs to RocksDB so callers can ask `Env` or `FileSystem` to sync a named file without hand-rolled reopen logic.
This diff:
- Adds public `Env::SyncFile` and `FileSystem::SyncFile` APIs for syncing or fsyncing a file by name without requiring callers to reopen it directly.
- Documents the `SyncFile` contract: filesystems may override it as a no-op when flush/close already provides durability, and RocksDB callers should use it instead of hand-rolled `ReopenWritableFile()+Sync()/Fsync()` so filesystems can reject post-close data-write reopen paths.
- Implements the default `Env` and `FileSystem` behavior by reopening the file as writable, calling `Sync` or `Fsync`, closing the handle, returning the sync/fsync error ahead of a close error when both fail, and explicitly consuming the close status on the sync-failure path.
- Wires `SyncFile` through the Env/FileSystem bridge and wrapper layers, including `EnvWrapper`, `FileSystemWrapper`, `CompositeEnvWrapper`, `EncryptedFileSystem`, `RemapFileSystem`, read-only file systems, mock file systems, and fault-injection wrappers.
- Keeps fault-injection `SyncFile` overrides on the base default implementation so the operation still exercises the wrapper's `ReopenWritableFile`, wrapped-file `Sync`/`Fsync`, and `Close` hooks instead of bypassing them through target forwarding.
- Updates external SST ingestion to call `FileSystem::SyncFile` instead of manually reopening an ingested file as writable, while preserving the `NotSupported` skip behavior and failure logging.
- Adds release-note coverage for the new public API and unit coverage for default `Env`/`FileSystem` success, reopen failure, close failure, sync-versus-close failure precedence, and external SST sync behavior.
There was an earlier version of this API which got reverted (PR #13987) due to internal change broken. Re-apply the change. The internal issue will be resolved in next release.
Reviewed By: pdillinger
Differential Revision: D104918547
fbshipit-source-id: 3f8d2d127fc962b68b423bbb90de551fe6706224
Summary:
Fix flaky test failures in EnvPosixTestWithParam.AllocateTest and DBWALTest.TruncateLastLogAfterRecoverWithFlush that occur when running on filesystems where preallocated space is not reliably reflected in st_blocks.
The tests use stat() to check st_blocks and verify that fallocate with FALLOC_FL_KEEP_SIZE actually preallocates disk space. However, on certain filesystems, this check is unreliable:
1. btrfs and zfs: Copy-on-write filesystems where preallocated extents are not reliably reflected in st_blocks, especially under load.
2. tmpfs: Memory filesystem that may not report preallocated blocks in st_blocks.
3. overlayfs: Union filesystem common in containers that may not pass through fallocate properly or report preallocated space.
4. Any filesystem where FALLOC_FL_KEEP_SIZE is not supported: The preallocation will fail silently (error ignored with PermitUncheckedError), leaving only the written data in st_blocks.
Changes made:
env/env_test.cc:
- Added filesystem magic number definitions for TMPFS_MAGIC, OVERLAYFS_SUPER_MAGIC, and ZFS_SUPER_MAGIC
- Extended the AllocateTest to skip block count checks on zfs, tmpfs, and overlayfs in addition to btrfs
- Added runtime fallback: if st_blocks is less than expected, print a warning and skip the check instead of failing. This handles unknown filesystems or configurations where preallocation isn't supported.
db/db_wal_test.cc:
- Added includes and filesystem magic number definitions
- Added ShouldSkipAllocationCheck() helper function to detect problematic filesystems
- Modified TruncateLastLogAfterRecoverWithoutFlush, TruncateLastLogAfterRecoverWithFlush, TruncateLastLogAfterRecoverWALEmpty, and ReadOnlyRecoveryNoTruncate tests to skip allocation checks on problematic filesystems
- Added runtime fallback checks similar to env_test.cc
These changes make the tests robust against filesystem differences while still validating preallocation behavior on filesystems where it works correctly (ext4, xfs).
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14744
Test Plan: Many local 'make -j100 check' runs that would previously fail with good probability.
Reviewed By: hx235
Differential Revision: D105331256
Pulled By: pdillinger
fbshipit-source-id: 862a1512da1466cb037af15342404939b677c02a
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
Summary:
TSAN was reporting false data-race warnings after the kernel reused a virtual address for a new mapping while TSAN still associated that address range with the old mapping.
This showed up in two RocksDB paths:
- `io_uring` setup on helper threads vs later `io_uring` `MultiRead` access
- `io_uring` setup vs file mmap reads when `use_mmap_reads` is enabled
## Changes
Introduce `TsanAnnotateMappedMemory()`, which calls `AnnotateNewMemory()` as soon as a fresh mapping exists so TSAN drops any stale shadow state for the recycled address range.
Apply the helper to:
- io_uring SQ/CQ/SQE mappings created by `CreateIOUring()`
- `PosixFileSystem` mmap-read mappings
- `PosixFileSystem` raw memory-mapped file buffers
- `PosixMmapFile` writable mmap region growth
Also add deterministic regression tests that force virtual-address reuse with `MAP_FIXED` for both the io_uring and mmap-read cases. The tests pass mapping metadata over a pipe so teardown/remap ordering is deterministic without introducing TSAN-visible synchronization that would mask the problem.
## Testing
- `COMPILE_WITH_TSAN=1 make -j192 env_test`
- `env_test --gtest_filter='EnvPosixTest.SupportedOpsNoAsyncIOOnIOUringInitFailure:EnvPosixTest.IOUringAddressReuseNoTsanFalsePositive:EnvPosixTest.MmapReadAddressReuseNoTsanFalsePositive'`
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14594
Reviewed By: archang19
Differential Revision: D100235746
Pulled By: xingbowang
fbshipit-source-id: 8ef8d85c08a7646540b9c11c3741978898a5af3d
Summary:
btrfs accepts fallocate without error but uses copy-on-write, so preallocated extents are not reflected in st_blocks. This caused AllocateTest to fail spuriously on btrfs filesystems. Detect btrfs via statfs and skip only the block-count assertions, keeping the rest of the test (write, flush, close, size checks) intact.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14553
Test Plan: env_test AllocateTest passes on btrfs (skips with message) and would still enforce preallocation checks on ext4.
Reviewed By: joshkang97
Differential Revision: D99307962
Pulled By: pdillinger
fbshipit-source-id: 8f40c0109aa397e9ca7d9ee4496fa0614e971970
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14566
**What**: Adds an explicit `file->Close()` call in `WriteStringToFile()` before
the function returns, instead of relying on the `unique_ptr` destructor to close
the file.
**How**: In `file_system.cc`, after the optional `Sync()` and before the
error-cleanup path, calls `file->Close()`. If `Close()` fails, the file is
deleted just like any other write failure. Adds two unit tests in `env_test.cc`:
- `WriteStringToFileClosesFile`: uses `CountedFileSystem` to verify `Close()` is
called and content is written correctly.
- `WriteStringToFileCloseFailureDeletesFile`: uses a custom `CloseFailFS`
wrapper to inject a `Close()` failure and verify the file is deleted on error.
**Why**: The previous implementation never called `Close()` explicitly — it
relied on the `unique_ptr` destructor which may silently swallow write errors
that occur during close (e.g., flushing buffered data). Explicit `Close()`
ensures such errors are detected and propagated to the caller.
Reviewed By: archang19
Differential Revision: D99462173
fbshipit-source-id: 525b5489bd0dbfc3159b007a13f9474f84d3c84e
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14521
ReadAsync calls io_uring_get_sqe() without checking for nullptr. When the
io_uring submission queue is full (outstanding completions not yet reaped),
io_uring_get_sqe returns NULL and the subsequent io_uring_prep_readv
dereferences it, causing a segfault.
MultiRead already handles this correctly by using io_uring_sq_space_left()
to cap submissions. ReadAsync submits exactly one SQE per call so a simple
null check with error return is sufficient.
On null sqe, clean up the already-allocated Posix_IOHandle and return
IOStatus::Busy so the caller can retry after reaping completions.
The io_uring queue depth is kIoUringDepth (256), and each thread gets its
own io_uring instance via thread-local storage. In practice the SQ rarely
fills because ReadAsync calls io_uring_submit() after each io_uring_get_sqe(),
immediately flushing the SQE to the kernel. The null SQE would only occur
under unusual kernel backpressure where the kernel cannot consume from the
SQ ring fast enough.
IOStatus::Busy was chosen (over IOError) because this is a transient
condition. The caller has two options:
1. Call Poll() to reap outstanding completions from the CQ, then retry
ReadAsync. This mirrors how MultiRead handles queue pressure internally
by capping submissions and reaping between batches.
2. Fall back to synchronous Read(). Existing callers (FilePrefetchBuffer,
IODispatcher) already have synchronous fallback paths for non-OK
ReadAsync status, so IOStatus::Busy naturally triggers that fallback
without additional code changes. Given the rarity of this condition,
the synchronous fallback is pragmatic and avoids adding retry complexity.
Also adds a TEST_SYNC_POINT_CALLBACK on io_uring_get_sqe to enable test
injection, and a new ReadAsyncQueueFull unit test that uses SyncPoint to
force a null SQE and verifies the Busy return, handle cleanup, and no crash.
Reviewed By: xingbowang
Differential Revision: D98533853
fbshipit-source-id: f6d181e5c0d5154b570ff6da39e2f52e2a6aea84
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14514
SupportedOps previously checked only that the ThreadLocalPtr container existed (non-null), which was set during construction based on a one-time probe on the main thread. However, CreateIOUring() can fail on other threads due to kernel resource limits or flag incompatibilities, causing ReadAsync to hit "failed to init io_uring" at runtime.
Now SupportedOps eagerly initializes the thread-local io_uring instance via Get() + CreateIOUring() and only advertises kAsyncIO if it succeeds. Also logs to stderr (with thread id) when io_uring init fails, and adds a TEST_SYNC_POINT_CALLBACK for testing simulated CreateIOUring failures.
Reviewed By: mszeszko-meta, xingbowang
Differential Revision: D98409140
fbshipit-source-id: efa92d9ac920860e95a46710c4a87e36bacbb466
Summary:
and remove deprecated DB::MaxMemCompactionLevel(). In the process of pushing through a relatively clean refactoring of uses of the old functions, some other minor public APIs are also migrated from raw DB pointers to unique_ptr.
Claude did pretty much all the work, but requiring dozens of prompts to actually push through relatively clean phase out of raw DB pointers from what needed to be touched, and leaving that code in better shape. (Hundreds of `DB*` still remain all over the place even outside C and Java bindings.)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14335
Test Plan: existing tests; no functional changes intended
Reviewed By: xingbowang, mszeszko-meta
Differential Revision: D93523820
Pulled By: pdillinger
fbshipit-source-id: e4ca22ad81cd2cfe91122d7507d7ca34fe03d043
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14319
The test asserted h1->is_finished and h2->is_finished immediately after AbortIO({H0}), before calling Poll. This is invalid because AbortIO only guarantees that handles in its abort set are finalized. Non-aborted handles' CQEs may or may not be consumed during AbortIO depending on io_uring completion ordering. If H0's two CQEs (original read + cancel) arrive before H1/H2's CQEs, AbortIO breaks out of its wait loop without processing them. Move the H1/H2 is_finished assertions to after Poll, which correctly handles either case. Also remove the racy req_count checks for non-aborted handles since Poll does not increment req_count.
Reviewed By: jaykorean
Differential Revision: D92848827
fbshipit-source-id: 0c09b44ceada99877e8311cff799fa94f1056545
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14301
When AbortIO was called with a subset of outstanding async read handles,
it would consume io_uring completions for handles NOT in the abort set
but fail to finalize them. This caused subsequent Poll calls on those
handles to hang forever waiting for completions that had already been
consumed.
The fix adds an `is_being_aborted` flag to Posix_IOHandle that is set
when submitting the cancel request. When processing completions in
AbortIO, handles with this flag wait for req_count==2 (original + cancel),
while handles without the flag are finalized immediately at req_count==1.
Also refactored the completion finalization logic into a shared
FinalizeAsyncRead() helper function used by both Poll and AbortIO.
Reviewed By: mszeszko-meta, archang19
Differential Revision: D92230883
fbshipit-source-id: e6d11e009a4930e5608459771990f6cf7d46d827
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14252
Fixed a bug in PosixFileSystem::AbortIO that could cause an infinite hang
when aborting multiple concurrent async IO handles.
The bug occurred in the completion processing loop: when an io_uring
completion arrived for a handle other than the one currently being waited
for (io_handles[i]), the code would increment that handle's req_count but
only mark it as finished if it also matched io_handles[i]. This meant
completions for other handles were consumed but those handles were never
marked as finished.
Later, when iterating to those handles, the code would enter
io_uring_wait_cqe expecting more completions, but they had already been
consumed - causing an infinite hang.
The fix aligns AbortIO's completion handling with what Poll() already does:
mark handles as finished whenever their completions arrive, regardless of
which handle we're currently waiting for in the outer loop. Only the break
statement remains conditional on matching io_handles[i].
Reviewed By: anand1976
Differential Revision: D91070044
fbshipit-source-id: 47faf5f0df3e26a2aa83444bbac623f43f560933
Summary:
`PosixRandomAccessFile::MultiRead` was introduced in Dec 2019 in https://github.com/facebook/rocksdb/pull/5881. Subsequently, 2 years after, we introduced the `PosixRandomAccessFile::ReadAsync` API in https://github.com/facebook/rocksdb/pull/9578, which was reusing the same `PosixFileSystem` IO ring as `MultiRead` API, consequently writing to the very same ring's submission queue (without waiting!). This 'shared ring' design is problematic, since sequentially interleaving `ReadAsync` and `MultiRead` API calls on the very same thread might result in reading 'unknown' events in `MultiRead` leading to `Bad cqe data` errors (and therefore falsely perceived as a corruption) - which, for some services (running on local flash), in itself is a hard blocker for adopting RocksDB async prefetching ('async IO') that heavily relies on the `ReadAsync` API. This change aims to solve this problem by maintaining separate thread local IO rings for `async reads` and `multi reads` assuring correct execution. In addition, we're adding more robust error handling in form of retries for kernel interrupts and draining the queue when process is experiencing terse memory condition. Separately, we're enhancing the performance aspect by explicitly marking the rings to be written to / read from by a single thread (`IORING_SETUP_SINGLE_ISSUER` [if available]) and defer the task just before the application intends to process completions (`IORING_SETUP_DEFER_TASKRUN` [if available]). See https://man7.org/linux/man-pages/man2/io_uring_setup.2.html for reference.
## Benchmark
**TLDR**
There's no evident advantage of using `io_uring_submit` (relative to proposed `io_uring_submit_and_wait`) across batches of size 10, 250 and 1000 simulating significantly-less, close-to and 4x-above `kIoUringDepth` batch size. `io_uring_submit` might be more appealing if (at least) one of the IOs is slow (which was NOT the case during the benchmark). More notably, with this PR switching from `io_uring_submit_and_wait` -> `io_uring_submit` can be done with a single line change due to implemented guardrails (we can followup with adding optional config for true ring semantics [if needed]).
**Compilation**
```
DEBUG_LEVEL=0 make db_bench
```
**Create DB**
```
./db_bench \
--db=/db/testdb_2.5m_k100_v6144_16kB_LZ4 \
--benchmarks=fillseq \
--num=2500000 \
--key_size=100 \
--value_size=6144 \
--compression_type=LZ4 \
--block_size=16384 \
--seed=1723056275
```
**LSM**
* L0: 2 files, L1: 5, L2: 49, L3: 79
* Each file is roughly ~35M in size
### MultiReadRandom (with caching disabled)
Each run was preceded by OS page cache cleanup with `echo 1 | sudo tee /proc/sys/vm/drop_caches`.
```
./db_bench \
--use_existing_db=true \
--db=/db/testdb_2.5m_k100_v6144_16kB_LZ4 \
--compression_type=LZ4 \
--benchmarks=multireadrandom \
--num= **<N>** \
--batch_size= **<B>** \
--io_uring_enabled=true \
--async_io=false \
--optimize_multiget_for_io=false \
--threads=4 \
--cache_size=0 \
--use_direct_reads=true \
--use_direct_io_for_flush_and_compaction=true \
--cache_index_and_filter_blocks=false \
--pin_l0_filter_and_index_blocks_in_cache=false \
--pin_top_level_index_and_filter=false \
--prepopulate_block_cache=0 \
--row_cache_size=0 \
--use_blob_cache=false \
--use_compressed_secondary_cache=false
```
| B=10; N=100,000 | B = 250; N=80,000 | B = 1,000; N=20,000
-- | -- | -- | --
baseline | 31.5 (± 0.4) us/op | 17.5 (± 0.5) us/op | 13.5 (± 0.4) us/op
io_uring_submit_and_wait | 31.5 (± 0.6) us/op | 17.7 (± 0.4) us/op | 13.6 (± 0.4) us/op
io_uring_submit | 31.5 (± 0.6) us/op | 17.5 (± 0.5) us/op | 13.4 (± 0.45) us/op
### Specs
| Property | Value
-- | --
RocksDB | version 10.9.0
Date | Tue Dec 9 15:57:03 2025
CPU | 56 * Intel Sapphire Rapids (T10 SPR)
Kernel version | 6.9.0-0_fbk12_0_g28f2d09ad102
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14158
Reviewed By: anand1976
Differential Revision: D88172809
Pulled By: mszeszko-meta
fbshipit-source-id: 5198de3d2f18f76fee661a2ec5f447e79ba06fbd
Summary:
There are some internal use cases that do not map cleanly onto the existing `IOActivity` enums. This PR creates new custom IOActivity types that internal users can use as they see fit.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/13924
Test Plan: Wrote a simple unit test
Reviewed By: pdillinger
Differential Revision: D82029992
Pulled By: archang19
fbshipit-source-id: a3e23c360baa96cd2e9adf570e71c6e43947bfc8
Summary:
Add file size validation in ReadFooterFromFile function.
Deprecate skip_checking_sst_file_sizes_on_db_open option.
This change is used to address this issue
https://github.com/facebook/rocksdb/issues/13619
It supports file size validation in ReadFooterFromFile. In favor of this
change, CheckConsistency function and
skip_checking_sst_file_sizes_on_db_open flag are deprecated.
The CheckConsistency function checks each file size matches what was
recorded in manifest during DB open. Meantime, ReadFooterFromFile was
called for each file in LoadTables function. Since ReadFooterFromFile
always validates file size, the CheckConsistency is redundant.
In addtion, CheckConsistency is executed in a single thread. This could
slow down DB open when a network file system is used. Therefore, the
flag skip_checking_sst_file_sizes_on_db_open was added to skip this
check. After this change, ReadFooterFromFile was executed in parallel
through multiple threads. Therefore, the concern of DB open slowness is
eliminated, and the flag could be deprecated.
When paranoid check flag is set to true, corrupted file will fail to open the DB.
When paranoid check flag is set to false, DB will still be able to open, the
healthy ones can be accessed, while the corrupted ones not.
There is 2 slight concerns of this change.
*If max_open_files is set with smaller value, engine will not open all
the files during DB open. This means if there is a corruption on file
size, it will not be detected during DB open, but rather at a later
time. Since the default is -1, which means open all the files, and it is
rarely overridden and a lot of new features rely on it to be -1, the
risk is very low.
*If FIFO compaction is used, engine could fail to open DB unnecessarily
on the corrupted files that would never be used again. However, this is
a very rare case as well. The error could still be ignored by setting
paranoid_checks operationally. The risk is very low.
To remain backward compatibility. The public facing flag was kept and
marked as no-op internally. Another change is required to fully remove
the flag.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/13676
Test Plan:
make check
A new unit test was added to validate file size check API works as
expected.
Reviewed By: pdillinger
Differential Revision: D76168033
Pulled By: xingbowang
fbshipit-source-id: 8ceacf39bcfe02ff7aa289868c341366ee9f3a8e
Summary:
... which is the default for CentOS 9 and Ubuntu 24, the latter of which is now available in GitHub Actions. Relevant CI job updated.
Re-formatted all cc|c|h files except in third-party/, using
```
clang-format -i `git ls-files | grep -E '[.](cc|c|h)$' | grep -v third-party/`
```
Pull Request resolved: https://github.com/facebook/rocksdb/pull/13233
Test Plan: CI
Reviewed By: jaykorean, archang19
Differential Revision: D67461638
Pulled By: pdillinger
fbshipit-source-id: 0c9ac21a3f5eea6f5ade68bb6af7b6ba16c8b301
Summary:
Modify ReadAsync callback API to remove const from FSReadRequest as const doesn't let to fs_scratch to move the ownership.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11649
Test Plan: CircleCI jobs
Reviewed By: anand1976
Differential Revision: D53585309
Pulled By: akankshamahajan15
fbshipit-source-id: 3bff9035db0e6fbbe34721a5963443355807420d
Summary:
The following are risks associated with pointer-to-pointer reinterpret_cast:
* Can produce the "wrong result" (crash or memory corruption). IIRC, in theory this can happen for any up-cast or down-cast for a non-standard-layout type, though in practice would only happen for multiple inheritance cases (where the base class pointer might be "inside" the derived object). We don't use multiple inheritance a lot, but we do.
* Can mask useful compiler errors upon code change, including converting between unrelated pointer types that you are expecting to be related, and converting between pointer and scalar types unintentionally.
I can only think of some obscure cases where static_cast could be troublesome when it compiles as a replacement:
* Going through `void*` could plausibly cause unnecessary or broken pointer arithmetic. Suppose we have
`struct Derived: public Base1, public Base2`. If we have `Derived*` -> `void*` -> `Base2*` -> `Derived*` through reinterpret casts, this could plausibly work (though technical UB) assuming the `Base2*` is not dereferenced. Changing to static cast could introduce breaking pointer arithmetic.
* Unnecessary (but safe) pointer arithmetic could arise in a case like `Derived*` -> `Base2*` -> `Derived*` where before the Base2 pointer might not have been dereferenced. This could potentially affect performance.
With some light scripting, I tried replacing pointer-to-pointer reinterpret_casts with static_cast and kept the cases that still compile. Most occurrences of reinterpret_cast have successfully been changed (except for java/ and third-party/). 294 changed, 257 remain.
A couple of related interventions included here:
* Previously Cache::Handle was not actually derived from in the implementations and just used as a `void*` stand-in with reinterpret_cast. Now there is a relationship to allow static_cast. In theory, this could introduce pointer arithmetic (as described above) but is unlikely without multiple inheritance AND non-empty Cache::Handle.
* Remove some unnecessary casts to void* as this is allowed to be implicit (for better or worse).
Most of the remaining reinterpret_casts are for converting to/from raw bytes of objects. We could consider better idioms for these patterns in follow-up work.
I wish there were a way to implement a template variant of static_cast that would only compile if no pointer arithmetic is generated, but best I can tell, this is not possible. AFAIK the best you could do is a dynamic check that the void* conversion after the static cast is unchanged.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/12308
Test Plan: existing tests, CI
Reviewed By: ltamasi
Differential Revision: D53204947
Pulled By: pdillinger
fbshipit-source-id: 9de23e618263b0d5b9820f4e15966876888a16e2
Summary:
## Context/Summary
Similar to https://github.com/facebook/rocksdb/pull/11288, https://github.com/facebook/rocksdb/pull/11444, categorizing SST/blob file write according to different io activities allows more insight into the activity.
For that, this PR does the following:
- Tag different write IOs by passing down and converting WriteOptions to IOOptions
- Add new SST_WRITE_MICROS histogram in WritableFileWriter::Append() and breakdown FILE_WRITE_{FLUSH|COMPACTION|DB_OPEN}_MICROS
Some related code refactory to make implementation cleaner:
- Blob stats
- Replace high-level write measurement with low-level WritableFileWriter::Append() measurement for BLOB_DB_BLOB_FILE_WRITE_MICROS. This is to make FILE_WRITE_{FLUSH|COMPACTION|DB_OPEN}_MICROS include blob file. As a consequence, this introduces some behavioral changes on it, see HISTORY and db bench test plan below for more info.
- Fix bugs where BLOB_DB_BLOB_FILE_SYNCED/BLOB_DB_BLOB_FILE_BYTES_WRITTEN include file failed to sync and bytes failed to write.
- Refactor WriteOptions constructor for easier construction with io_activity and rate_limiter_priority
- Refactor DBImpl::~DBImpl()/BlobDBImpl::Close() to bypass thread op verification
- Build table
- TableBuilderOptions now includes Read/WriteOpitons so BuildTable() do not need to take these two variables
- Replace the io_priority passed into BuildTable() with TableBuilderOptions::WriteOpitons::rate_limiter_priority. Similar for BlobFileBuilder.
This parameter is used for dynamically changing file io priority for flush, see https://github.com/facebook/rocksdb/pull/9988?fbclid=IwAR1DtKel6c-bRJAdesGo0jsbztRtciByNlvokbxkV6h_L-AE9MACzqRTT5s for more
- Update ThreadStatus::FLUSH_BYTES_WRITTEN to use io_activity to track flush IO in flush job and db open instead of io_priority
## Test
### db bench
Flush
```
./db_bench --statistics=1 --benchmarks=fillseq --num=100000 --write_buffer_size=100
rocksdb.sst.write.micros P50 : 1.830863 P95 : 4.094720 P99 : 6.578947 P100 : 26.000000 COUNT : 7875 SUM : 20377
rocksdb.file.write.flush.micros P50 : 1.830863 P95 : 4.094720 P99 : 6.578947 P100 : 26.000000 COUNT : 7875 SUM : 20377
rocksdb.file.write.compaction.micros P50 : 0.000000 P95 : 0.000000 P99 : 0.000000 P100 : 0.000000 COUNT : 0 SUM : 0
rocksdb.file.write.db.open.micros P50 : 0.000000 P95 : 0.000000 P99 : 0.000000 P100 : 0.000000 COUNT : 0 SUM : 0
```
compaction, db oopen
```
Setup: ./db_bench --statistics=1 --benchmarks=fillseq --num=10000 --disable_auto_compactions=1 -write_buffer_size=100 --db=../db_bench
Run:./db_bench --statistics=1 --benchmarks=compact --db=../db_bench --use_existing_db=1
rocksdb.sst.write.micros P50 : 2.675325 P95 : 9.578788 P99 : 18.780000 P100 : 314.000000 COUNT : 638 SUM : 3279
rocksdb.file.write.flush.micros P50 : 0.000000 P95 : 0.000000 P99 : 0.000000 P100 : 0.000000 COUNT : 0 SUM : 0
rocksdb.file.write.compaction.micros P50 : 2.757353 P95 : 9.610687 P99 : 19.316667 P100 : 314.000000 COUNT : 615 SUM : 3213
rocksdb.file.write.db.open.micros P50 : 2.055556 P95 : 3.925000 P99 : 9.000000 P100 : 9.000000 COUNT : 23 SUM : 66
```
blob stats - just to make sure they aren't broken by this PR
```
Integrated Blob DB
Setup: ./db_bench --enable_blob_files=1 --statistics=1 --benchmarks=fillseq --num=10000 --disable_auto_compactions=1 -write_buffer_size=100 --db=../db_bench
Run:./db_bench --enable_blob_files=1 --statistics=1 --benchmarks=compact --db=../db_bench --use_existing_db=1
pre-PR:
rocksdb.blobdb.blob.file.write.micros P50 : 7.298246 P95 : 9.771930 P99 : 9.991813 P100 : 16.000000 COUNT : 235 SUM : 1600
rocksdb.blobdb.blob.file.synced COUNT : 1
rocksdb.blobdb.blob.file.bytes.written COUNT : 34842
post-PR:
rocksdb.blobdb.blob.file.write.micros P50 : 2.000000 P95 : 2.829360 P99 : 2.993779 P100 : 9.000000 COUNT : 707 SUM : 1614
- COUNT is higher and values are smaller as it includes header and footer write
- COUNT is 3X higher due to each Append() count as one post-PR, while in pre-PR, 3 Append()s counts as one. See https://github.com/facebook/rocksdb/pull/11910/files#diff-32b811c0a1c000768cfb2532052b44dc0b3bf82253f3eab078e15ff201a0dabfL157-L164
rocksdb.blobdb.blob.file.synced COUNT : 1 (stay the same)
rocksdb.blobdb.blob.file.bytes.written COUNT : 34842 (stay the same)
```
```
Stacked Blob DB
Run: ./db_bench --use_blob_db=1 --statistics=1 --benchmarks=fillseq --num=10000 --disable_auto_compactions=1 -write_buffer_size=100 --db=../db_bench
pre-PR:
rocksdb.blobdb.blob.file.write.micros P50 : 12.808042 P95 : 19.674497 P99 : 28.539683 P100 : 51.000000 COUNT : 10000 SUM : 140876
rocksdb.blobdb.blob.file.synced COUNT : 8
rocksdb.blobdb.blob.file.bytes.written COUNT : 1043445
post-PR:
rocksdb.blobdb.blob.file.write.micros P50 : 1.657370 P95 : 2.952175 P99 : 3.877519 P100 : 24.000000 COUNT : 30001 SUM : 67924
- COUNT is higher and values are smaller as it includes header and footer write
- COUNT is 3X higher due to each Append() count as one post-PR, while in pre-PR, 3 Append()s counts as one. See https://github.com/facebook/rocksdb/pull/11910/files#diff-32b811c0a1c000768cfb2532052b44dc0b3bf82253f3eab078e15ff201a0dabfL157-L164
rocksdb.blobdb.blob.file.synced COUNT : 8 (stay the same)
rocksdb.blobdb.blob.file.bytes.written COUNT : 1043445 (stay the same)
```
### Rehearsal CI stress test
Trigger 3 full runs of all our CI stress tests
### Performance
Flush
```
TEST_TMPDIR=/dev/shm ./db_basic_bench_pre_pr --benchmark_filter=ManualFlush/key_num:524288/per_key_size:256 --benchmark_repetitions=1000
-- default: 1 thread is used to run benchmark; enable_statistics = true
Pre-pr: avg 507515519.3 ns
497686074,499444327,500862543,501389862,502994471,503744435,504142123,504224056,505724198,506610393,506837742,506955122,507695561,507929036,508307733,508312691,508999120,509963561,510142147,510698091,510743096,510769317,510957074,511053311,511371367,511409911,511432960,511642385,511691964,511730908,
Post-pr: avg 511971266.5 ns, regressed 0.88%
502744835,506502498,507735420,507929724,508313335,509548582,509994942,510107257,510715603,511046955,511352639,511458478,512117521,512317380,512766303,512972652,513059586,513804934,513808980,514059409,514187369,514389494,514447762,514616464,514622882,514641763,514666265,514716377,514990179,515502408,
```
Compaction
```
TEST_TMPDIR=/dev/shm ./db_basic_bench_{pre|post}_pr --benchmark_filter=ManualCompaction/comp_style:0/max_data:134217728/per_key_size:256/enable_statistics:1 --benchmark_repetitions=1000
-- default: 1 thread is used to run benchmark
Pre-pr: avg 495346098.30 ns
492118301,493203526,494201411,494336607,495269217,495404950,496402598,497012157,497358370,498153846
Post-pr: avg 504528077.20, regressed 1.85%. "ManualCompaction" include flush so the isolated regression for compaction should be around 1.85-0.88 = 0.97%
502465338,502485945,502541789,502909283,503438601,504143885,506113087,506629423,507160414,507393007
```
Put with WAL (in case passing WriteOptions slows down this path even without collecting SST write stats)
```
TEST_TMPDIR=/dev/shm ./db_basic_bench_pre_pr --benchmark_filter=DBPut/comp_style:0/max_data:107374182400/per_key_size:256/enable_statistics:1/wal:1 --benchmark_repetitions=1000
-- default: 1 thread is used to run benchmark
Pre-pr: avg 3848.10 ns
3814,3838,3839,3848,3854,3854,3854,3860,3860,3860
Post-pr: avg 3874.20 ns, regressed 0.68%
3863,3867,3871,3874,3875,3877,3877,3877,3880,3881
```
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11910
Reviewed By: ajkr
Differential Revision: D49788060
Pulled By: hx235
fbshipit-source-id: 79e73699cda5be3b66461687e5147c2484fc5eff
Summary:
The CreateEnvTest.CreateEncryptedFileSystem unit test is to verify the creation functionality of EncryptedFileSystem, but now it just support the builtin CTREncryptionProvider class.
This patch make it flexible to use environment variable `TEST_FS_URI`, it is useful to test customer encryption plugins.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/12025
Reviewed By: anand1976
Differential Revision: D50799656
Pulled By: ajkr
fbshipit-source-id: dbcacfefbf07de9c7803f7707b34c5193bec17bf
Summary:
## Context checksum
All RocksDB checksums currently use 32 bits of checking
power, which should be 1 in 4 billion false negative (FN) probability (failing to
detect corruption). This is true for random corruptions, and in some cases
small corruptions are guaranteed to be detected. But some possible
corruptions, such as in storage metadata rather than storage payload data,
would have a much higher FN rate. For example:
* Data larger than one SST block is replaced by data from elsewhere in
the same or another SST file. Especially with block_align=true, the
probability of exact block size match is probably around 1 in 100, making
the FN probability around that same. Without `block_align=true` the
probability of same block start location is probably around 1 in 10,000,
for FN probability around 1 in a million.
To solve this problem in new format_version=6, we add "context awareness"
to block checksum checks. The stored and expected checksum value is
modified based on the block's position in the file and which file it is in. The
modifications are cleverly chosen so that, for example
* blocks within about 4GB of each other are guaranteed to use different context
* blocks that are offset by exactly some multiple of 4GiB are guaranteed to use
different context
* files generated by the same process are guaranteed to use different context
for the same offsets, until wrap-around after 2^32 - 1 files
Thus, with format_version=6, if a valid SST block and checksum is misplaced,
its checksum FN probability should be essentially ideal, 1 in 4B.
## Footer checksum
This change also adds checksum protection to the SST footer (with
format_version=6), for the first time without relying on whole file checksum.
To prevent a corruption of the format_version in the footer (e.g. 6 -> 5) to
defeat the footer checksum, we change much of the footer data format
including an "extended magic number" in format_version 6 that would be
interpreted as empty index and metaindex block handles in older footer
versions. We also change the encoding of handles to free up space for
other new data in footer.
## More detail: making space in footer
In order to keep footer the same size in format_version=6 (avoid change to IO
patterns), we have to free up some space for new data. We do this two ways:
* Metaindex block handle is encoded down to 4 bytes (from 10) by assuming
it immediately precedes the footer, and by assuming it is < 4GB.
* Index block handle is moved into metaindex. (I don't know why it was
in footer to begin with.)
## Performance
In case of small performance penalty, I've made a "pay as you go" optimization
to compensate: replace `MutableCFOptions` in BlockBasedTableBuilder::Rep
with the only field used in that structure after construction: `prefix_extractor`.
This makes the PR an overall performance improvement (results below).
Nevertheless I'm seeing essentially no difference going from fv=5 to fv=6,
even including that improvement for both. That's based on extreme case table
write performance testing, many files with many blocks. This is relatively
checksum intensive (small blocks) and salt generation intensive (small files).
```
(for I in `seq 1 100`; do TEST_TMPDIR=/dev/shm/dbbench2 ./db_bench -benchmarks=fillseq -memtablerep=vector -disable_wal=1 -allow_concurrent_memtable_write=false -num=3000000 -compaction_style=2 -fifo_compaction_max_table_files_size_mb=10000 -fifo_compaction_allow_compaction=0 -write_buffer_size=100000 -compression_type=none -block_size=1000; done) 2>&1 | grep micros/op | tee out
awk '{ tot += $5; n += 1; } END { print int(1.0 * tot / n) }' < out
```
Each value below is ops/s averaged over 100 runs, run simultaneously with competing
configuration for load fairness
Before -> after (both fv=5): 483530 -> 483673 (negligible)
Re-run 1: 480733 -> 485427 (1.0% faster)
Re-run 2: 483821 -> 484541 (0.1% faster)
Before (fv=5) -> after (fv=6): 482006 -> 485100 (0.6% faster)
Re-run 1: 482212 -> 485075 (0.6% faster)
Re-run 2: 483590 -> 484073 (0.1% faster)
After fv=5 -> after fv=6: 483878 -> 485542 (0.3% faster)
Re-run 1: 485331 -> 483385 (0.4% slower)
Re-run 2: 485283 -> 483435 (0.4% slower)
Re-run 3: 483647 -> 486109 (0.5% faster)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9058
Test Plan:
unit tests included (table_test, db_properties_test, salt in env_test). General DB tests
and crash test updated to test new format_version.
Also temporarily updated the default format version to 6 and saw some test failures. Almost all
were due to an inadvertent additional read in VerifyChecksum to verify the index block checksum,
though it's arguably a bug that VerifyChecksum does not appear to (re-)verify the index block
checksum, just assuming it was verified in opening the index reader (probably *usually* true but
probably not always true). Some other concerns about VerifyChecksum are left in FIXME
comments. The only remaining test failure on change of default (in block_fetcher_test) now
has a comment about how to upgrade the test.
The format compatibility test does not need updating because we have not updated the default
format_version.
Reviewed By: ajkr, mrambacher
Differential Revision: D33100915
Pulled By: pdillinger
fbshipit-source-id: 8679e3e572fa580181a737fd6d113ed53c5422ee
Summary:
1. Public API change: Replace `use_async_io` API in file_system with `SupportedOps` API which is used by underlying FileSystem to indicate to upper layers whether the FileSystem supports different operations introduced in `enum FSSupportedOps `. Right now operations are `async_io` and whether FS will provide its own buffer during reads or not. The api is changed to extend it to various FileSystem operations in one API rather than creating a separate API for each operation.
2. Provide support for underlying FS to pass their own buffer during Reads (async and sync read) instead of using RocksDB provided `scratch` (buffer) in `FSReadRequest`. Currently only MultiRead supports it and later will be extended to other reads as well (point lookup, scan etc). More details in how to enable in file_system.h
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11324
Test Plan: Tested locally
Reviewed By: anand1976
Differential Revision: D44465322
Pulled By: akankshamahajan15
fbshipit-source-id: 9ec9e08f839b5cc815e75d5dade6cd549998d0ec
Summary:
... so that a non-cryptographic whole file checksum would be highly resistant
to manipulation by a user able to manipulate key-value data (e.g. a user whose data is
stored in RocksDB) and able to predict SST metadata such as DB session id and file
number based on read access to logs or DB files. The adversary would also need to predict
the salt in order to influence the checksum result toward collision with another file's
checksum.
This change is just internal code to support such a future feature. I think this should be a
passive feature, not option-controlled, because you probably won't think about needing it
until you discover you do need it, and it should be low cost, in space (16 bytes per SST
file) and CPU.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11331
Test Plan: Unit tests added to verify at least pseudorandom behavior. (Actually caught a bug in first draft!) The new "stress" style tests run in ~3ms each on my system.
Reviewed By: ajkr
Differential Revision: D46129415
Pulled By: pdillinger
fbshipit-source-id: 7972dc74487e062b29b1fd9c227425e922c98796
Summary:
Use another static object to join threads instead.
This change is motivated by a case in which some code using NewLRUCache() -> ShardedCacheBase -> SemiStructuredUniqueIdGen -> GenerateRawUniqueId() -> Env::Default() was happening
during static destruction.
I didn't see anything else in PosixEnv or base classes that would cause a problem by not
destroying. (WinEnv is already not destroyed; see env_default.cc)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11538UndefinedBehaviorSanitizer: undefined-behavior env/env_test.cc:3561:23 in
$
```
Test Plan:
test added, which would previously fail with UBSAN:
```
$ ./env_test --gtest_filter=*Destruct*
Note: Google Test filter = *Destruct*
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from EnvTestMisc
[ RUN ] EnvTestMisc.StaticDestruction
[ OK ] EnvTestMisc.StaticDestruction (0 ms)
[----------] 1 test from EnvTestMisc (0 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (0 ms total)
[ PASSED ] 1 test.
env/env_test.cc:3561:23: runtime error: member call on address 0x7f7b96671ca8 which does not point to an object of type 'rocksdb::Env'
0x7f7b96671ca8: note: object is of type 'N7rocksdb12ConfigurableE'
00 00 00 00 90 a7 f7 95 7b 7f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^~~~~~~~~~~~~~~~~~~~~~~
vptr for 'N7rocksdb12ConfigurableE'
Reviewed By: jowlyzhang
Differential Revision: D46737389
Pulled By: pdillinger
fbshipit-source-id: 0f80a443bf799ffc5641e898cf3a75f7d10a987b
Summary:
See motivation and description in new ShardedCacheOptions::hash_seed option.
Updated db_bench so that its seed param is used for the cache hash seed.
Made its code more safe to ensure seed is set before use.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11391
Test Plan:
unit tests added / updated
**Performance** - no discernible difference seen running cache_bench repeatedly before & after. With lru_cache and hyper_clock_cache.
Reviewed By: hx235
Differential Revision: D45557797
Pulled By: pdillinger
fbshipit-source-id: 40bf4da6d66f9d41a8a0eb8e5cf4246a4aa07934
Summary:
We haven't been actively mantaining RocksDB LITE recently and the size must have been gone up significantly. We are removing the support.
Most of changes were done through following comments:
unifdef -m -UROCKSDB_LITE `git grep -l ROCKSDB_LITE | egrep '[.](cc|h)'`
by Peter Dillinger. Others changes were manually applied to build scripts, CircleCI manifests, ROCKSDB_LITE is used in an expression and file db_stress_test_base.cc.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11147
Test Plan: See CI
Reviewed By: pdillinger
Differential Revision: D42796341
fbshipit-source-id: 4920e15fc2060c2cd2221330a6d0e5e65d4b7fe2
Summary:
Moved linux builds to using docker to avoid CI instability caused by dependency installation site down.
Added the `Dockerfile` which is used to build the image.
The build time is also significantly reduced, because no dependencies installation and with using 2xlarge+ instance for slow build (like tsan test).
Also fixed a few issues detected while building this:
* `DestoryDB()` Status not checked for a few tests
* nullptr might be used in `inlineskiplist.cc`
Pull Request resolved: https://github.com/facebook/rocksdb/pull/10496
Test Plan: CI
Reviewed By: ajkr
Differential Revision: D38554200
Pulled By: jay-zhuang
fbshipit-source-id: 16e8fb2bf07b9c84bb27fb18421c4d54f2f248fd
Summary:
Travis CI is depreciated and haven't been maintained for some time.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/10407
Reviewed By: ajkr
Differential Revision: D38078382
Pulled By: jay-zhuang
fbshipit-source-id: f42057f2f41f722bdce56bf195f67a94835191fb
Summary:
Fix bug in O_DIRECT and io_uring when its EOF and bytes_read =
0 because of wrong check, it got added into incomplete list and gets stuck in an infinite loop as it will always return bytes_read = 0. The bug was introduced by PR https://github.com/facebook/rocksdb/pull/10197 and that PR is not released yet in any release branch.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/10368
Test Plan: Added new unit test
Reviewed By: siying
Differential Revision: D37885184
Pulled By: akankshamahajan15
fbshipit-source-id: 35b36a44b696d29b2f6f25301aa1b19547b4e03b
Summary:
Add `ReserveThreads` and `ReleaseThreads` functions in thread pool to support reservation in for a specific thread pool. With this feature, a thread will be blocked if the number of waiting threads (noted by `num_waiting_threads_`) equals the number of reserved threads (noted by `reserved_threads_`), normally `reserved_threads_` is upper bounded by `num_waiting_threads_`; in rare cases (e.g. `SetBackgroundThreadsInternal` is called when some threads are already reserved), `num_waiting_threads_` can be less than `reserved_threads`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/10278
Test Plan: Add `ReserveThreads` unit test in `env_test`. Update the unit test `SimpleColumnFamilyInfoTest` in `thread_list_test` with adding `ReserveThreads` related assertions.
Reviewed By: hx235
Differential Revision: D37640946
Pulled By: littlepig2013
fbshipit-source-id: 4d691f6b9a433569f96ab52d52c3defe5b065367
Summary:
There are some time-related POSIX APIs that are not available on Windows
(e.g. `localtime_r`), which we have worked around by providing our own
implementations in `port/sys_time.h`. This workaround actually relies on
some ambiguity: on Windows, a call to `localtime_r` calls
`ROCKSDB_NAMESPACE::port::localtime_r` (which is pulled into
`ROCKSDB_NAMESPACE` by a using-declaration), while on other platforms
it calls the global `localtime_r`. This works fine as long as there is only one
candidate function; however, it breaks down when there is more than one
`localtime_r` visible in a scope.
The patch fixes this by introducing `ROCKSDB_NAMESPACE::port::{TimeVal, GetTimeOfDay, LocalTimeR}`
to eliminate any ambiguity.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/10045
Test Plan: `make check`
Reviewed By: riversand963
Differential Revision: D36639372
Pulled By: ltamasi
fbshipit-source-id: fc13dbfa421b7c8918111a6d9e24ce77e91a7c50
Summary:
Add methods to set the various functions (Parse, Serialize, Equals) to the OptionTypeInfo. These methods simplify the number of constructors required for OptionTypeInfo and make the code a little clearer.
Add functions to the OptionTypeInfo for Prepare and Validate. These methods allow types other than Configurable and Customizable to have Prepare and Validate logic. These methods could be used by an option to guarantee that its settings were in a range or that a value was initialized.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9411
Reviewed By: pdillinger
Differential Revision: D36174849
Pulled By: mrambacher
fbshipit-source-id: 72517d8c6bab4723788a4c1a9e16590bff870125
Summary:
ToString() is created as some platform doesn't support std::to_string(). However, we've already used std::to_string() by mistake for 16 months (in db/db_info_dumper.cc). This commit just remove ToString().
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9955
Test Plan: Watch CI tests
Reviewed By: riversand963
Differential Revision: D36176799
fbshipit-source-id: bdb6dcd0e3a3ab96a1ac810f5d0188f684064471
Summary:
If FilePrefetchBuffer object is destroyed and then later Poll() calls callback on object which has been destroyed, it gives segfault on accessing destroyed object. It was caught after adding unit tests that tests Posix implementation of ReadAsync and Poll APIs.
This PR also updates and fixes existing IOURing tests which were not running locally because RocksDbIOUringEnable function wasn't defined and IOUring was disabled for those tests
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9777
Test Plan: Added new unit test
Reviewed By: anand1976
Differential Revision: D35254002
Pulled By: akankshamahajan15
fbshipit-source-id: 68e80054ffb14ae25c255920ebc6548ca5f130a1
Summary:
Provide support for Async Read and Poll in Posix file system using IOUring.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9578
Test Plan: In progress
Reviewed By: anand1976
Differential Revision: D34690256
Pulled By: akankshamahajan15
fbshipit-source-id: 291cbd1380a3cb904b726c34c0560d1b2ce44a2e
Summary:
Update the signature of Poll and ReadAsync APIs in filesystem.
Instead of unique_ptr, void** will be passed as io_handle and the delete function.
io_handle and delete function should be provided by underlying
FileSystem and its lifetime will be maintained by RocksDB. io_handle
will be deleted by RocksDB once callback is made to update the results or Poll is
called to get the results.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9623
Test Plan: Add a new unit test.
Reviewed By: anand1976
Differential Revision: D34403529
Pulled By: akankshamahajan15
fbshipit-source-id: ea185a5f4c7bec334631e4f781ea7ba4135645f0
Summary:
Thread-pool pops a thread function and then run the function,
which may cause thread-pool is empty but the last function is still
running.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9502
Test Plan:
`gtest-parallel ./env_test
--gtest_filter=DefaultEnvWithoutDirectIO/EnvPosixTestWithParam.RunMany/0
-r 10000 -w 1000`
Reviewed By: ajkr
Differential Revision: D34011184
Pulled By: jay-zhuang
fbshipit-source-id: 8c38bef155205bef96fd1c988dcc643a6b2ac270
Summary:
Added a CountedFileSystem that tracks a number of file operations (opens, closes, deletes, renames, flushes, syncs, fsyncs, reads, writes). This class was based on the ReportFileOpEnv from db_bench.
This is a stepping stone PR to be able to change the SpecialEnv into a SpecialFileSystem, where several of the file varieties wish to do operation counting.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9283
Reviewed By: pdillinger
Differential Revision: D33062004
Pulled By: mrambacher
fbshipit-source-id: d0d297a7fb9c48c06cbf685e5fa755c27193b6f5
Summary:
We see:
[ RUN ] ChrootEnvWithDirectIO/EnvPosixTestWithParam.RunMany/0
env/env_test.cc:464: Failure
Expected equality of these values:
4
cur
Which is: 0
The suspicious is that the wait time is not long enough. Increase the wait time to 10s and allows earlier check.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9413
Test Plan: Run the test
Reviewed By: riversand963
Differential Revision: D33697715
fbshipit-source-id: 3d71715562a8cceb694b773276dd9e4e451a18bc
Summary:
In order to support old-style regex function registration, restored the original "Register<T>(string, Factory)" method using regular expressions. The PatternEntry methods were left in place but renamed to AddFactory. The goal is to allow for the deprecation of the original regex Registry method in an upcoming release.
Added modes to the PatternEntry kMatchZeroOrMore and kMatchAtLeastOne to match * or +, respectively (kMatchAtLeastOne was the original behavior).
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9362
Reviewed By: pdillinger
Differential Revision: D33432562
Pulled By: mrambacher
fbshipit-source-id: ed88ab3f9a2ad0d525c7bd1692873f9bb3209d02
Summary:
Allows the Env to have options (Configurable) and loads like other Customizable classes.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9293
Reviewed By: pdillinger, zhichao-cao
Differential Revision: D33181591
Pulled By: mrambacher
fbshipit-source-id: 55e823886c654d214eda9eedd45ccdc54dac14d7
Summary:
* New public header unique_id.h and function GetUniqueIdFromTableProperties
which computes a universally unique identifier based on table properties
of table files from recent RocksDB versions.
* Generation of DB session IDs is refactored so that they are
guaranteed unique in the lifetime of a process running RocksDB.
(SemiStructuredUniqueIdGen, new test included.) Along with file numbers,
this enables SST unique IDs to be guaranteed unique among SSTs generated
in a single process, and "better than random" between processes.
See https://github.com/pdillinger/unique_id
* In addition to public API producing 'external' unique IDs, there is a function
for producing 'internal' unique IDs, with functions for converting between the
two. In short, the external ID is "safe" for things people might do with it, and
the internal ID enables more "power user" features for the future. Specifically,
the external ID goes through a hashing layer so that any subset of bits in the
external ID can be used as a hash of the full ID, while also preserving
uniqueness guarantees in the first 128 bits (bijective both on first 128 bits
and on full 192 bits).
Intended follow-up:
* Use the internal unique IDs in cache keys. (Avoid conflicts with https://github.com/facebook/rocksdb/issues/8912) (The file offset can be XORed into
the third 64-bit value of the unique ID.)
* Publish the external unique IDs in FileStorageInfo (https://github.com/facebook/rocksdb/issues/8968)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8990
Test Plan:
Unit tests added, and checking of unique ids in stress test.
NOTE in stress test we do not generate nearly enough files to thoroughly
stress uniqueness, but the test trims off pieces of the ID to check for
uniqueness so that we can infer (with some assumptions) stronger
properties in the aggregate.
Reviewed By: zhichao-cao, mrambacher
Differential Revision: D31582865
Pulled By: pdillinger
fbshipit-source-id: 1f620c4c86af9abe2a8d177b9ccf2ad2b9f48243
Summary:
This header file was including everything and the kitchen sink when it did not need to. This resulted in many places including this header when they needed other pieces instead.
Cleaned up this header to only include what was needed and fixed up the remaining code to include what was now missing.
Hopefully, this sort of code hygiene cleanup will speed up the builds...
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8930
Reviewed By: pdillinger
Differential Revision: D31142788
Pulled By: mrambacher
fbshipit-source-id: 6b45de3f300750c79f751f6227dece9cfd44085d
Summary:
Made SystemClock into a Customizable class, complete with CreateFromString.
Cleaned up some of the existing SystemClock implementations that were redundant (NoSleep was the same as the internal one for MockEnv).
Changed MockEnv construction to allow Clock to be passed to the Memory/MockFileSystem.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8636
Reviewed By: zhichao-cao
Differential Revision: D30483360
Pulled By: mrambacher
fbshipit-source-id: cd0e3a876c39f8c98fe13374c06e8edbd5b9f2a1
Xingbo Wang