mirror of
https://github.com/facebook/rocksdb.git
synced 2026-10-24 19:59:35 +03:00
main
26 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Xingbo Wang
|
19b6f85266 |
Support async file open for SecondaryDB (#15294)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15294 Why the change is required: `open_files_async` reduces startup latency by moving SST validation and table-reader opening to background work, but `DB::OpenAsSecondary()` still performed that work synchronously. Previous behavior: A secondary using `open_files_async=true` replayed its MANIFEST through `ReactiveVersionSet`, synchronously verified and opened every current SST, and only then returned. The option therefore provided no startup benefit for secondary instances. New behavior: `DB::OpenAsSecondary()` can now return after installing its initial read view and start initial SST validation and table-reader pinning through the existing asynchronous opener. With `max_open_files=-1`, the opener processes all current SSTs in the background; a finite table cache can stop the eager pass at capacity and validate remaining files on demand. Later `TryCatchUpWithPrimary()` rounds remain synchronous. Async-open failures notify listeners and are latched according to the existing severity and `paranoid_checks` policy. If an error is not latched, a later read retries the file access and reports a persistent failure. A latched error requires fixing the underlying problem and reopening the secondary. Read-only instances do not start write-oriented recovery. Implementation: Initial reactive MANIFEST replay can defer SST verification and handler loading. The deferral is immutable constructor state on `ReactiveVersionSet`, is cleared before catch-up mode, and is restricted to normal secondary recovery. `VersionBuilder::GetAndClearIntermediateFiles()` now moves out and clears its own historical add-then-delete bookkeeping so callers cannot accidentally retain stale state. Secondary open schedules the background opener after publishing the read view. Its context pins the captured Versions and column families, including retained secondary handles whose column family was subsequently dropped by the primary. Existing table-reader opening performs SST size and unique-ID validation, avoiding a separate serial metadata pass. Closing the DB or canceling background work safely cancels a queued opener or waits for a running opener. Read-only async-open errors use an ErrorHandler path that notifies listeners without automatic recovery. Secondary async open explicitly rejects FIFO compaction. Safety, compatibility, and limitations: The default `open_files_async=false` path and existing primary async-open behavior are unchanged. WAL and blob recovery, as well as later secondary catch-up, remain synchronous. Follower DB and `OpenAndCompact()` do not use this deferral. There are no file-format, ABI, or public API signature changes. Secondary async open requires `skip_stats_update_on_db_open=true` and does not support FIFO compaction. Applications must keep files accessible by coordinating deletion with the primary or by using an Env/FileSystem that delays physical deletion until every instance releases a file. With asynchronous opening, `max_open_files=-1` cannot guarantee every SST is pinned before `OpenAsSecondary()` returns; the table-files-only `max_open_files=-1` workaround applies only when asynchronous opening is disabled. No completion API is added. Validation covers initial deferral and pinning, synchronous catch-up, table-reader detection of physical-size mismatch, FIFO rejection, listener-adjusted errors and synchronous read retry, retained handles after primary column-family deletion, close and explicit cancellation with no HIGH-priority worker, stopping new table opens during shutdown, primary async open, follower isolation, and remote compaction. Linux Buck builds and focused tests passed. Cross-platform CI is pending, and `make check-sources` cannot run successfully in this Sapling checkout because its script requires Git. Reviewed By: pdillinger Differential Revision: D122417547 fbshipit-source-id: 91d408469c35c4ea21165c5e7db75068e65c2389 |
||
|
Xingbo Wang
|
31675ca6d0 |
Recover from poisoned WAL handles on remote file systems (#15193)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15193 Keep `DB::Write`'s existing immediate `IOError` contract while allowing RocksDB to recover safely when a remote file system reports that a WAL handle is permanently unusable. The public API documentation makes the persisted-state boundary, reopen behavior, and safe post-resumption retry point explicit. Normal operation remains unchanged; the additional protocol is entered only after a supported file-scoped WAL error. ### Motivation A WAL write crosses an acknowledgement boundary: storage can accept the complete record and then fail before the caller receives confirmation. The resulting `IOError` therefore cannot tell RocksDB or the application whether the record is absent, partial, or complete. At the same time, the remote file handle can become permanently poisoned, so retrying that same handle can never succeed. The previous recovery path retried the poisoned handle. Simply replacing it is not sufficient: RocksDB must also prevent reuse of an ambiguous sequence range, keep every column family on one durable boundary, and prevent WAL tailers from publishing a physically complete record that successful live resumption deliberately abandoned. The new protocol quarantines the affected WALs, reserves the ambiguous sequence range, creates a replacement WAL, atomically persists acknowledged state, retires the quarantined handles, and only then resumes writes. ### Behavior changes #### 1. Recovering from a poisoned remote WAL handle - **Why required:** A remote file system can permanently poison a WAL handle after an I/O failure. Retrying that handle cannot succeed and leaves the database write-stopped. - **Previous behavior:** RocksDB retried the same WAL handle during recovery. - **New behavior:** The remote file system marks the failure as file-scoped. RocksDB creates a replacement WAL, atomically persists all state that could depend on the failed WAL, and then removes the quarantined WAL handles from active use. Physical file deletion remains part of normal cleanup. #### 2. Preserving an indeterminate WAL write outcome - **Why required:** A WAL record can reach storage before the append returns `IOError`; neither RocksDB nor the application can know the outcome from that error alone. - **Previous behavior:** Recovery could reuse a file-scoped failed write group's sequence numbers, allowing a later acknowledged write to overlap a record that might already be in the WAL. The public write API did not explain the acknowledgement ambiguity or distinguish live resumption from close/reopen recovery. - **New behavior:** `DB::Write` still returns the original `IOError` immediately and does not retry the logical batch. Normal DB reads can continue, but the failed write is not visible in that DB session. The persisted DB remains indeterminate with respect to the failed write until supported file-scoped live resumption succeeds without closing or reopening the DB. On reopen, or for any other WAL error without the durable-abandonment guarantee, applications must resolve consistency within RocksDB's documented guarantees, including `WALRecoveryMode`, through idempotency, deduplication, reconciliation, or an equivalent mechanism. After successful supported file-scoped live resumption, RocksDB has durably abandoned the failed batch and reserved its sequence range; retrying the same logical write is then recommended to preserve one linear logical history. #### 3. Keeping TransactionLogIterator consistent with live resumption - **Why required:** A physically complete error-returned record can remain readable from its WAL even though supported file-scoped recovery excludes that batch from the database. Returning it from `GetUpdatesSince()` would make replication or change-data-capture consumers diverge from the recovered DB. - **Previous behavior:** An iterator opened before recovery could return the abandoned batch. A new iterator could also return it while the physical WAL remained visible. - **New behavior:** For file-scoped failures, RocksDB records the exact sequence range and WAL number of each indeterminate write. For the lifetime of that open DB, existing and new WAL iterators stop with `Status::NotFound` before the range, whether the physical record is complete or absent. A caller can begin a new iterator after the reserved range. Iterator creation snapshots the recovery-gap generation together with its WAL-file view so a failure on a later, unseen WAL returns `TryAgain` for the old iterator instead of skipping its valid prefix. #### 4. Keeping all column families on one recovery boundary - **Why required:** A write batch can update multiple column families. Retiring its WAL after independently persisting only some column families can expose a partially applied batch after reopen. - **Previous behavior:** With `atomic_flush=false`, recovery could persist column families independently before abandoning the WAL. - **New behavior:** File-scoped WAL recovery force-flushes every live column family atomically and verifies that each one advanced beyond the failed WAL before writers are retired or writes resume. #### 5. Merging concurrent WAL failures - **Why required:** Another WAL write can fail while an earlier recovery attempt temporarily releases the DB mutex. - **Previous behavior:** A later failed WAL cutoff or ambiguous sequence could be overwritten or cleared by the earlier recovery attempt. - **New behavior:** Each error episode monotonically merges the largest failed WAL cutoff and the sequence range of the latest ambiguous write group. Recovery clears the quarantine cutoff only if no concurrent failure extended it; otherwise the newer failure remains write-stopping for the next attempt. Iterator fencing tracks the actual ambiguous write range separately, so an older failed `SyncWAL()` and a newer ambiguous write are not incorrectly paired. #### 6. Retaining file scope through `NoSpace` recovery - **Why required:** `NoSpace` can still identify one permanently unusable WAL handle. - **Previous behavior:** The `SstFileManager` recovery path entered generic recovery without carrying the failed WAL context. - **New behavior:** File-scoped `NoSpace` carries the same cutoff and sequence evidence through the `SstFileManager` path as other file-scoped WAL errors. #### 7. Scoping recovery context to one error episode - **Why required:** A completed recovery must not affect a later unrelated background error. - **Previous behavior:** WAL recovery context could remain latched after `ClearBGError` succeeded. - **New behavior:** Successful background-error clearing resets the context. A failed recovery retains it so a later retry has the same safety information. The separate iterator gap record remains for the lifetime of the open DB because later WAL readers must continue to reject the abandoned physical record. #### 8. Preserving successful `SyncWAL` metadata - **Why required:** A healthy WAL can finish syncing while a separate recovery error already exists. - **Previous behavior:** `SyncWAL` could return the existing background error before applying the successful WAL's `VersionEdit`, dropping its MANIFEST update. - **New behavior:** `SyncWAL` skips quarantined writers, applies metadata for successfully synced healthy WALs, and only then returns any existing recovery error. #### 9. Classifying only the intended remote-file-system failure - **Why required:** File-scoped recovery is valid only for an actual WAL `IOError` from a handle whose ownership is still valid. - **Previous behavior:** Poison state could be queried before ownership fencing was resolved, and file scope could be stamped onto a non-I/O status. - **New behavior:** Ownership fencing is resolved first, poison state is queried afterward, and only an actual `IOError` can be classified as file-scoped. `IOFenced` remains fatal. #### 10. Preserving `paranoid_checks=false` retry behavior - **Why required:** Non-paranoid mode historically cleared a writer's sticky error so a later operation could retry after storage recovered. - **Previous behavior:** Avoiding reclassification of a cached synthetic writer error also bypassed `WALIOStatusCheck`, so later `SyncWAL` calls kept returning the stale error. - **New behavior:** A cached error still fails the current `SyncWAL` call, but the non-paranoid path resets the writer for a later retry without setting a background error or entering file-scoped recovery. #### 11. Preserving async WAL precreation across recovery - **Why required:** A precreated WAL is an unpublished replacement and is not the active WAL. Recovery can also consume the prepared slot while background work is stopped. - **Previous behavior:** A file-scoped failure while creating or starting the unpublished replacement was attributed to the active WAL, unnecessarily quarantining it. After successful recovery consumed a prepared WAL, the next slot was not replenished, so the following rotation created its WAL synchronously. - **New behavior:** Only an active WAL `WriteBuffer()` failure attaches the active-WAL cutoff. A failed unpublished candidate is discarded while the current WAL remains active, and successful recovery schedules the next async precreation after `ClearBGError()` re-enables background work. ### Implementation - Carry `DBRecoverContext` through generic, `NoSpace`, and `SstFileManager` recovery, merging WAL cutoffs and failed write-group sequence ranges independently of error severity. Generic errors retain the range only so a later retry that establishes file scope can activate the stronger protocol; they do not persist a reservation or install an iterator gap on their own. - Document the WAL acknowledgement ambiguity, persisted-state boundary, reopen behavior, application consistency requirements, and post-resumption retry guidance on the public `DB::Write` API. - Publish a lock-free quarantine cutoff used by write and sync paths; replace even an empty failed current WAL; persist file-scoped ambiguous sequence reservations; force and verify an atomic all-column-family flush; and detach quarantined writers only after that durable boundary. - Record each file-scoped ambiguous write group's exact WAL number and sequence range in a DB-lifetime tracker shared by `WalManager` and all `WalIteratorImpl` instances. Stop an iterator before a batch that contains or crosses the range, and use a generation snapshot to preserve the existing `TryAgain` behavior when an iterator's fixed WAL-file view predates the failure. - Keep successful healthy-WAL tracking edits in `SyncWAL`, reset completed recovery context, and preserve legacy non-paranoid writer reset behavior. - Distinguish failures on an unpublished replacement WAL from failures flushing the active WAL. Log the failed replacement identity, keep the active WAL out of quarantine, and replenish the async-precreation slot after successful recovery. - Classify poisoned remote-file-system handles after ownership fencing and only for `IOError`. - Add lifecycle logging and a state-machine comment covering quarantine, sequence reservation, iterator fencing, replacement, atomic persistence, retirement, and concurrent-error handoff. - Add an opt-in `db_stress` mode that injects file-scoped faults only into WAL writes, disables WAL tracking in that mode, validates unsupported direct invocations, and sanitizes crash-test parameters to the supported recovery envelope. A small fraction of ordinary non-specialized crash-test runs select this mode automatically; existing stress behavior is unchanged when the flag is off. - Make the concurrent-error regression test hold the initial background flush until `SyncWAL` records the intended older-WAL error, preventing scheduler-dependent retirement of the test WAL before fault injection. ### Safety boundaries and unsupported cases - Automatic file-scoped WAL recovery fails closed with `NotSupported` for manual WAL flush, 2PC/transactions, two write queues, pipelined or unordered writes, WAL recycling, WAL TTL or size retention, either WAL-tracking mode, or blob direct writes. - Unsupported recovery leaves the DB write-stopped. `DB::Resume()` returns `NotSupported`; after correcting the I/O condition, operational recovery requires close and reopen. - The failed logical batch is never retried. Only supported file-scoped recovery durably reserves its sequence range and fences `GetUpdatesSince()`. Generic WAL errors retain the existing indeterminate outcome and can replay after close/reopen; applications must use idempotency or reconciliation unless successful file-scoped live resumption has provided the stronger guarantee. - Failed recovery attempts retain their context. Only a successful `ClearBGError` resets it. - Physical WAL deletion is not part of the recovery correctness boundary; normal obsolete-file cleanup removes files later. - The TransactionLogIterator ambiguity fence is kept for the lifetime of the DB instance that observed the file-scoped error and is not persisted as new MANIFEST metadata. After a later close/reopen, normal WAL recovery and obsolete-WAL retention/deletion semantics apply. - The in-memory iterator-gap tracker can grow with the number of recovered file-scoped failures during one DB lifetime. Safe pruning must account for retained WALs and existing iterator snapshots; it is left for follow-up. - Async WAL precreation remains disabled with WAL recycling. A prepared WAL is unpublished until its start records succeed; failure before installation does not make the active WAL file-scoped. - The new stress mode is test-only and defaults off, so normal `db_stress` and database operation without an error are unchanged. - Automatic stress-profile selection is limited to fully parsed, supported configurations. Explicit targeted runs reject options files and other unsupported combinations. ### Tests and known limitations - The affected recovery, WAL-manager, WAL-iterator, fault-injection, async-WAL-precreation, and secondary-close binaries build successfully. - Checked-status suites pass for all 70 `error_handler_fs_test` cases, all 7 `wal_manager_test` cases, all 11 `db_log_iter_test` cases, all 8 `fault_injection_fs_test` cases, and all 5 `DBWALTest.AsyncWalPrecreate*` cases. `DBSecondaryTest.SecondaryCloseFiles` passes 20 consecutive repetitions. - Focused regressions cover file-scoped gap fencing after an iterator snapshot, generic errors not installing a file-scoped sequence gap, and later recovery discovering that a generic error is file-scoped. - Earlier validation on this diff also passed crash-test unit coverage, remote-file-system adapter tests, repeated concurrency tests, two bounded multi-column-family stress runs, and bounded black-box crash/reopen dry runs. - The exact internal RocksDB format check and `arc f --take-formatters --enforce-lint-clean` report no formatting or lint issues. - The full crash/stress matrix and TSAN were not run. Reviewed By: pdillinger Differential Revision: D119324616 fbshipit-source-id: 05779d2bff7018f0ae69c4cbbe5b6455a45675d6 |
||
Josh Kang
|
9475798205 |
Refresh recovery plan after error escalation (#15111)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15111 T284514290 exposed an online-recovery race between the automatic recovery coordinator and a foreground synchronous write. A retryable no-WAL flush error selected lightweight retry-flush recovery. While that recovery attempt waited for the failed background flush, `Delete(sync=true)` appended and synced sequence N to the active WAL, then failed while persisting WAL-tracking metadata to the MANIFEST. Because memtable insertion and sequence publication happen after that MANIFEST step, the record was durable but logically unpublished. OLD: the concurrent error triggered another attempt, but the attempt reused a stale recovery plan. That preserved both ingredients needed for a duplicate: the first `Delete(key, N)` remained in the active WAL, while the published sequence remained `N-1`. ``` Recovery coordinator Foreground sync-write thread ------------------------------------ -------------------------------- [db_mutex held] plan = RetryFlush (copied once before the retry loop) attempt 1: ResumeImpl(plan) WaitForBackgroundWork() bg_cv_.Wait() [atomically releases db_mutex] ---------> assign sequence N append + sync Delete(key, N) [db_mutex held] MANIFEST WAL edit fails no memtable insert no LastSequence publication write returns I/O error recovery_error_ = Hard (requests another attempt) bg_error_ = Hard recover_context_ eventually becomes FullRecovery [db_mutex released] [db_mutex reacquired] <--------- ClearBGError() sees recovery_error_ attempt 1 cannot clear the error attempt 2: recovery_error_ = OK ResumeImpl(plan) ^ +-- still the cached RetryFlush plan RetryFlushesForErrorRecovery() retries the failed immutable flush only does not call SwitchMemtable() does not switch the active WAL ClearBGError() succeeds caller retries Delete(key) LastSequence is still N-1 => assigns sequence N again appends Delete(key, N) to same WAL memtable insert succeeds publishes LastSequence = N Reopen / WAL replay: first (key, N, deletion) -> memtable insert succeeds second (key, N, deletion) -> TryAgain("key+seq exists") normal WAL recovery has seq_per_batch_ == false assert(seq_per_batch_) fails ``` The cached argument to `ResumeImpl()` is therefore the direct bridge to the assertion: lightweight retry-flush recovery can report success without retiring the WAL that contains the unpublished record. Since it also does not publish sequence N, the successful caller retry writes the identical internal key into that same WAL. NEW: the same recovery coordinator consumes the concurrent error through a mutex-ordered handoff. ``` Recovery coordinator Foreground sync-write thread ------------------------------------ -------------------------------- [db_mutex held] attempt 1: plan = recover_context_ // RetryFlush ResumeImpl(plan) WaitForBackgroundWork() bg_cv_.Wait() [atomically releases db_mutex] ---------> append + sync Delete(key, seq=N) [db_mutex held] MANIFEST WAL edit fails CheckAndSet...(Hard, FullRecovery): recovery_error_ = Hard bg_error_ = Hard recover_context_ = FullRecovery [db_mutex released] [db_mutex reacquired] <--------- ClearBGError() sees recovery_error_ current attempt cannot clear the error next retry, with db_mutex held: recovery_error_ = OK plan = recover_context_ // FullRecovery ResumeImpl(plan) SwitchMemtable() switch to a new WAL clear the background error ``` The foreground unlock of `db_mutex_` happens before the recovery coordinator reacquires it and snapshots the next plan. Publishing `bg_error_` and `recover_context_` together under that mutex prevents the next attempt from observing a new severity with an old recovery requirement. `recovery_error_` is the per-attempt latch: a concurrent I/O error cannot be cleared as part of the in-flight attempt; it either stops recovery or forces another attempt to consume the updated context. If another error arrives while `ResumeImpl()` releases the mutex again, the same handoff repeats. No second recovery coordinator is needed. Manual `Resume()` now uses the retained recovery context instead of reconstructing one from the background-work scheduling flag, and the soft-error clear-only shortcut is allowed only when that flag shows there is no pending recovery work. The write ordering and `assert(seq_per_batch_)` detector are unchanged. Reviewed By: anand1976, xingbowang Differential Revision: D116106177 fbshipit-source-id: a78450c0924a326b5302ae6709e367f07bd3d5a6 |
||
Changyu Bi
|
cc30226b1f |
Deflake unit test DBErrorHandlingFSTest.AtomicFlushNoSpaceError (#13234)
Summary:
`DBErrorHandlingFSTest.AtomicFlushNoSpaceError` is flaky due to seg fault during error recovery:
```
...
frame https://github.com/facebook/rocksdb/issues/5: 0x00007f0b3ea0a9d6 librocksdb.so.9.10`rocksdb::VersionSet::GetObsoleteFiles(std::vector<rocksdb::ObsoleteFileInfo, std::allocator<rocksdb::ObsoleteFileInfo>>*, std::vector<rocksdb::ObsoleteBlobFileInfo, std::allocator<rocksdb::ObsoleteBlobFileInfo>>*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>*, unsigned long) [inlined] std::vector<rocksdb::ObsoleteFileInfo, std::allocator<rocksdb::ObsoleteFileInfo>>::begin(this=<unavailable>) at stl_vector.h:812:16
frame https://github.com/facebook/rocksdb/issues/6: 0x00007f0b3ea0a9d6 librocksdb.so.9.10`rocksdb::VersionSet::GetObsoleteFiles(this=0x0000000000000000, files=size=0, blob_files=size=0, manifest_filenames=size=0, min_pending_output=18446744073709551615) at version_set.cc:7258:18
frame https://github.com/facebook/rocksdb/issues/7: 0x00007f0b3e8ccbc0 librocksdb.so.9.10`rocksdb::DBImpl::FindObsoleteFiles(this=<unavailable>, job_context=<unavailable>, force=<unavailable>, no_full_scan=<unavailable>) at db_impl_files.cc:162:30
frame https://github.com/facebook/rocksdb/issues/8: 0x00007f0b3e85e698 librocksdb.so.9.10`rocksdb::DBImpl::ResumeImpl(this=<unavailable>, context=<unavailable>) at db_impl.cc:434:20
frame https://github.com/facebook/rocksdb/issues/9: 0x00007f0b3e921516 librocksdb.so.9.10`rocksdb::ErrorHandler::RecoverFromBGError(this=<unavailable>, is_manual=<unavailable>) at error_handler.cc:632:46
```
I suspect this is due to DB being destructed and reopened during recovery. Specifically, the [ClearBGError() call](
|
||
|
Changyu Bi
|
f97e33454f |
Fix a bug with auto recovery on WAL write error (#12995)
Summary:
A recent crash test failure shows that auto recovery from WAL write failure can cause CFs to be inconsistent. A unit test repro in P1569398553. The following is an example sequence of events:
```
0. manual_wal_flush is true. There are multiple CFs in a DB.
1. Submit a write batch with updates to multiple CF
2. A FlushWAL or a memtable swtich that will try to write the buffered WAL data. Fail this write so that buffered WAL data is dropped:
|
||
Changyu Bi
|
a31fe52173 |
Remove the return value of SetBGError() (#12792)
Summary: the return value for `ErrorHandler::SetBGError(error)` seems to be not well-defined, it can be `bg_error_` (no matter if the `bg_error_` is set to the input error), ok status or [`recovery_error_`]( |
||
Yu Zhang
|
ef342246dc |
Consolidate stats recording in error handler (#11992)
Summary: This is a non functional refactor, mostly for deduplicating the stats recording logic in error handler. Plus some documentation update and simple code dedupe. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11992 Test Plan: existing tests Reviewed By: hx235 Differential Revision: D52967713 Pulled By: jowlyzhang fbshipit-source-id: d584eae1a06410438f5a4c59c2cb67666ea7de1a |
||
|
Yu Zhang
|
509947ce2c |
Quarantine files in a limbo state after a manifest error (#12030)
Summary: Part of the procedures to handle manifest IO error is to disable file deletion in case some files in limbo state get deleted prematurely. This is not ideal because: 1) not all the VersionEdits whose commit encounter such an error contain updates for files, disabling file deletion sometimes are not necessary. 2) `EnableFileDeletion` has a force mode that could make other threads accidentally disrupt this procedure in recovery. 3) Disabling file deletion as a whole is also not as efficient as more precisely tracking impacted files from being prematurely deleted. This PR replaces this mechanism with tracking such files and quarantine them from being deleted in `ErrorHandler`. These are the types of files being actively tracked in quarantine in this PR: 1) new table files and blob files from a background job 2) old manifest file whose immediately following new manifest file's CURRENT file creation gets into unclear state. Current handling is not sufficient to make sure the old manifest file is kept in case it's needed. Note that WAL logs are not part of the quarantine because `min_log_number_to_keep` is a safe mechanism and it's only updated after successful manifest commits so it can prevent this premature deletion issue from happening. We track these files' file numbers because they share the same file number space. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12030 Test Plan: Modified existing unit tests Reviewed By: ajkr Differential Revision: D51036774 Pulled By: jowlyzhang fbshipit-source-id: 84ef26271fbbc888ef70da5c40fe843bd7038716 |
||
|
Yu Zhang
|
933ee295f4 |
Fix a race condition between recovery and backup (#11955)
Summary: A race condition between recovery and backup can happen with error messages like this: ```Failure in BackupEngine::CreateNewBackup with: IO error: No such file or directory: While opening a file for sequentially reading: /dev/shm/rocksdb_test/rocksdb_crashtest_whitebox/002653.log: No such file or directory``` PR https://github.com/facebook/rocksdb/issues/6949 introduced disabling file deletion during error handling of manifest IO errors. Aformentioned race condition is caused by this chain of event: [Backup engine] disable file deletion [Recovery] disable file deletion <= this is optional for the race condition, it may or may not get called [Backup engine] get list of file to copy/link [Recovery] force enable file deletion .... some files refered by backup engine get deleted [Backup engine] copy/link file <= error no file found This PR fixes this with: 1) Recovery thread is currently forcing enabling file deletion as long as file deletion is disabled. Regardless of whether the previous error handling is for manifest IO error and that disabled it in the first place. This means it could incorrectly enabling file deletions intended by other threads like backup threads, file snapshotting threads. This PR does this check explicitly before making the call. 2) `disable_delete_obsolete_files_` is designed as a counter to allow different threads to enable and disable file deletion separately. The recovery thread currently does a force enable file deletion, because `ErrorHandler::SetBGError()` can be called multiple times by different threads when they receive a manifest IO error(details per PR https://github.com/facebook/rocksdb/issues/6949), resulting in `DBImpl::DisableFileDeletions` to be called multiple times too. Making a force enable file deletion call that resets the counter `disable_delete_obsolete_files_` to zero is a workaround for this. However, as it shows in the race condition, it can incorrectly suppress other threads like a backup thread's intention to keep the file deletion disabled. <strike>This PR adds a `std::atomic<int> disable_file_deletion_count_` to the error handler to track the needed counter decrease more precisely</strike>. This PR tracks and caps file deletion enabling/disabling in error handler. 3) for recovery, the section to find obsolete files and purge them was moved to be done after the attempt to enable file deletion. The actual finding and purging is more likely to happen if file deletion was previously disabled and get re-enabled now. An internal function `DBImpl::EnableFileDeletionsWithLock` was added to support change 2) and 3). Some useful logging was explicitly added to keep those log messages around. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11955 Test Plan: existing unit tests Reviewed By: anand1976 Differential Revision: D50290592 Pulled By: jowlyzhang fbshipit-source-id: 73aa8331ca4d636955a5b0324b1e104a26e00c9b |
||
Andrew Kryczka
|
77d160ef47 |
Consolidate ErrorHandler's recovery status variables (#11937)
Summary: cbi42 pointed out a race condition in which `recovery_io_error_` and `recovery_error_` could be updated inconsistently due to releasing the DB mutex in `EventHelpers::NotifyOnBackgroundError()`. There doesn't seem to be a point to having two status objects, so this PR consolidates them. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11937 Reviewed By: cbi42 Differential Revision: D50105793 Pulled By: ajkr fbshipit-source-id: 3de95baccfa44351a49a5c2aa0986c9bc81baa8f |
||
|
Changyu Bi
|
1c871a4d86 |
Only flush after recovery for retryable IOError (#11880)
Summary: https://github.com/facebook/rocksdb/issues/11872 causes a unit test to start failing with the error message below. The cause is that the additional call to `FlushAllColumnFamilies()` in `DBImpl::ResumeImpl()` can run while DB is closing. More detailed explanation: there are two places where we call `ResumeImpl()`: 1. in `ErrorHandler::RecoverFromBGError`, for manual resume or recovery from errors like OutOfSpace through sst file manager, and 2. in `Errorhandler::RecoverFromRetryableBGIOError`, for error recovery from errors like flush failure due to retryable IOError. This is tracked by `ErrorHandler::recovery_thread_`. Here is how DB close waits for error recovery: |
||
Andrew Kryczka
|
5cf6ab6f31 |
Ran clang-format on db/ directory (#10910)
Summary: Ran `find ./db/ -type f | xargs clang-format -i`. Excluded minor changes it tried to make on db/db_impl/. Everything else it changed was directly under db/ directory. Included minor manual touchups mentioned in PR commit history. Pull Request resolved: https://github.com/facebook/rocksdb/pull/10910 Reviewed By: riversand963 Differential Revision: D40880683 Pulled By: ajkr fbshipit-source-id: cfe26cda05b3fb9a72e3cb82c286e21d8c5c4174 |
||
Wallace
|
1e9bf25f61 |
Do not hold mutex when write keys if not necessary (#7516)
Summary: ## Problem Summary RocksDB will acquire the global mutex of db instance for every time when user calls `Write`. When RocksDB schedules a lot of compaction jobs, it will compete the mutex with write thread and it will hurt the write performance. ## Problem Solution: I want to use log_write_mutex to replace the global mutex in most case so that we do not acquire it in write-thread unless there is a write-stall event or a write-buffer-full event occur. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7516 Test Plan: 1. make check 2. CI 3. COMPILE_WITH_TSAN=1 make db_stress make crash_test make crash_test_with_multiops_wp_txn make crash_test_with_multiops_wc_txn make crash_test_with_atomic_flush Reviewed By: siying Differential Revision: D36908702 Pulled By: riversand963 fbshipit-source-id: 59b13881f4f5c0a58fd3ca79128a396d9cd98efe |
||
anand76
|
a88d8795ec |
Expand auto recovery to background read errors (#9679)
Summary: Fix and enhance the background error recovery logic to handle the following situations - 1. Background read errors during flush/compaction (previously was resulting in unrecoverable state) 2. Fix auto recovery failure on read/write errors during atomic flush. It was failing due to a bug in setting the resuming_from_bg_err variable in AtomicFlushMemTablesToOutputFiles. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9679 Test Plan: Add new unit tests in error_handler_fs_test Reviewed By: riversand963 Differential Revision: D34770097 Pulled By: anand1976 fbshipit-source-id: 136da973a28d684b9c74bdf668519b0cbbbe1742 |
||
Zhichao Cao
|
efaef9b40a |
cleanup error_handler related code (#9098)
Summary: Remove code not in use, add comments, remove redundant code. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9098 Test Plan: make check Reviewed By: anand1976 Differential Revision: D32027219 Pulled By: zhichao-cao fbshipit-source-id: 253aae926c87726268af6c027bf805dc9156c8a8 |
||
storagezhang
|
711881bc25 |
Fix some typos in comments (#8066)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8066 Reviewed By: jay-zhuang Differential Revision: D27280799 Pulled By: mrambacher fbshipit-source-id: 68f91f5af4ffe0a84be581961bf9366887f47702 |
||
|
Zhichao Cao
|
08ec5e7321 |
Add the statistics and info log for Error handler (#8050)
Summary: Add statistics and info log for error handler: counters for bg error, bg io error, bg retryable io error, auto resume, auto resume total retry, and auto resume sucess; Histogram for auto resume retry count in each recovery call. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8050 Test Plan: make check and add test to error_handler_fs_test Reviewed By: anand1976 Differential Revision: D26990565 Pulled By: zhichao-cao fbshipit-source-id: 49f71e8ea4e9db8b189943976404205b56ab883f |
||
mrambacher
|
db03172d08 |
Change ErrorHandler methods to return const Status& (#7539)
Summary: This change eliminates the need for a lot of the PermitUncheckedError calls on return from ErrorHandler methods. The calls are no longer needed as the status is returned as a reference rather than a copy. Additionally, this means that the originating status (recovery_error_, bg_error_) is not cleared implicitly as a result of calling one of these methods. For this class, I do not know if the proper behavior should be to call PermitUncheckedError in the destructor or if the checked state should be cleared when the status is cleared. I did tests both ways. Without the code in the destructor, the status will need to be cleared in at least some of the places where it is set to OK. When running tests, I found no instances where this class was destructed with a non-OK, non-checked Status. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7539 Reviewed By: anand1976 Differential Revision: D25340565 Pulled By: pdillinger fbshipit-source-id: 1730c035c81a475875ea745226112030ec25136c |
||
|
Zhichao Cao
|
c268628c25 |
Map retryable IO error during Flush without WAL to soft error and no switch memtable during resume (#7310)
Summary: In the current implementation, any retryable IO error happens during Flush is mapped to a hard error. In this case, DB is stopped and write is stalled unless the background error is cleaned. In this PR, if WAL is DISABLED, the retryable IO error during FLush is mapped to a soft error. Such that, the memtable can continue receive the writes. At the same time, if auto resume is triggered, SwtichMemtable will not be called during Flush when resuming the DB to avoid to many small memtables. Testing cases are added. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7310 Test Plan: adding new unit test, pass make check. Reviewed By: anand1976 Differential Revision: D23710892 Pulled By: zhichao-cao fbshipit-source-id: bc4ca50d11c6b23b60d2c0cb171d86d542b038e9 |
||
|
mrambacher
|
e9befdebbf |
Add EnvTestWithParam::OptionsTest to the ASSERT_STATUS_CHECKED passes (#7283)
Summary: This test uses database functionality and required more extensive work to get it to pass than the other tests. The DB functionality required for this test now passes the check. When it was unclear what the proper behavior was for unchecked status codes, a TODO was added. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7283 Reviewed By: akankshamahajan15 Differential Revision: D23251497 Pulled By: ajkr fbshipit-source-id: 52b79629bdafa0a58de8ead1d1d66f141b331523 |
||
|
Zhichao Cao
|
a10f12eda1 |
Auto resume the DB from Retryable IO Error (#6765)
Summary: In current codebase, in write path, if Retryable IO Error happens, SetBGError is called. The retryable IO Error is converted to hard error and DB is in read only mode. User or application needs to resume it. In this PR, if Retryable IO Error happens in one DB, SetBGError will create a new thread to call Resume (auto resume). otpions.max_bgerror_resume_count controls if auto resume is enabled or not (if max_bgerror_resume_count<=0, auto resume will not be enabled). options.bgerror_resume_retry_interval controls the time interval to call Resume again if the previous resume fails due to the Retryable IO Error. If non-retryable error happens during resume, auto resume will terminate. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6765 Test Plan: Added the unit test cases in error_handler_fs_test and pass make asan_check Reviewed By: anand1976 Differential Revision: D21916789 Pulled By: zhichao-cao fbshipit-source-id: acb8b5e5dc3167adfa9425a5b7fc104f6b95cb0b |
||
|
Zhichao Cao
|
4246888101 |
Pass IOStatus to write path and set retryable IO Error as hard error in BG jobs (#6487)
Summary: In the current code base, we use Status to get and store the returned status from the call. Specifically, for IO related functions, the current Status cannot reflect the IO Error details such as error scope, error retryable attribute, and others. With the implementation of https://github.com/facebook/rocksdb/issues/5761, we have the new Wrapper for IO, which returns IOStatus instead of Status. However, the IOStatus is purged at the lower level of write path and transferred to Status. The first job of this PR is to pass the IOStatus to the write path (flush, WAL write, and Compaction). The second job is to identify the Retryable IO Error as HardError, and set the bg_error_ as HardError. In this case, the DB Instance becomes read only. User is informed of the Status and need to take actions to deal with it (e.g., call db->Resume()). Pull Request resolved: https://github.com/facebook/rocksdb/pull/6487 Test Plan: Added the testing case to error_handler_fs_test. Pass make asan_check Reviewed By: anand1976 Differential Revision: D20685017 Pulled By: zhichao-cao fbshipit-source-id: ff85f042896243abcd6ef37877834e26f36b6eb0 |
||
sdong
|
fdf882ded2 |
Replace namespace name "rocksdb" with ROCKSDB_NAMESPACE (#6433)
Summary: When dynamically linking two binaries together, different builds of RocksDB from two sources might cause errors. To provide a tool for user to solve the problem, the RocksDB namespace is changed to a flag which can be overridden in build time. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6433 Test Plan: Build release, all and jtest. Try to build with ROCKSDB_NAMESPACE with another flag. Differential Revision: D19977691 fbshipit-source-id: aa7f2d0972e1c31d75339ac48478f34f6cfcfb3e |
||
Dmitry Alimov
|
e13d8dcbbb |
Fix typos in comments (#4456)
Summary: Fix some typos in the comments Pull Request resolved: https://github.com/facebook/rocksdb/pull/4456 Differential Revision: D10209214 Pulled By: miasantreble fbshipit-source-id: dff857ba60396bc95126e635db96d7dc8330d2cb |
||
Anand Ananthabhotla
|
a27fce408e |
Auto recovery from out of space errors (#4164)
Summary: This commit implements automatic recovery from a Status::NoSpace() error during background operations such as write callback, flush and compaction. The broad design is as follows - 1. Compaction errors are treated as soft errors and don't put the database in read-only mode. A compaction is delayed until enough free disk space is available to accomodate the compaction outputs, which is estimated based on the input size. This means that users can continue to write, and we rely on the WriteController to delay or stop writes if the compaction debt becomes too high due to persistent low disk space condition 2. Errors during write callback and flush are treated as hard errors, i.e the database is put in read-only mode and goes back to read-write only fater certain recovery actions are taken. 3. Both types of recovery rely on the SstFileManagerImpl to poll for sufficient disk space. We assume that there is a 1-1 mapping between an SFM and the underlying OS storage container. For cases where multiple DBs are hosted on a single storage container, the user is expected to allocate a single SFM instance and use the same one for all the DBs. If no SFM is specified by the user, DBImpl::Open() will allocate one, but this will be one per DB and each DB will recover independently. The recovery implemented by SFM is as follows - a) On the first occurance of an out of space error during compaction, subsequent compactions will be delayed until the disk free space check indicates enough available space. The required space is computed as the sum of input sizes. b) The free space check requirement will be removed once the amount of free space is greater than the size reserved by in progress compactions when the first error occured c) If the out of space error is a hard error, a background thread in SFM will poll for sufficient headroom before triggering the recovery of the database and putting it in write-only mode. The headroom is calculated as the sum of the write_buffer_size of all the DB instances associated with the SFM 4. EventListener callbacks will be called at the start and completion of automatic recovery. Users can disable the auto recov ery in the start callback, and later initiate it manually by calling DB::Resume() Todo: 1. More extensive testing 2. Add disk full condition to db_stress (follow-on PR) Pull Request resolved: https://github.com/facebook/rocksdb/pull/4164 Differential Revision: D9846378 Pulled By: anand1976 fbshipit-source-id: 80ea875dbd7f00205e19c82215ff6e37da10da4a |
||
|
Anand Ananthabhotla
|
52d4c9b7f6 |
Allow DB resume after background errors (#3997)
Summary: Currently, if RocksDB encounters errors during a write operation (user requested or BG operations), it sets DBImpl::bg_error_ and fails subsequent writes. This PR allows the DB to be resumed for certain classes of errors. It consists of 3 parts - 1. Introduce Status::Severity in rocksdb::Status to indicate whether a given error can be recovered from or not 2. Refactor the error handling code so that setting bg_error_ and deciding on severity is in one place 3. Provide an API for the user to clear the error and resume the DB instance This whole change is broken up into multiple PRs. Initially, we only allow clearing the error for Status::NoSpace() errors during background flush/compaction. Subsequent PRs will expand this to include more errors and foreground operations such as Put(), and implement a polling mechanism for out-of-space errors. Closes https://github.com/facebook/rocksdb/pull/3997 Differential Revision: D8653831 Pulled By: anand1976 fbshipit-source-id: 6dc835c76122443a7668497c0226b4f072bc6afd |