Anand Ananthabhotla 074134d98f Stop writes on retryable background IO errors when auto resume is disabled (#15444) ...

Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/15444

**Why the change is required**

`max_bgerror_resume_count <= 0` turns off automatic `DB::Resume()`, so the application is responsible for recovering from background errors. Before automatic recovery was introduced, a background error under `paranoid_checks` latched `bg_error_` and made the DB read-only. Since then, two classes of retryable IO error are either never latched or latched as soft errors that leave the DB accepting writes. When auto resume is disabled, no one recovers from them and the application gets no clear signal.

**Previous behavior** (with `max_bgerror_resume_count <= 0`)

- A retryable or file-scoped IO error during compaction was never set in `bg_error_`. Compaction rescheduled itself and retried indefinitely. A persistent failure was visible only through `EventListener::OnBackgroundError` and the LOG, while compaction debt grew until writes slowed or stopped on L0/pending-compaction triggers.
- A retryable or file-scoped IO error while writing a flush SST or the MANIFEST after a successful WAL sync (`kFlushNoWAL` / `kManifestWriteNoWAL`, which covers almost every retryable flush failure, not only `disableWAL` writes) was a soft error with `soft_error_no_bg_work_` set. Background work stopped, but writes kept succeeding into the memtable. Nothing retried the flush, so writers eventually blocked indefinitely on the memtable limit instead of failing.

**New behavior**

With `max_bgerror_resume_count <= 0`, every retryable or file-scoped background IO error that reaches the retryable path in `ErrorHandler::SetBGErrorImpl` is latched as `kHardError`, whatever the reason (compaction, flush, MANIFEST, WAL). Writes fail immediately and reads continue to work. `DB::Resume()` clears the error, flushes and reschedules compaction as before.

**Implementation**

In `SetBGErrorImpl`, the compaction special case (notify the listener and return without setting `bg_error_`) now applies only when auto resume is enabled, and the severity selection maps everything to `kHardError` when auto resume is disabled. Because `kErrorRecoveryRetryFlush` is no longer selected in that mode, `soft_error_no_bg_work_` is not set. The option documentation in `include/rocksdb/options.h` and the comment above `SetBGError` are updated.

**Safety, compatibility, and limitations**

- This is an intentional behavior change, limited to DBs that set `max_bgerror_resume_count <= 0`. With the default (`INT_MAX`) or any positive value, nothing changes.
- fbcode callers that set the option to 0, and their apparent expectations:
  - `unicorn/live_updates/dispatcher/MessageStore.cpp`: the comment says "we do not allow any [background errors] as they should be retried by the underlying FS". Stopping on retryable compaction errors fits that intent better than silent retries.
  - `reliable_volumes/client/index_manager/RocksDBIndexStorage.cpp`: a JustKnob (`disableRocksDbBackgroundErrorAutoResume`) disables auto resume alongside other corruption-detection options, which suggests it wants background errors surfaced rather than masked.
  - `zippydb/server/store` (`RocksDBStoreResumeWritesTest`): uses 0 with `disableWAL` and drives recovery through its own `ResumeWrites` task calling `DB::Resume()`, so it already expects writes to stop until resume. I read the test and expect it to pass, but did not run it. No production zippydb configuration setting 0 was found.
- Out-of-space errors (`kNoSpace`/`kSpaceLimit`) are untouched: they skip this path and are still recovered through `SstFileManager`, which does not consult `max_bgerror_resume_count`. Errors ignored under `paranoid_checks=false` are also unchanged.
- No API, ABI or on-disk format change.
- Found during testing, pre-existing and not fixed here: `SetBGErrorImpl` releases the DB mutex in `NotifyOnBackgroundError` before calling `StartRecoverFromRetryableBGIOError`. If the last failed auto-resume attempt's flush error is reported in that window, the recovery thread can give up and clear `recovery_in_prog_` first, and the late report then starts a new recovery with a fresh retry budget, so `max_bgerror_resume_count` can be exceeded. This only applies with auto resume enabled. Worth a separate fix.

Reviewed By: joshkang97

Differential Revision: D126453053

fbshipit-source-id: 3f4be1d12a8d44552313b022db5afe200bc1c6ce

2026-10-20 20:21:11 -07:00

168 KiB

Raw Permalink Blame History

View Raw