From f56112abb7efc825a2ce4febe592447bf04c04b6 Mon Sep 17 00:00:00 2001 From: Anusha Mukka Date: Thu, 23 Jul 2026 18:43:39 -0700 Subject: [PATCH] Plumb Env::IOActivity into DBImpl::ResumeImpl (#14952) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14952 `DBImpl::ResumeImpl()` constructed its local `ReadOptions`/`WriteOptions` with defaults and carried a `// TODO: plumb Env::IOActivity, Env::IOPriority` comment. Those options flow into `VersionSet::LogAndApply()` (the MANIFEST write during error recovery), so the rate limiter could not attribute or prioritize that I/O correctly. This resolves the TODO for `ResumeImpl` (1 of 91 identical TODOs across the codebase) by accepting an `Env::IOActivity` parameter and constructing the options with it, following the pattern already established in `Get()`, `MultiGet()`, and `NewIterator()`, and in the `kDBOpen` paths in `db_impl_open.cc`. Error recovery is driven by flush operations (as the existing comment in `ErrorHandler::RecoverFromRetryableBGIOError` notes), so both production callers and the `TEST_ResumeImpl` hook pass `Env::IOActivity::kFlush`. Setting `io_activity` on these options is safe: `LogAndApply` does not assert `io_activity == kUnknown`. Reviewed By: pdillinger Differential Revision: D112537885 fbshipit-source-id: 59045fd2f0a6e5bbacef6e0381e03ff95cd28c51 --- db/db_impl/db_impl.cc | 8 ++++---- db/db_impl/db_impl.h | 5 ++++- db/db_impl/db_impl_debug.cc | 2 +- db/error_handler.cc | 4 ++-- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index f221e58945..9a9b82fbab 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -323,12 +323,12 @@ Status DBImpl::Resume() { // 4. Schedule compactions if needed for all the CFs. This is needed as the // flush in the prior step might have been a no-op for some CFs, which // means a new super version wouldn't have been installed -Status DBImpl::ResumeImpl(DBRecoverContext context) { +Status DBImpl::ResumeImpl(DBRecoverContext context, + Env::IOActivity io_activity) { mutex_.AssertHeld(); - // TODO: plumb Env::IOActivity, Env::IOPriority - const ReadOptions read_options; - const WriteOptions write_options; + const ReadOptions read_options(io_activity); + const WriteOptions write_options(io_activity); WaitForBackgroundWork(); diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index ae316cc3e4..a9d8435aac 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -2233,7 +2233,10 @@ class DBImpl : public DB { // Required: DB mutex held Status PersistentStatsProcessFormatVersion(); - Status ResumeImpl(DBRecoverContext context); + // `io_activity` is used to construct the ReadOptions/WriteOptions for the + // internal recovery work (e.g. the MANIFEST write in LogAndApply). Recovery + // is driven by flush operations, so callers pass Env::IOActivity::kFlush. + Status ResumeImpl(DBRecoverContext context, Env::IOActivity io_activity); void MaybeIgnoreError(Status* s) const; diff --git a/db/db_impl/db_impl_debug.cc b/db/db_impl/db_impl_debug.cc index b13d3251af..2ab541063e 100644 --- a/db/db_impl/db_impl_debug.cc +++ b/db/db_impl/db_impl_debug.cc @@ -37,7 +37,7 @@ Status DBImpl::TEST_SwitchWAL() { Status DBImpl::TEST_ResumeImpl(DBRecoverContext context) { InstrumentedMutexLock l(&mutex_); - return ResumeImpl(context); + return ResumeImpl(context, Env::IOActivity::kFlush); } uint64_t DBImpl::TEST_MaxNextLevelOverlappingBytes( diff --git a/db/error_handler.cc b/db/error_handler.cc index e15de116cb..9138bdb7be 100644 --- a/db/error_handler.cc +++ b/db/error_handler.cc @@ -668,7 +668,7 @@ Status ErrorHandler::RecoverFromBGError(bool is_manual) { // can generate background errors should be the flush operations recovery_error_ = IOStatus::OK(); recovery_error_.PermitUncheckedError(); - Status s = db_->ResumeImpl(recover_context_); + Status s = db_->ResumeImpl(recover_context_, Env::IOActivity::kFlush); if (s.ok()) { soft_error_no_bg_work_ = false; } else { @@ -764,7 +764,7 @@ void ErrorHandler::RecoverFromRetryableBGIOError() { TEST_SYNC_POINT("RecoverFromRetryableBGIOError:BeforeResume1"); recovery_error_ = IOStatus::OK(); retry_count++; - Status s = db_->ResumeImpl(context); + Status s = db_->ResumeImpl(context, Env::IOActivity::kFlush); RecordStats({ERROR_HANDLER_AUTORESUME_RETRY_TOTAL_COUNT}, {} /* int_histograms */); if (s.IsShutdownInProgress() ||