mirror of
https://github.com/facebook/rocksdb.git
synced 2026-07-07 14:47:40 +08:00
de7b8e8f7d34dccce4995a31f3851ccc0ca0f5d4
261 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Xingbo Wang
|
de7b8e8f7d |
Add RocksDB flush reason and atomic flush request metrics (#14898)
Summary:
This PR adds flush observability for debugging write stalls and flush bottlenecks.
Changes include:
- Add flush reason tickers for physical flush jobs:
- rocksdb.flush.reason.write_buffer_full
- rocksdb.flush.reason.write_buffer_manager
- rocksdb.flush.reason.memtable_max_range_deletions
- Add flush memtable size histograms:
- rocksdb.flush.memtable.memory.bytes
- rocksdb.flush.memtable.total.data.size.bytes
- rocksdb.flush.write_buffer_full.memtable.memory.bytes
- rocksdb.flush.write_buffer_manager.memtable.memory.bytes
- Add FlushReason::kMemtableMaxRangeDeletions and propagate it from memtables when memtable_max_range_deletions triggers a flush.
- Preserve atomic flush as a single request-level reason. For atomic flush, the reason is derived only from CFs that scheduled the flush, not every CF selected into the atomic flush cut.
- Add atomic flush request-level counters:
- rocksdb.atomic_flush.request.reason.write_buffer_full
- rocksdb.atomic_flush.request.reason.write_buffer_manager
- rocksdb.atomic_flush.request.reason.memtable_max_range_deletions
- rocksdb.atomic_flush.request.reason.other
- Keep existing per-CF flush job reason counters unchanged. Under atomic flush, rocksdb.flush.reason.* still counts physical CF flush jobs, while rocksdb.atomic_flush.request.reason.* counts one logical atomic flush request.
- Wire the new flush reason, ticker, and histogram types through Java and JNI bindings.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14898
Test Plan: CI
Reviewed By: anand1976
Differential Revision: D110214337
Pulled By: xingbowang
fbshipit-source-id: 4a445c4175d2e09ff5edbe66fde81fa00bb9e8da
|
||
|
Josh Kang
|
8ba4204b28 |
Do not allow read only DBs to delete obsolete files (#14881)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14881 Read-only DB instances can share a directory with a live writer, so their view of live files is only a snapshot from the time they opened. After read-only DB open began setting `opened_successfully_` for its intended open-success semantics, that state had an unintended side effect: the common close path treated a successful read-only open like a read-write open and ran obsolete-file cleanup. If the writer created or made files live after the read-only handle opened, that cleanup could use the stale read-only live set and delete files still needed by the writer. This change records whether a `DBImpl` was opened read-only and skips close-time obsolete-file cleanup for read-only DBs instead of simply keeping `opened_successfully_=false`, which could be confusing and easily mistaken in the future again. For completeness I also updated other callsites of `opened_successfully_`, but those should not be real bugs as ReadOnly DBs do not run flushes/compactions or write to WALs. Reviewed By: xingbowang Differential Revision: D109622445 fbshipit-source-id: d7be4b20fce86ccb218a63ac6f5b707b316aac79 |
||
|
Maciej Szeszko
|
02941016d0 |
Fix seqno sync race with WriteUnprepared (#14864)
Summary: # Summary Pull Request resolved: https://github.com/facebook/rocksdb/pull/14864 With `two_write_queues`=`true`, `WRITE_UNPREPARED` can allocate sequence numbers through `FetchAddLastAllocatedSequence()` before those numbers are published through `SetLastSequence()`. Error recovery can then create new memtable/WAL boundaries from a stale `LastSequence()`, which can later surface as "sequence number going backwards" corruption. Fix this by syncing `LastSequence()` to `LastAllocatedSequence()` at the recovery entry point after draining both write queues, and again at the recovery flush fence after `FlushAllColumnFamilies()` has released/reacquired the DB mutex and before `SwitchMemtable()` consumes `LastSequence()`. Apply the same fence to atomic recovery flushes. # Perf comparative results TLDR; no visible regression. I ran a deterministic local recovery benchmark to check whether the new two-write-queue recovery fence adds measurable recovery latency. ## Workload (P2385173320) - Optimized build. - `TransactionDB` with `WRITE_UNPREPARED`. - `two_write_queues=true`. - Each iteration dirties the memtable with WUP transactions, forces a retryable flush IO error through `FaultInjectionTestFS`, re-enables the filesystem, then measures `DB::Resume()`. - 50 warmup recoveries + 500 measured recoveries per run. ## Why this is relevant This targets the changed recovery path directly rather than measuring generic write throughput. The change only runs during recovery, so the useful signal is recovery latency around `ResumeImpl()` and the recovery flush fence. ## Code path exercised - `DB::Resume()` - `ErrorHandler::RecoverFromBGError(true)` - `DBImpl::ResumeImpl()` - new `two_write_queues_` recovery queue fence - non-atomic recovery `FlushMemTable()` fence before `SwitchMemtable()` This does not cover atomic flush recovery or a writer-backlog stress case. Comparative results: | Revision | Attempt | resume_p50_us | resume_p95_us | Retryable BG errors | | Parent | | 1555.75 | 2092.22 | 550 | | Fix | 1 | 1537.68 | 1905.65 | 550 | | Fix | 2 | 1475.10 | 1883.26 | 550 | | Fix | 3 | 1617.52 | 1993.97 | 550 | Reviewed By: xingbowang, anand1976 Differential Revision: D108946310 fbshipit-source-id: 75fcb4bf0c9a932b5fbed7e934c7f14d981a1bf2 |
||
|
Xingbo Wang
|
30dba7f41a |
Fix compaction abort rescheduling before queue pick (#14800)
Summary: - AbortAllCompactions can race with an already-scheduled automatic compaction before the background worker calls PickCompactionFromQueue(). MaybeScheduleFlushOrCompaction() has already consumed one unscheduled_compactions_ credit when it scheduled the worker, but the CF is still present in compaction_queue_ with queued_for_compaction=true. If the worker returns kCompactionAborted before popping the CF, ResumeAllCompactions() can see unscheduled_compactions_ == 0 and fail to schedule the still-queued work, leaving compaction permanently stalled until DB restart. - Restore the unscheduled compaction credit in the non-prepicked abort path, matching the existing BG-work-stopped handling for the same scheduled-before-pick state. Prepicked/manual compactions are not adjusted because they do not represent an unpopped automatic compaction_queue_ entry whose scheduling credit was consumed. - Add AbortScheduledAutomaticCompactionBeforePick to deterministically reproduce the lost-credit race with a sync point at BackgroundCallCompaction:0. The test verifies that ResumeAllCompactions() schedules the still-queued automatic compaction by checking COMPACT_WRITE_BYTES advances and L0 file count drops. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14800 Test Plan: - Without the implementation fix, `DBCompactionAbortTest.AbortScheduledAutomaticCompactionBeforePick` fails because `COMPACT_WRITE_BYTES` remains unchanged after `ResumeAllCompactions()`. Reviewed By: pdillinger Differential Revision: D106684155 Pulled By: xingbowang fbshipit-source-id: 6dacea473ef481eb3122eb15e296e5b69635413f |
||
|
Peter Dillinger
|
01d8c7720f |
Fix crash test failures in OnCompactionPreCommit listener checks (#14753)
Summary: NotifyOnCompactionPreCommit initially omitted the shutting_down_ guard to avoid a false positive abort in the db_stress OnTableFileDeleted check, where stale compacting_files_ entries from skipped notifications would be mistaken for a bug. The better fix is to add the shutting_down_ guard for consistency with Begin and Completed, and instead make OnTableFileDeleted tolerate stale tracking during shutdown by checking a new atomic bool in the listener intended to track DBImpl's shutting_down_. Also fix lint: release mutex before RandomSleep() in PreCommit; use char literal for find_last_of; avoid unnecessary string copy. Document WART in listener.h: all three compaction callbacks are skipped during DB shutdown, so a committed compaction may go unobserved. Bonus: update CLAUDE.md with instructions on avoiding non-ASCII characters Pull Request resolved: https://github.com/facebook/rocksdb/pull/14753 Test Plan: manually trigger many crash test runs Reviewed By: hx235 Differential Revision: D105591913 Pulled By: pdillinger fbshipit-source-id: 32029fea4c2571d88f645eb325db2e25a94e0d26 |
||
|
Peter Dillinger
|
805a476a5f |
Add OnCompactionPreCommit listener callback (#14740)
Summary: Adds a new `EventListener::OnCompactionPreCommit` callback that fires after a compaction job's output files are written but *before* the manifest write commits the new Version. At that point input files still have `FileMetaData::being_compacted == true`, so listeners that maintain bookkeeping of "files currently being compacted" can clean up that state without racing the compaction picker. A check implemented by a Meta-internal RocksDB user crashes when the same file appears as input to two concurrent compactions. Tracking that set in `OnCompactionBegin` / `OnCompactionCompleted` produces false positives because `Compaction::ReleaseCompactionFiles()` flips `being_compacted` back to false before `OnCompactionCompleted` fires, so another thread can pick the same file and trigger `OnCompactionBegin` before the previous compaction's `Completed` callback runs. Doing the cleanup in `OnCompactionPreCommit` closes that race. Default implementation is a no-op, so no API break. A trivial refactoring to split PerformTrivialMove ensures data is populated for the new callback, while calling back before the trivial move compaction is committed. Bonus: `CLAUDE.md` update for when to call `make clean` as I've recently had it get thoroughly confused TWICE mixing build modes. Task: T269479969 Pull Request resolved: https://github.com/facebook/rocksdb/pull/14740 Test Plan: - New unit test in `db/listener_test.cc` checks that `OnCompactionPreCommit` fires strictly between `OnCompactionBegin` and `OnCompactionCompleted` for the same compaction, and that input files still have `being_compacted == true` at the time it fires. - Crash test: adds a concurrent-compaction sanity check to `db_stress`'s listener resembling the Meta-internal intended usage: tracks input file numbers from `OnCompactionBegin` to `OnCompactionPreCommit`, aborting if the same file appears as input to two concurrent compactions. Also checks other ordering constraints and checks for "leaks" from possible failure to call OnCompactionPreCommit(). Exercises all compaction styles and trivial-move/FIFO paths under load. Reviewed By: hx235 Differential Revision: D105065326 Pulled By: pdillinger fbshipit-source-id: 8f606a7c0fac899574e7340c600b71fed902394a |
||
|
generatedunixname3846135475516776
|
1dc4813d97 |
Rocksdb Crash Test failed: assertion failed - cached_file_is_live_or_quar (#14717)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14717 Reviewed By: pdillinger Differential Revision: D103953026 fbshipit-source-id: 4b81853dad792d77fe6dbc1ab7255a9f04834078 |
||
|
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 |
||
|
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 |
||
|
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 |
||
|
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 |
||
|
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 |
||
|
Peter Dillinger
|
d3817f058d |
Remove deprecated DB::Open raw pointer variants (and more) (#14335)
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 |
||
|
Xingbo Wang
|
656b734a5f |
Support abort background compaction jobs. (#14227)
Summary: This adds a new public API to allow applications to abort all running compactions and prevent new ones from starting. Unlike DisableManualCompaction() which only pauses manual compactions and waits for them to finish naturally, AbortAllCompactions() actively signals running compactions (both automatic and manual) to terminate early and waits for them to complete before returning. The abort signal is checked periodically during compaction (every 100 keys), so ongoing compactions abort quickly. Any output files from aborted compactions are automatically cleaned up to prevent partial results from being installed. This is useful for scenarios where applications need to quickly stop all compaction activity, such as during graceful shutdown or when performing maintenance operations. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14227 Test Plan: - Unit tests in db_compaction_abort_test.cc cover various abort scenarios including: abort before/during compaction, abort with multiple subcompactions, nested abort/resume calls, abort with CompactFiles API, abort across multiple column families, and timing guarantees - Updated compaction_job_test.cc to include the new parameter Reviewed By: anand1976 Differential Revision: D91480994 Pulled By: xingbowang fbshipit-source-id: 36837971d8a540cd34d3ec28a78bc94b582625b0 |
||
|
Xingbo Wang
|
ea5e649225 |
Fix an infinite compaction loop bug with udt (#14228)
Summary: Problem The TEST_WaitForCompact in TimestampCompatibleCompactionTest.UdtTombstoneCollapsingTest would sometimes run forever, indicating an infinite compaction loop. Issue https://github.com/facebook/rocksdb/issues/14223 Root Cause In ComputeBottommostFilesMarkedForCompaction(), files were marked for bottommost compaction based only on the condition largest_seqno < oldest_snapshot_seqnum. However, for User-Defined Timestamps (UDT) columns, compaction can only zero sequence numbers when the file's maximum timestamp is below full_history_ts_low. When timestamps were above this threshold: 1. File gets marked for compaction (seqno condition met) 2. Compaction runs but cannot zero seqno (timestamp condition not met) 3. Output file immediately gets re-marked for compaction 4. Infinite loop Solution Added timestamp range tracking to FileMetaData and updated the marking logic to check timestamps before marking files. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14228 Test Plan: Unit test Reviewed By: pdillinger Differential Revision: D90586045 Pulled By: xingbowang fbshipit-source-id: addfa4f988db8c87fb513a1bf58ee54623a6c210 |
||
|
Josh Kang
|
4bf2bcdbb3 |
Allow setting options for multiple column families (#14201)
Summary: Currently to set options for multiple CFs, the caller must repeatedly call SetOptions() for each CF. This in turn serializes the entire options file each time. This PR exposes a new API that allows SetOptions to be called on multiple CFs at once, thus only paying the OPTIONS file serialization once. Also added a new unit test for SetOptions. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14201 Reviewed By: pdillinger Differential Revision: D89735181 Pulled By: joshkang97 fbshipit-source-id: 9b7a721b7e8769b653243b1581678ffd05d038e8 |
||
|
Xingbo Wang
|
b33c547b06 |
Add trivial move support in CompactFiles API (#14112)
Summary: Support trivial move in CompactFiles API, which is not supported previously. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14112 Test Plan: Unit test Reviewed By: cbi42 Differential Revision: D86546150 Pulled By: xingbowang fbshipit-source-id: 08a3ae9a055f3d3d41711403b1695f44977e6ea8 |
||
|
Josh Kang
|
7ae602e80a |
Support output temperature in CompactFiles (#13955)
Summary: It is useful to be able to specify output temperatures in the CompactFiles API. For example it may be useful to store small L0 files produced by flushes locally, while larger intra-L0 compactions can store the compacted L0 file remotely. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13955 Test Plan: New unit tests Reviewed By: jaykorean Differential Revision: D82492503 Pulled By: joshkang97 fbshipit-source-id: e1225fe572a15d7c5c30a265762b048a4a9e7f0b |
||
|
Changyu Bi
|
3bd7d968e1 |
Introduce column family option cf_allow_ingest_behind (#13810)
Summary: this option has the same functionality as DBOptions::allow_ingest_behind but allows the feature at per CF level. `DBOptions::allow_ingest_behind` is deprecated after this PR and users should use `cf_allow_ingest_behind` instead. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13810 Test Plan: updated some existing tests to use the new option. Reviewed By: xingbowang Differential Revision: D79191969 Pulled By: cbi42 fbshipit-source-id: 0da45f6be472ace6754ad15df93d45ac86313837 |
||
|
Jay Huh
|
b6e804b7de |
Rename CompactFiles() and CompactRange() in CompactionPickers (#13831)
Summary: #Summary Quick follow-up from https://github.com/facebook/rocksdb/pull/13816: `CompactFiles()` and `CompactRange()` in CompactionPickers do not run compaction as their names might suggest. What they actually do is create the Compaction object that will be passed to `CompactionJob` to run the compaction. Renaming these two functions to better represent their purposes. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13831 Test Plan: No functional change. Existing CI should be sufficient. Reviewed By: hx235 Differential Revision: D79660196 Pulled By: jaykorean fbshipit-source-id: ca831dbef5120e7115b52fd07b0059ca16c8f1e8 |
||
|
Hui Xiao
|
6e4113e92d |
Remove reductant Compaction parameters (#13777)
Summary: **Context/Summary:** a small refactoring to make Compaction constructor simpler (though still complicated now). Pull Request resolved: https://github.com/facebook/rocksdb/pull/13777 Test Plan: Existing tests Reviewed By: jaykorean Differential Revision: D78385166 Pulled By: hx235 fbshipit-source-id: cd93d1ba3936d9f9077ffceb0dc4ef5506e51017 |
||
|
virajthakur
|
4bdfb7e7da |
support canceling ongoing CompactFiles (#13687)
Summary: Add an atomic bool to CompactionOptions to cancel an ongoing CompactFiles() operation, in the same fashion we do for CompactRange(). Pull Request resolved: https://github.com/facebook/rocksdb/pull/13687 Test Plan: ./db_test2 --gtest_filter=DBTest2.TestCancelCompactFiles Reviewed By: jaykorean Differential Revision: D76538529 Pulled By: virajthakur fbshipit-source-id: 77db5b4fb4cbd5280584834df28e51a72b084dab |
||
|
Hui Xiao
|
02bce9b1af |
Reduce universal compaction input lock time by forwarding intended compaction and re-picking (#13633)
Summary: **Context:** RocksDB currently selects files for long-running compaction outputs to the bottommost level, preventing these selected files files from being selected, but does not execute the compaction immediately like other compactions. Instead, this compaction is forwarded to another Env::Priority::bottom thread pool, where it waits (potentially for a long time) until its thread is ready to execute. This extended L0 lock time in universal compaction caused our users write stall and read performance regression. **Summary:** This PR is to eliminate L0 lock time during bottom priority compaction waiting to execute by the following - Create and forward an intended compaction only consists of last input file (or sorted run if non-L0) instead of all the input files. This eliminate the locking for non-bottommost level input files while waiting for bottom priority thread is up to run. - Re-pick compaction that outputs to max output level when bottom priority thread is up to run - Refactor universal compaction picking logic to make it cleaner and easier to force picking compaction with max output level when bottom priority thread is up to run - Guard feature behind a temporary option as requested Pull Request resolved: https://github.com/facebook/rocksdb/pull/13633 Test Plan: - New unit test to cover the case that's not covered by existing tests - bottom priority thread re-picks compaction ends up picking nothing due to LSM shape changes - Adapted existing unit tests to verify various bottom priority compaction behavior with this new option - Stress test `python3 tools/db_crashtest.py --simple blackbox --compaction_style=1 --target_file_size_base=1000 --write_buffer_size=1000 --compact_range_one_in=10000 --compact_files_one_in=10000 ` Reviewed By: cbi42 Differential Revision: D76005505 Pulled By: hx235 fbshipit-source-id: 9688f22d4a84f619452820f12f15b765c17301fd |
||
|
Changyu Bi
|
0119a8c78b |
Fix Checkpoint::ExportColumnFamily() returning staled data (#13654)
Summary: `Checkpoint::ExportColumnFamily()` calls DB::Flush() before getting all SST file metadata through `GetColumnFamilyMetaData()`. `GetColumnFamilyMetaData()` gets metadata through the SuperVersion but Flush() does not guarantee the flush result is reflected in SuperVersion upon return (explained below). This PR updates `GetColumnFamilyMetaData()` to get metadata from version instead. Since `GetColumnFamilyMetaData()` [acquires db mutex](https://github.com/facebook/rocksdb/blob/0c533e61bc6d89fdf1295e8e0bcee4edb3aef401/db/db_impl/db_impl.cc#L5023-L5031), it should not need to acquire SV anyway. Fixes https://github.com/facebook/rocksdb/issues/13652. Here we explain how Flush(wait=true) does not guarantee that the flush result is in SuperVersion when the call returns. - RocksDB uses group commit to do MANIFEST update. - When a flush tries to install its flush result, it may be done by another MANIFEST writer. - MANIFEST write is done atomically together with updating Version and cfd->imm() (the list of immutable memtables), but it does not install new SuperVresion - When the MANIFEST writer releases db mutex, the flush wait thread finds that cfd->imm() does not have the relevant memtable anymore: https://github.com/facebook/rocksdb/blob/09175119d2464d7ceecdf1cb7d6d5b517b730965/db/db_impl/db_impl_compaction_flush.cc#L2739-L2742 Pull Request resolved: https://github.com/facebook/rocksdb/pull/13654 Test Plan: the repro in https://github.com/pcholakov/rocksdb/commit/a52d426e82ff5a3dd181dbd5d676dbb54080f5fa pass after this change. Reviewed By: hx235 Differential Revision: D75795658 Pulled By: cbi42 fbshipit-source-id: 4f10baff67944bcd762cf0d237d653a8a35dbca3 |
||
|
Changyu Bi
|
1d94aeea44 |
Refactor snapshot context into JobContext and fix deadlock on db mutex in WP/WUP (#13632)
Summary: With WP/WUP, we can deadlock on db mutex here: https://github.com/facebook/rocksdb/blob/8dc3d77b591443e405b2b171b3eb4f8461ffd2a3/db/db_impl/db_impl_compaction_flush.cc#L4626. Here we release a snapshot (which will acquire db mutex) while already holding the mutex. This caused some transaction lock timeout error in crash test. This PR fixes this by refactoring snapshot related context into JobContext and only allow snapshot related context to be initialized once. This also reduces the number of parameters being passed around. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13632 Test Plan: - existing tests - this fails with timeout before this fix ``` ./db_stress --WAL_size_limit_MB=0 --WAL_ttl_seconds=60 --acquire_snapshot_one_in=10000 --adaptive_readahead=1 --adm_policy=2 --advise_random_on_open=1 --allow_data_in_errors=True --allow_fallocate=0 --allow_unprepared_value=1 --async_io=0 --auto_readahead_size=0 --auto_refresh_iterator_with_snapshot=0 --avoid_flush_during_recovery=0 --avoid_flush_during_shutdown=1 --avoid_unnecessary_blocking_io=0 --backup_max_size=104857600 --backup_one_in=100000 --batch_protection_bytes_per_key=8 --bgerror_resume_retry_interval=1000000 --block_align=0 --block_protection_bytes_per_key=4 --block_size=16384 --bloom_before_level=6 --bloom_bits=2 --bottommost_compression_type=lz4 --bottommost_file_compaction_delay=0 --bytes_per_sync=0 --cache_index_and_filter_blocks=1 --cache_index_and_filter_blocks_with_high_priority=0 --cache_size=33554432 --cache_type=fixed_hyper_clock_cache --charge_compression_dictionary_building_buffer=1 --charge_file_metadata=0 --charge_filter_construction=1 --charge_table_reader=0 --check_multiget_consistency=1 --check_multiget_entity_consistency=0 --checkpoint_one_in=0 --checksum_type=kxxHash --clear_column_family_one_in=0 --commit_bypass_memtable_one_in=0 --compact_files_one_in=1000 --compact_range_one_in=1000000 --compaction_pri=1 --compaction_readahead_size=0 --compaction_style=1 --compaction_ttl=0 --compress_format_version=2 --compressed_secondary_cache_ratio=0.0 --compressed_secondary_cache_size=0 --compression_checksum=1 --compression_max_dict_buffer_bytes=0 --compression_max_dict_bytes=0 --compression_parallel_threads=1 --compression_type=zlib --compression_use_zstd_dict_trainer=0 --compression_zstd_max_train_bytes=0 --continuous_verification_interval=0 --create_timestamped_snapshot_one_in=0 --daily_offpeak_time_utc= --data_block_index_type=1 --db_write_buffer_size=0 --decouple_partitioned_filters=1 --default_temperature=kWarm --default_write_temperature=kHot --delete_obsolete_files_period_micros=30000000 --delpercent=5 --delrangepercent=0 --destroy_db_initially=1 --detect_filter_construct_corruption=1 --disable_file_deletions_one_in=1000000 --disable_manual_compaction_one_in=1000000 --disable_wal=0 --dump_malloc_stats=1 --enable_checksum_handoff=0 --enable_compaction_filter=0 --enable_custom_split_merge=0 --enable_do_not_compress_roles=0 --enable_index_compression=0 --enable_memtable_insert_with_hint_prefix_extractor=0 --enable_pipelined_write=0 --enable_remote_compaction=0 --enable_sst_partitioner_factory=0 --enable_thread_tracking=0 --enable_write_thread_adaptive_yield=1 --error_recovery_with_no_fault_injection=0 --exclude_wal_from_write_fault_injection=1 --fifo_allow_compaction=1 --file_checksum_impl=none --file_temperature_age_thresholds= --fill_cache=1 --flush_one_in=1000 --format_version=3 --get_all_column_family_metadata_one_in=10000 --get_current_wal_file_one_in=0 --get_live_files_apis_one_in=10000 --get_properties_of_all_tables_one_in=100000 --get_property_one_in=1000000 --get_sorted_wal_files_one_in=0 --hard_pending_compaction_bytes_limit=274877906944 --high_pri_pool_ratio=0 --index_block_restart_interval=2 --index_shortening=1 --index_type=0 --ingest_external_file_one_in=0 --ingest_wbwi_one_in=0 --initial_auto_readahead_size=16384 --inplace_update_support=0 --iterpercent=10 --key_len_percent_dist=1,30,69 --key_may_exist_one_in=100 --last_level_temperature=kWarm --level_compaction_dynamic_level_bytes=0 --lock_wal_one_in=1000000 --log2_keys_per_lock=10 --log_file_time_to_roll=60 --log_readahead_size=16777216 --long_running_snapshots=1 --low_pri_pool_ratio=0 --lowest_used_cache_tier=1 --manifest_preallocation_size=0 --manual_wal_flush_one_in=0 --mark_for_compaction_one_file_in=0 --max_auto_readahead_size=0 --max_background_compactions=20 --max_bytes_for_level_base=10485760 --max_key=100000 --max_key_len=3 --max_log_file_size=1048576 --max_manifest_file_size=1073741824 --max_sequential_skip_in_iterations=1 --max_total_wal_size=0 --max_write_batch_group_size_bytes=16777216 --max_write_buffer_number=3 --max_write_buffer_size_to_maintain=8388608 --memtable_insert_hint_per_batch=1 --memtable_max_range_deletions=1000 --memtable_op_scan_flush_trigger=100 --memtable_prefix_bloom_size_ratio=0 --memtable_protection_bytes_per_key=1 --memtable_whole_key_filtering=0 --memtablerep=skip_list --metadata_charge_policy=0 --metadata_read_fault_one_in=0 --metadata_write_fault_one_in=0 --min_write_buffer_number_to_merge=2 --mmap_read=1 --mock_direct_io=False --nooverwritepercent=1 --num_file_reads_for_auto_readahead=0 --open_files=500000 --open_metadata_read_fault_one_in=8 --open_metadata_write_fault_one_in=0 --open_read_fault_one_in=32 --open_write_fault_one_in=0 --ops_per_thread=200000 --optimize_filters_for_hits=1 --optimize_filters_for_memory=1 --optimize_multiget_for_io=1 --paranoid_file_checks=1 --paranoid_memory_checks=0 --partition_filters=0 --partition_pinning=0 --pause_background_one_in=10000 --periodic_compaction_seconds=0 --prefix_size=-1 --prefixpercent=0 --prepopulate_block_cache=0 --preserve_internal_time_seconds=60 --progress_reports=0 --promote_l0_one_in=0 --read_amp_bytes_per_bit=0 --read_fault_one_in=32 --readahead_size=524288 --readpercent=50 --recycle_log_file_num=0 --reopen=20 --report_bg_io_stats=1 --reset_stats_one_in=1000000 --sample_for_compression=5 --secondary_cache_fault_one_in=0 --snapshot_hold_ops=100000 --soft_pending_compaction_bytes_limit=68719476736 --sqfc_name=bar --sqfc_version=0 --sst_file_manager_bytes_per_sec=104857600 --sst_file_manager_bytes_per_truncate=1048576 --stats_dump_period_sec=10 --stats_history_buffer_size=0 --strict_bytes_per_sync=0 --subcompactions=4 --sync=0 --sync_fault_injection=0 --table_cache_numshardbits=6 --target_file_size_base=524288 --target_file_size_multiplier=2 --test_batches_snapshots=0 --test_ingest_standalone_range_deletion_one_in=0 --top_level_index_pinning=0 --two_write_queues=1 --txn_write_policy=2 --uncache_aggressiveness=2 --universal_max_read_amp=10 --unordered_write=0 --unpartitioned_pinning=1 --use_adaptive_mutex=1 --use_adaptive_mutex_lru=0 --use_attribute_group=0 --use_delta_encoding=0 --use_direct_io_for_flush_and_compaction=0 --use_direct_reads=0 --use_full_merge_v1=0 --use_get_entity=0 --use_merge=1 --use_multi_cf_iterator=0 --use_multi_get_entity=0 --use_multiget=1 --use_optimistic_txn=0 --use_put_entity_one_in=0 --use_sqfc_for_range_queries=1 --use_timed_put_one_in=0 --use_txn=1 --use_write_buffer_manager=0 --user_timestamp_size=0 --value_size_mult=32 --verification_only=0 --verify_checksum=1 --verify_checksum_one_in=1000 --verify_compression=1 --verify_db_one_in=10000 --verify_file_checksums_one_in=0 --verify_iterator_with_expected_state_one_in=5 --verify_sst_unique_id_in_manifest=1 --wal_bytes_per_sync=0 --wal_compression=zstd --write_buffer_size=4194304 --write_dbid_to_manifest=1 --write_fault_one_in=0 --write_identity_file=0 --writepercent=35 --db=/dev/shm/rocksdb_test/rocksdb_crashtest_whitebox ``` Reviewed By: hx235 Differential Revision: D75173149 Pulled By: cbi42 fbshipit-source-id: ec68cadc78469730dfe26824e20b8ca4ab993101 |
||
|
Michael C Huang
|
13d865f6f1 |
Add trivial copy support when FIFO compaction reason is kChangeTemperature (#13562)
Summary: Prior to this PR, for FIFO kChangeTemperature compaction was done by iterating and reading thru the input sst and generate the output sst. This was wasteful since for FIFO we could apply the "trivial" move by copying the input sst to the out sst without need decompress/compress and reading thru the input sst content at all. This PR added "allow_trivial_copy_when_change_temperature" to the CompactionOptionsFIFO. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13562 Reviewed By: cbi42 Differential Revision: D73295404 Pulled By: mikechuangmeta fbshipit-source-id: 02241c7389797730ecd4a3b636837cb5f912b424 |
||
|
Peter Dillinger
|
2a0ee4ddd8 |
Refactor wal related naming and more (#13490)
Summary: * Clarify in API comments which `log_` options in DBOptions relate to WALs, info log, and/or manifest files. * Rename a bunch of "log" things to "wal" for clarity, especially in DBImpl. (More to go, especially some more challenging cases like `DBImpl::logs_`, but a step in the right direction IMHO) * Simplify DBImpl ctor by moving constant initializers to field definitions. * Use RelaxedAtomic for (renamed) `wals_total_size_` Pull Request resolved: https://github.com/facebook/rocksdb/pull/13490 Test Plan: existing tests Reviewed By: cbi42 Differential Revision: D71939382 Pulled By: pdillinger fbshipit-source-id: 852f4737eca83e6ad653010cc197ad1b6e6bae13 |
||
|
Peter Dillinger
|
82794e0a4f |
Deprecate RangePtr, favor new RangeOpt and OptSlice (#13481)
Summary: The new API in https://github.com/facebook/rocksdb/issues/13453 is awkward and precarious because of using RangePtr, which encodes optional keys using raw pointers to Slice. We could use `std::optional<Slice>` instead but that is unsatisfyingly a larger object with an inefficient size (typically 17 bytes). Here I introduce a custom optional Slice type, `OptSlice`, that is the same size as a Slice, and use it in a number of places to clean up code and make some public APIs easier to work with. This includes * `atomic_replace_range` (not yet released, OK to change) * `GetAllKeyVersions()` which gets a behavior change because of its unusual handling of empty keys. * `DeleteFilesInRanges()` * TODO in follow-up: `CompactRange()` Most of the diff is associated updates and refactorings. Also * Move some relevant things out of db.h to keep it as tidy as possible. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13481 Test Plan: tests updated Reviewed By: hx235 Differential Revision: D71747774 Pulled By: pdillinger fbshipit-source-id: b4c8519608d119b8bceca9bb0fd778608f62a141 |
||
|
Yu Zhang
|
0b815cf3b3 |
Add a CompactionJobStats.num_input_files_trivially_moved field (#13479)
Summary: This PR adds a new field `CompactionJobStats.num_input_files_trivially_moved` representing the number of files this compaction trivially moved. It should either equal to the total number of input files, or being 0. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13479 Test Plan: Added tests Reviewed By: hx235 Differential Revision: D71638796 Pulled By: jowlyzhang fbshipit-source-id: 794c085408a0dc95f11874ca60fca3e6b5b92cba |
||
|
Hui Xiao
|
24952ff088 |
Expose number of L0 files in the CF right before the compaction starts in CompactionJobInfo (#13462)
Summary: **Context/Summary:** For users who are interested in knowing how efficient their compaction in reducing L0 files or how bad their long-running compaction in "locking" L0 files, they now have a reference point "L0 files in the CF pre compaction" for their input compaction files. - Compared to the existing stats or exposing in some other way, exposing this info in CompactionJobInfo allows users to compare it with other compaction data (e.g, compaction input num, compaction reason) of within **one** compaction (of per-compaction granularity). - If this number is high while their "short-running" compaction has little L0 files input, then those compaction may have a room for improvement. Similar for those long-running compaction. This PR is to add a new field `CompactionJobInfo::num_l0_files_pre_compaction` for that. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13462 Test Plan: - Piggyback on an existing test Reviewed By: jaykorean Differential Revision: D71124938 Pulled By: hx235 fbshipit-source-id: aa47c9c86c62d9425771b320f5636e50671fd289 |
||
|
Peter Dillinger
|
da8eba8b49 |
Improve consistency of SeqnoToTime tracking in SuperVersion (#13316)
Summary: This is an unexpectedly complex follow-up to https://github.com/facebook/rocksdb/issues/13269. This change solves (and detects regressed) inconsistencies between whether a CF's SuperVersion is configured with a preserve/preclude option and whether it gets a usable SeqnoToTimeMapping. Operating with preserve/preclude and no usable mapping is degraded functionality we need to avoid. And no mapping is useful for actually disabling the feature (except with respect to existing SST files, but that's less of a concern for now). The challenge is that how we maintain the DB's SeqnoToTimeMapping can depend on all the column families, and we don't want to iterate over all column families *for each column family* (e.g. on initially creating each). The existing code was a bit relaxed: * On initially creating or re-configuring a CF, we might install an empty mapping, but soon thereafter (after releasing and re-acquiring the DB mutex) re-install another SuperVersion with a useful mapping. The solution here is to refactor the logic so that there's a distinct but related workflow for (a) ensuring a quality set of mappings when we might only be considering a single CF (`EnsureSeqnoToTimeMapping()`), and (b) massaging that set of mappings to account for all CFs (`RegisterRecordSeqnoTimeWorker`) which doesn't need to re-install new SuperVersions because each CF already has good mappings and will get updated SuperVersions when the periodic task adds new mappings. This should eliminate the extra SuperVersion installs associated with preserve/preclude on CF creation or re-configure, making it the same as any other CF. Some more details: * Some refactorings such as removing new_seqno_to_time_mapping from SuperVersionContext. (Now use parameter instead of being stateful.) * Propagate `read_only` aspect of DB to more places so that we can pro-actively disable preserve/preclude on read-only DBs, so that we don't run afoul of the assertion expecting SeqnoToTime entries. * Introduce a utility struct `MinAndMaxPreserveSeconds` for aggregating preserve/preclude settings in a useful way, sometimes on one CF and sometimes across multiple CFs. Much cleaner! (IMHO) * Introduce a function `InstallSuperVersionForConfigChange` that is a superset of `InstallSuperVersionAndScheduleWork` for when a CF is new or might have had a change to its mutable options. * Eliminate redundant re-install SuperVersions of created "missing" CFs in DBImpl::Open. Intended follow-up: * Ensure each flush has an "upper bound" SeqnoToTime entry, which would resolve a FIXME in tiered_compaction_test, but causes enough test churn to deserve its own PR + investigation. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13316 Test Plan: This change is primarily validated by a new assertion in SuperVersion::Init to ensure consistency between (a) presence of any SeqnoToTime mappings in the SuperVersion and (b) preserve/preclude option being currently set. One unit test update was needed because we now ensure at least one SeqnoToTime entry is created on any DB::Open with preserve/preclude, so that there is a lower bound time on all the future data writes. This required a small hack in associating the time with Seqno 1 instead of 0, which is reserved for "unspecified old." Reviewed By: cbi42 Differential Revision: D70540638 Pulled By: pdillinger fbshipit-source-id: bb419fdbeb5a1f115fc429c211f9b8efaf2f56d7 |
||
|
Andrew Chang
|
62531da510 |
Track the total number of compaction sorted runs from inside CompactionMergingIterator (#13325)
Summary: **This PR adds a new statistic to track the total number of sorted runs for running compactions.** Context: I am currently working on a separate project, where I am trying to tune the read request sizes made by `FilePrefetchBuffer` to the storage backend. In this particular case, `FilePrefetchBuffer` will issue larger reads and have to buffer larger read responses. This means we expect to see higher memory utilization. At least for the initial rollout, we only want to enable this optimization for compaction reads. **I want some way to get a sense of what the memory usage _impact_ will be if the prefetch read request size is increased from (for instance) 8MB to 64MB.** **If I know the number of files that compactions are actively reading from (i.e. the number of sorted runs / "input iterators"), I can determine how much the memory usage will increase if I bump up the readahead size inside `FilePrefetchBuffer`.** For instance, if there are 16 sorted runs at any given point in time and I bump up the readahead size by 64MB, I can project an increase of 16 * 64 MB. In most cases, the number of sorted runs processed per compaction is the number of L0 files plus the number of non-L0 levels. However, we need to be aware of exceptions like trivial compactions, deletion compactions, and subcompactions. This is a major reason why this PR chooses to implement the stats counting inside `CompactionMergingIterator`, since by the time we get down to that part of the stack, we know the "true" values for the number of input iterators / sorted runs. Alternatives considered: - https://github.com/facebook/rocksdb/issues/13299 gives you a histogram for the number of sorted runs ("input iterators") for a _single compaction_. While this statistic is interested and in the direction of what we want, we are going to be assessing the memory impact across _all_ compactions that are currently running. Thus, this statistic does not give us all the information we need. - https://github.com/facebook/rocksdb/issues/13302 gives you the total prefetch buffer memory usage, but it doesn't tell you what happens when the readahead size is increased. Furthermore, the code change is error prone and very "invasive" -- look at how many places in the code had to be updated. This would be useful in the future for general memory accounting purposes, but it does not serve our immediate needs. - https://github.com/facebook/rocksdb/issues/13320 aimed to track the same metric, but did this inside `DbImpl:: BackgroundCallCompaction`. It turns out that this does not handle the case where a compaction is divided into multiple subcompactions (in which case, there would be _more_ sorted runs being processed at the same time than you would otherwise predict.) The current PR handles subcompactions automatically, and I think it is cleaner overall. Note: When I attempted to put this statistic as part of the `cf_stats_value_` array, even after updating the array to use `std::atomic<uint64_t>`, I still was able to get assertions to _fail_ inside the crash tests. These assertions checked that the unsigned integer would not underflow below zero during compaction. I experimented for many hours but could not figure out a solution, even though it would seem like things "should" work with `fetch_add` and `fetch_sub`. One possibility is that the values in `cf_stats_value_` are being cleared to 0, but I added a `fprintf` to that portion of the code and didn't see it getting printed out before my assertions failed. Regardless, I think that this statistic is different enough from the CF-specific and the other DB-wide stats that the best solution is to just have it defined as a separate `std::atomic<uint64_t>`. I also do not want to spend more hours trying to debug why the crash test assertions break, when the solution in the current version of the PR can get the assertions to consistently pass. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13325 Test Plan: - I updated one unit test to confirm that `num_running_compaction_sorted_runs` starts and ends at 0. This checks that all the additions and subtractions cancel out. I also made sure the statistic got incremented at least once. - When I added `fprintf` manually, I confirmed that my statistics updating code was being exercised numerous times inside `db_compaction_test`. I printed out the results before and after the increments/decrements, and the numbers looked good. - We will monitor the generated statistics after this PR is merged. - There are assertion checks after each increment and before each decrement. If there are bugs, the crash test will almost certainly find them, since they quickly found issues with my initial implementation for this PR which tried using the `cf_stats_value_` array (modified to use `std::atomic`). Reviewed By: anand1976, hx235 Differential Revision: D68527895 Pulled By: archang19 fbshipit-source-id: 135cf210e0ff1550ea28ae4384d429ae620b1784 |
||
|
Peter Dillinger
|
780280b52e |
Reduce unnecessary MutableCFOptions copies and parameters (#13301)
Summary: As follow-up to https://github.com/facebook/rocksdb/issues/13239, this change is primarily motivated by simplifying the calling conventions of LogAndApply. Since it must be called while holding the DB mutex, it can read safely read cfd->GetLatestMutableCFOptions(), until it releases the mutex within ProcessManifestWrites. Before it releases the mutex, it makes a copy of the mutable options in a new, unpublished Version object, which can be used when not holding the DB mutex. This eliminates the need for callers of LogAndApply to copy mutable options for its sake, or even specify mutable options at all. And it eliminates the need for *another* copy to be saved in ManifestWriter. Other functions that don't need the mutable options parameter: * ColumnFamilyData::CreateNewMemtable() * CompactionJob::Install() / InstallCompactionResults() * MemTableList::*InstallMemtable*() * Version::PrepareAppend() Pull Request resolved: https://github.com/facebook/rocksdb/pull/13301 Test Plan: existing tests, CI with sanitizers Reviewed By: mszeszko-meta Differential Revision: D68234865 Pulled By: pdillinger fbshipit-source-id: 6ce95f9cc479834e09ffc8ce93cbae7b664329e5 |
||
|
Peter Dillinger
|
b3333587eb |
Clean up some CFOptions code hygiene, fix SetOptions() bug (#13294)
Summary: To start, I wanted to remove the unnecessary new_options parameter of `InstallSuperVersionAndScheduleWork()`. Passing it something other than the latest mutable options would be inconsistent/outdated. There was even a comment "Use latest MutableCFOptions" on a place that was using the saved options in effect for the compaction. On investigation, this fixes an undiagnosed but longstanding serious bug in SetOptions() where the new settings can be reverted if a flush or compaction started before the SetOptions() finishes after. Fix confirmed with new unit test in db_test.cc. I also got tired of seeing the cumbersome usage of pointer rather than const reference for related options accesses, so there's kind of a large (but trivial) refactoring tied in here as well. (Sorry for combining them; wasn't planning a major bug fix) Intended follow-up: Clarify/simplify the crazy calling conventions of LogAndApply, and remove some unnecessary copying of MutableCFOptions (see new FIXMEs) Pull Request resolved: https://github.com/facebook/rocksdb/pull/13294 Test Plan: test for bug fix, confirmed fails on main and at least as far back as version 8.10. Plus existing tests and CI Reviewed By: mszeszko-meta Differential Revision: D68141563 Pulled By: pdillinger fbshipit-source-id: f6c3290145afa06cc2fe8b485a5de17560a5deea |
||
|
Peter Dillinger
|
54b614de5b |
Some tiered storage refactorings setting up more work (#13230)
Summary: * Simplify some testing callbacks for tiered_compaction_test ahead of some significant functional updates. * Refactor CompactionJob::Prepare() for sharing with CompactionServiceCompactionJob. This is a minor functional change in computing preserve/preclude sequence numbers for remote compaction, but it is a start toward support for tiered storage with remote compaction. A test is added that is only partly working but does check that outputs are being split (just not to the correct levels). Pull Request resolved: https://github.com/facebook/rocksdb/pull/13230 Test Plan: mostly test changes and additions. Arguably makes tiered storage + remote compaction MORE broken as a step toward supporting it. Reviewed By: jaykorean Differential Revision: D67493682 Pulled By: pdillinger fbshipit-source-id: fd6db74e08ef0e4fc7fdd599ff8555aab0c8ddc4 |
||
|
Peter Dillinger
|
c72e79a262 |
Standardize on clang-format version 18 (#13233)
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 |
||
|
Yu Zhang
|
8089eae240 |
Fix assertion that compaction input files are freeed (#13109)
Summary: This assertion could fail if the compaction input files were successfully trivially moved. On re-locking db mutex after successful `LogAndApply`, those files could have been picked up again by some other compactions. And the assertion will fail. Example failure: P1669529213 Pull Request resolved: https://github.com/facebook/rocksdb/pull/13109 Reviewed By: cbi42 Differential Revision: D65308574 Pulled By: jowlyzhang fbshipit-source-id: 32413bdc8e28e67a0386c3fe6327bf0b302b9d1d |
||
|
Peter Dillinger
|
e7ffca9493 |
Refactoring toward making preserve/preclude options mutable (#13114)
Summary: Move them to MutableCFOptions and perform appropriate refactorings to make that work. I didn't want to mix up refactoring with interesting functional changes. Potentially non-trivial bits here: * During DB Open or RegisterRecordSeqnoTimeWorker we use `GetLatestMutableCFOptions()` because either (a) there might not be a current version, or (b) we are in the process of applying the desired next options. * Upgrade some test infrastructure to allow some options in MutableCFOptions to be mutable (should be a temporary state) * Fix a warning that showed up about uninitialized `paranoid_memory_checks` Pull Request resolved: https://github.com/facebook/rocksdb/pull/13114 Test Plan: existing tests, manually check options are still not settable with SetOptions Reviewed By: jowlyzhang Differential Revision: D65429031 Pulled By: pdillinger fbshipit-source-id: 6e0906d08dd8ddf62731cefffe9b8d94149942b9 |
||
|
changyubi
|
2ce6902cf5 |
Introduce an interface ReadOnlyMemTable for immutable memtables (#13107)
Summary: This PR sets up follow-up changes for large transaction support. It introduces an interface that allows custom implementations of immutable memtables. Since transactions use a WriteBatchWithIndex to index their operations, I plan to add a ReadOnlyMemTable implementation backed by WriteBatchWithIndex. This will enable direct ingestion of WriteBatchWithIndex into the DB as an immutable memtable, bypassing memtable writes for transactions. The changes mostly involve moving required methods for immutable memtables into the ReadOnlyMemTable class. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13107 Test Plan: * Existing unit test and stress test. * Performance: I do not expect this change to cause noticeable performance regressions with LTO and devirtualization. The memtable-only readrandom benchmark shows no consistent performance difference: ``` USE_LTO=1 OPTIMIZE_LEVEL="-O3" DEBUG_LEVEL=0 make -j160 db_bench (for I in $(seq 1 50);do ./db_bench --benchmarks=fillseq,readrandom --write_buffer_size=268435456 --writes=250000 --num=250000 --reads=500000 --seed=1723056275 2>&1 | grep "readrandom"; done;) | awk '{ t += $5; c++; print } END { print 1.0 * t / c }'; 3 runs: main: 760728, 752727, 739600 PR: 763036, 750696, 739022 ``` Reviewed By: jowlyzhang Differential Revision: D65365062 Pulled By: cbi42 fbshipit-source-id: 40c673ab856b91c65001ef6d6ac04b65286f2882 |
||
|
Yu Zhang
|
9c94559de7 |
Optimize compaction for standalone range deletion files (#13078)
Summary: This PR adds some optimization for compacting standalone range deletion files. A standalone range deletion file is one with just a single range deletion. Currently, such a file is used in bulk loading to achieve something like atomically delete old version of all data with one big range deletion and adding new version of data. These are the changes included in the PR: 1) When a standalone range deletion file is ingested via bulk loading, it's marked for compaction. 2) When picking input files during compaction picking, we attempt to only pick a standalone range deletion file when oldest snapshot is at or above the file's seqno. To do this, `PickCompaction` API is updated to take existing snapshots as an input. This is only done for the universal compaction + UDT disabled combination, we save querying for existing snapshots and not pass it for all other cases. 3) At `Compaction` construction time, the input files will be filtered to examine if any of them can be skipped for compaction iterator. For example, if all the data of the file is deleted by a standalone range tombstone, and the oldest snapshot is at or above such range tombstone, this file will be filtered out. 4) Every time a snapshot is released, we examine if any column family has standalone range deletion files that becomes eligible to be scheduled for compaction. And schedule one for it. Potential future improvements: - Add some dedicated statistics for the filtered files. - Extend this input filtering to L0 files' compactions cases when a newer L0 file could shadow an older L0 file Pull Request resolved: https://github.com/facebook/rocksdb/pull/13078 Test Plan: Added unit tests and stress tested a few rounds Reviewed By: cbi42 Differential Revision: D64879415 Pulled By: jowlyzhang fbshipit-source-id: 02b8683fddbe11f093bcaa0a38406deb39f44d9e |
||
|
Jay Huh
|
f22557886e |
Fix Compaction Stats (#13071)
Summary: Compaction stats code is not so straightforward to understand. Here's a bit of context for this PR and why this change was made. - **CompactionStats (compaction_stats_.stats):** Internal stats about the compaction used for logging and public metrics. - **CompactionJobStats (compaction_job_stats_)**: The public stats at job level. It's part of Compaction event listener and included in the CompactionResult. - **CompactionOutputsStats**: output stats only. resides in CompactionOutputs. It gets aggregated toward the CompactionStats (internal stats). The internal stats, `compaction_stats_.stats`, has the output information recorded from the compaction iterator, but it does not have any input information (input records, input output files) until `UpdateCompactionStats()` gets called. We cannot simply call `UpdateCompactionStats()` to fill in the input information in the remote compaction (which is a subcompaction of the primary host's compaction) because the `compaction->inputs()` have the full list of input files and `UpdateCompactionStats()` takes the entire list of records in all files. `num_input_records` gets double-counted if multiple sub-compactions are submitted to the remote worker. The job level stats (in the case of remote compaction, it's subcompaction level stat), `compaction_job_stats_`, has the correct input records, but has no output information. We can use `UpdateCompactionJobStats(compaction_stats_.stats)` to set the output information (num_output_records, num_output_files, etc.) from the `compaction_stats_.stats`, but it also sets all other fields including the input information which sets all back to 0. Therefore, we are overriding `UpdateCompactionJobStats()` in remote worker only to update job level stats, `compaction_job_stats_`, with output information of the internal stats. Baiscally, we are merging the aggregated output info from the internal stats and aggregated input info from the compaction job stats. In this PR we are also fixing how we are setting `is_remote_compaction` in CompactionJobStats. - OnCompactionBegin event, if options.compaction_service is set, `is_remote_compaction=true` for all compactions except for trivial moves - OnCompactionCompleted event, if any of the sub_compactions were done remotely, compaction level stats's `is_remote_compaction` will be true Other minor changes - num_output_records is already available in CompactionJobStats. No need to store separately in CompactionResult. - total_bytes is not needed. - Renamed `SubcompactionState::AggregateCompactionStats()` to `SubcompactionState::AggregateCompactionOutputStats()` to make it clear that it's only aggregating output stats. - Renamed `SetTotalBytes()` to `AddBytesWritten()` to make it more clear that it's adding total written bytes from the compaction output. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13071 Test Plan: Unit Tests added and updated ``` ./compaction_service_test ``` Reviewed By: anand1976 Differential Revision: D64479657 Pulled By: jaykorean fbshipit-source-id: a7a776a00dc718abae95d856b661bcbafd3b0ed5 |
||
|
Jay Huh
|
da5e11310b |
Preserve Options File (#13074)
Summary: In https://github.com/facebook/rocksdb/issues/13025 , we made a change to load the latest options file in the remote worker instead of serializing the entire set of options. That was done under assumption that OPTIONS file do not get purged often. While testing, we learned that this happens more often than we want it to be, so we want to prevent the OPTIONS file from getting purged anytime between when the remote compaction is scheduled and the option is loaded in the remote worker. Like how we are protecting new SST files from getting purged using `min_pending_output`, we are doing the same by keeping track of `min_options_file_number`. Any OPTIONS file with number greater than `min_options_file_number` will be protected from getting purged. Just like `min_pending_output`, `min_options_file_number` gets bumped when the compaction is done. This is only applicable when `options.compaction_service` is set. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13074 Test Plan: ``` ./compaction_service_test --gtest_filter="*PreservedOptionsLocalCompaction*" ./compaction_service_test --gtest_filter="*PreservedOptionsRemoteCompaction*" ``` Reviewed By: anand1976 Differential Revision: D64433795 Pulled By: jaykorean fbshipit-source-id: 0d902773f0909d9481dec40abf0b4c54ce5e86b2 |
||
|
Yu Zhang
|
9375c3b635 |
Fix needs_flush assertion in file ingestion (#13045)
Summary: This PR makes file ingestion job's flush wait a bit further until the SuperVersion is also updated. This is necessary since follow up operations will use the current SuperVersion to do range overlapping check and level assignment. In debug mode, file ingestion job's second `NeedsFlush` call could have been invoked when the memtables are flushed but the SuperVersion hasn't been updated yet, triggering the assertion. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13045 Test Plan: Existing tests Manually stress tested Reviewed By: cbi42 Differential Revision: D63671151 Pulled By: jowlyzhang fbshipit-source-id: 95a169e58a7e59f6dd4125e7296e9060fe4c63a7 |
||
|
Changyu Bi
|
e490f2b051 |
Fix a bug in ReFitLevel() where FileMetaData::being_compacted is not cleared (#13009)
Summary: in ReFitLevel(), we were not setting being_compacted to false after ReFitLevel() is done. This is not a issue if refit level is successful, since new FileMetaData is created for files at the target level. However, if there's an error during RefitLevel(), e.g., Manifest write failure, we should clear the being_compacted field for these files. Otherwise, these files will not be picked for compaction until db reopen. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13009 Test Plan: existing test. - stress test failure in T200339331 should not happen anymore. Reviewed By: hx235 Differential Revision: D62597169 Pulled By: cbi42 fbshipit-source-id: 0ba659806da6d6d4b42384fc95268b2d7bad720e |
||
|
Yu Zhang
|
a24574e80a |
Add documentation for background job's state transition (#12994)
Summary: The `SchedulePending*` API is a bit confusing since it doesn't immediately schedule the work and can be confused with the actual scheduling. So I have changed these to be `EnqueuePending*` and added some documentation for the corresponding state transitions of these background work. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12994 Test Plan: existing tests Reviewed By: cbi42 Differential Revision: D62252746 Pulled By: jowlyzhang fbshipit-source-id: ee68be6ed33070cad9a5004b7b3e16f5bcb041bf |
||
|
Hui Xiao
|
349b1ec08f |
Fix duplicate WAL entries caused by write after error recovery (#12873)
Summary: **Context/Summary:** We recently discovered a case where write of the same key right after error recovery of a previous failed write of the same key finishes causes two same WAL entries, violating our assertion. This is because we don't advance seqno on failed write and reuse the same WAL containing the failed write for the new write if the memtable at the time is empty. This PR reuses the flush path for an empty memtable to switch WAL and update min WAL to keep in error recovery flush as well as updates the INFO log message for clarity. ``` 2024/07/17-15:01:32.271789 327757 (Original Log Time 2024/07/17-15:01:25.942234) [/flush_job.cc:1017] [default] [JOB 2] Level-0 flush table https://github.com/facebook/rocksdb/issues/9: 0 bytes OK It's an empty SST file from a successful flush so won't be kept in the DB 2024/07/17-15:01:32.271798 327757 (Original Log Time 2024/07/17-15:01:32.269954) [/memtable_list.cc:560] [default] Level-0 commit flush result of table https://github.com/facebook/rocksdb/issues/9 started 2024/07/17-15:01:32.271802 327757 (Original Log Time 2024/07/17-15:01:32.271217) [/memtable_list.cc:760] [default] Level-0 commit flush result of table https://github.com/facebook/rocksdb/issues/9: memtable https://github.com/facebook/rocksdb/issues/1 done ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/12873 Test Plan: New UT that failed before this PR with following assertion failure (i.e, duplicate WAL entries) and passes after ``` db_wal_test: db/write_batch.cc:2254: rocksdb::Status rocksdb::{anonymous}::MemTableInserter::PutCFImpl(uint32_t, const rocksdb::Slice&, const rocksdb::Slice&, rocksdb::ValueType, RebuildTxnOp, const ProtectionInfoKVOS64*) [with RebuildTxnOp = rocksdb::{anonymous}::MemTableInserter::PutCF(uint32_t, const rocksdb::Slice&, const rocksdb::Slice&)::<lambda(rocksdb::WriteBatch*, uint32_t, const rocksdb::Slice&, const rocksdb::Slice&)>; uint32_t = unsigned int; rocksdb::ProtectionInfoKVOS64 = rocksdb::ProtectionInfoKVOS<long unsigned int>]: Assertion `seq_per_batch_' failed. ``` Reviewed By: anand1976 Differential Revision: D59884468 Pulled By: hx235 fbshipit-source-id: 5d854b719092552c69727a979f269fb7f6c39756 |
||
|
Changyu Bi
|
c064ac3bc5 |
Avoid opening table files and reading table properties under mutex (#12879)
Summary: InitInputTableProperties() can open and do IOs and is called under mutex_. This PR removes it from FinalizeInputInfo(). It is now called in CompactionJob::Run() and BuildCompactionJobInfo() (called in NotifyOnCompactionBegin()) without holding mutex_. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12879 Test Plan: existing unit tests. Added assert in GetInputTableProperties() to ensure that input_table_properties_ is initialized whenever it's called. Reviewed By: hx235 Differential Revision: D59933195 Pulled By: cbi42 fbshipit-source-id: c8089e13af8567fa3ab4b94d9ec384ae98ab2ec8 |
||
|
Yu Zhang
|
f5e44f3490 |
Fix manual flush hanging on waiting for no stall for UDT in memtable … (#12771)
Summary: This PR fix a possible manual flush hanging scenario because of its expectation that others will clear out excessive memtables was not met. The root cause is the FlushRequest rescheduling logic is using a stricter criteria for what a write stall is about to happen means than `WaitUntilFlushWouldNotStallWrites` does. Currently, the former thinks a write stall is about to happen when the last memtable is half full, and it will instead reschedule queued FlushRequest and not actually proceed with the flush. While the latter thinks if we already start to use the last memtable, we should wait until some other background flush jobs clear out some memtables before proceed this manual flush. If we make them use the same criteria, we can guarantee that at any time when`WaitUntilFlushWouldNotStallWrites` is waiting, it's not because the rescheduling logic is holding it back. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12771 Test Plan: Added unit test Reviewed By: ajkr Differential Revision: D58603746 Pulled By: jowlyzhang fbshipit-source-id: 9fa1c87c0175d47a40f584dfb1b497baa576755b |
||
|
Yu Zhang
|
13c758f986 |
Change the behavior of manual flush to not retain UDT (#12737)
Summary: When user-defined timestamps in Memtable only feature is enabled, all scheduled flushes go through a check to see if it's eligible to be rescheduled to retain user-defined timestamps. However when the user makes a manual flush request, their intention is for all the in memory data to be persisted into SST files as soon as possible. These two sides have some conflict of interest, the user can implement some workaround like https://github.com/facebook/rocksdb/issues/12631 to explicitly mark which one takes precedence. The implementation for this can be nuanced since the user needs to be aware of all the scenarios that can trigger a manual flush and handle the concurrency well etc. In this PR, we updated the default behavior to give manual flush precedence when it's requested. The user-defined timestamps rescheduling mechanism is turned off when a manual flush is requested. Likewise, all error recovery triggered flushes skips the rescheduling mechanism too. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12737 Test Plan: Add unit tests Reviewed By: ajkr Differential Revision: D58538246 Pulled By: jowlyzhang fbshipit-source-id: 0b9b3d1af3e8d882f2d6a2406adda19324ba0694 |
||
|
Yu Zhang
|
44aceb88d0 |
Add a OnManualFlushScheduled callback in event listener (#12631)
Summary: As titled. Also added the newest user-defined timestamp into the `MemTableInfo`. This can be a useful info in the callback. Added some unit tests as examples for how users can use two separate approaches to allow manual flush / manual compactions to go through when the user-defined timestamps in memtable only feature is enabled. One approach relies on selectively increase cutoff timestamp in `OnMemtableSeal` callback when it's initiated by a manual flush. Another approach is to increase cutoff timestamp in `OnManualFlushScheduled` callback. The caveats of the approaches are also documented in the unit test. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12631 Reviewed By: ajkr Differential Revision: D58260528 Pulled By: jowlyzhang fbshipit-source-id: bf446d7140affdf124744095e0a179fa6e427532 |