mirror of
https://github.com/facebook/rocksdb.git
synced 2026-10-23 08:09:02 +03:00
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
This commit is contained in:
committed by
meta-codesync[bot]
parent
744f120b12
commit
f56112abb7
@@ -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();
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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() ||
|
||||
|
||||
Reference in New Issue
Block a user