diff --git a/db/error_handler.cc b/db/error_handler.cc index d962208f34..c32deabdf6 100644 --- a/db/error_handler.cc +++ b/db/error_handler.cc @@ -413,6 +413,8 @@ void ErrorHandler::HandleKnownErrors(const Status& bg_err, // to a soft error. Note that, it includes the case that IO error happens // in SST or manifest write during flush. Auto resume will be called. // c) all other errors are mapped to hard error. Auto resume will be called. +// If auto resume is disabled (max_bgerror_resume_count <= 0), all of the +// above are mapped to hard error and wait for a manual DB::Resume(). // 3) for other cases, HandleKnownErrors(const Status& bg_err, // BackgroundErrorReason reason) will be called to handle other error cases // such as delegating to SstFileManager to handle no space error. @@ -523,7 +525,8 @@ void ErrorHandler::SetBGErrorImpl(const Status& bg_status, {} /* int_histograms */); ROCKS_LOG_INFO(db_options_.info_log, "ErrorHandler: Set background retryable IO error\n"); - if (BackgroundErrorReason::kCompaction == reason) { + const bool auto_resume_enabled = db_options_.max_bgerror_resume_count > 0; + if (BackgroundErrorReason::kCompaction == reason && auto_resume_enabled) { // We map the retryable IO error during compaction to soft error. Since // compaction can reschedule by itself. We will not set the BG error in // this case @@ -543,8 +546,14 @@ void ErrorHandler::SetBGErrorImpl(const Status& bg_status, } Status::Severity severity; - if (BackgroundErrorReason::kFlushNoWAL == reason || - BackgroundErrorReason::kManifestWriteNoWAL == reason) { + if (!auto_resume_enabled) { + // Without auto resume, nothing retries or recovers from the error on the + // user's behalf, so stop writes until the user calls DB::Resume(). This + // includes compaction and no-WAL flush errors, which would otherwise be + // retried by compaction or tolerated as soft errors. + severity = Status::Severity::kHardError; + } else if (BackgroundErrorReason::kFlushNoWAL == reason || + BackgroundErrorReason::kManifestWriteNoWAL == reason) { // When the BG Retryable IO error reason is flush without WAL, // We map it to a soft error. At the same time, all the background work // should be stopped except the BG work from recovery. Therefore, we @@ -569,7 +578,7 @@ void ErrorHandler::SetBGErrorImpl(const Status& bg_status, return; } CheckAndSetRecoveryAndBGError(bg_err, context); - bool auto_recovery = db_options_.max_bgerror_resume_count > 0; + bool auto_recovery = auto_resume_enabled; EventHelpers::NotifyOnBackgroundError(db_options_.listeners, reason, &new_bg_io_err, db_mutex_, &auto_recovery); diff --git a/db/error_handler_fs_test.cc b/db/error_handler_fs_test.cc index 248f743db5..a4cc62fa60 100644 --- a/db/error_handler_fs_test.cc +++ b/db/error_handler_fs_test.cc @@ -59,6 +59,34 @@ class DBErrorHandlingFSTest : public DBTestBase { return ""; } + // Holds automatic recovery from starting until ActivateFSAndRecover(), so + // the first auto resume attempt does not run against the failed FS. + void HoldAutoRecovery() { + SyncPoint::GetInstance()->LoadDependency( + {{"DBErrorHandlingFSTest::ReleaseAutoRecovery", + "RecoverFromRetryableBGIOError:BeforeStart"}, + {"RecoverFromRetryableBGIOError:RecoverSuccess", + "DBErrorHandlingFSTest::AutoRecoveryDone"}}); + } + + // Re-activates the FS and recovers from the background error: with a manual + // Resume() if auto resume is disabled, otherwise by waiting for the auto + // recovery held by HoldAutoRecovery() to succeed. The recovery thread is + // waited on directly because OnErrorRecoveryEnd() fires before the thread + // finishes, and a later shutdown would notify again. + void ActivateFSAndRecover() { + SyncPoint::GetInstance()->ClearAllCallBacks(); + fault_fs_->SetFilesystemActive(true); + TEST_SYNC_POINT("DBErrorHandlingFSTest::ReleaseAutoRecovery"); + if (dbfull()->GetDBOptions().max_bgerror_resume_count > 0) { + TEST_SYNC_POINT("DBErrorHandlingFSTest::AutoRecoveryDone"); + } else { + ASSERT_OK(dbfull()->Resume()); + } + ASSERT_OK(dbfull()->TEST_GetBGError()); + SyncPoint::GetInstance()->DisableProcessing(); + } + std::shared_ptr fault_fs_; std::unique_ptr fault_env_; }; @@ -278,7 +306,7 @@ TEST_F(DBErrorHandlingFSTest, FlushWriteRetryableError) { [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); + ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kHardError); SyncPoint::GetInstance()->DisableProcessing(); fault_fs_->SetFilesystemActive(true); s = dbfull()->Resume(); @@ -304,7 +332,7 @@ TEST_F(DBErrorHandlingFSTest, FlushWriteRetryableError) { [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); + ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kHardError); SyncPoint::GetInstance()->DisableProcessing(); fault_fs_->SetFilesystemActive(true); s = dbfull()->Resume(); @@ -318,7 +346,7 @@ TEST_F(DBErrorHandlingFSTest, FlushWriteRetryableError) { [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); + ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kHardError); SyncPoint::GetInstance()->DisableProcessing(); fault_fs_->SetFilesystemActive(true); s = dbfull()->Resume(); @@ -329,88 +357,60 @@ TEST_F(DBErrorHandlingFSTest, FlushWriteRetryableError) { Destroy(options); } +// Without auto resume, the error stops writes until a manual Resume(). With +// auto resume, it is a soft error cleared by automatic recovery. TEST_F(DBErrorHandlingFSTest, FlushWriteFileScopeError) { - std::shared_ptr listener = - std::make_shared(); - Options options = GetDefaultOptions(); - options.env = fault_env_.get(); - options.create_if_missing = true; - options.listeners.emplace_back(listener); - options.max_bgerror_resume_count = 0; - Status s; + for (int resume_count : {0, 2}) { + SCOPED_TRACE("max_bgerror_resume_count=" + std::to_string(resume_count)); + std::shared_ptr listener = + std::make_shared(); + Options options = GetDefaultOptions(); + options.env = fault_env_.get(); + options.create_if_missing = true; + options.listeners.emplace_back(listener); + options.max_bgerror_resume_count = resume_count; + const Status::Severity expected_severity = + resume_count > 0 ? Status::Severity::kSoftError + : Status::Severity::kHardError; - listener->EnableAutoRecovery(false); - DestroyAndReopen(options); + listener->EnableAutoRecovery(false); + DestroyAndReopen(options); - IOStatus error_msg = IOStatus::IOError("File Scope Data Loss Error"); - error_msg.SetDataLoss(true); - error_msg.SetScope( - ROCKSDB_NAMESPACE::IOStatus::IOErrorScope::kIOErrorScopeFile); - error_msg.SetRetryable(false); + IOStatus error_msg = IOStatus::IOError("File Scope Data Loss Error"); + error_msg.SetDataLoss(true); + error_msg.SetScope(IOStatus::IOErrorScope::kIOErrorScopeFile); + error_msg.SetRetryable(false); - ASSERT_OK(Put(Key(1), "val1")); - SyncPoint::GetInstance()->SetCallBack( - "BuildTable:BeforeFinishBuildTable", - [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); - SyncPoint::GetInstance()->EnableProcessing(); - s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - SyncPoint::GetInstance()->DisableProcessing(); - fault_fs_->SetFilesystemActive(true); - s = dbfull()->Resume(); - ASSERT_OK(s); - Reopen(options); - ASSERT_EQ("val1", Get(Key(1))); + auto fail_flush_and_recover = [&](const char* sync_point, + const std::string& key) { + ASSERT_OK(Put(key, "val")); + HoldAutoRecovery(); + SyncPoint::GetInstance()->SetCallBack(sync_point, [&](void*) { + fault_fs_->SetFilesystemActive(false, error_msg); + }); + SyncPoint::GetInstance()->EnableProcessing(); + ASSERT_EQ(Flush().severity(), expected_severity); + ASSERT_NO_FATAL_FAILURE(ActivateFSAndRecover()); + Reopen(options); + ASSERT_EQ("val", Get(key)); + }; - ASSERT_OK(Put(Key(2), "val2")); - SyncPoint::GetInstance()->SetCallBack( - "BuildTable:BeforeSyncTable", - [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); - SyncPoint::GetInstance()->EnableProcessing(); - s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - SyncPoint::GetInstance()->DisableProcessing(); - fault_fs_->SetFilesystemActive(true); - s = dbfull()->Resume(); - ASSERT_OK(s); - Reopen(options); - ASSERT_EQ("val2", Get(Key(2))); + ASSERT_NO_FATAL_FAILURE( + fail_flush_and_recover("BuildTable:BeforeFinishBuildTable", Key(1))); + ASSERT_NO_FATAL_FAILURE( + fail_flush_and_recover("BuildTable:BeforeSyncTable", Key(2))); + ASSERT_NO_FATAL_FAILURE( + fail_flush_and_recover("BuildTable:BeforeCloseTableFile", Key(3))); - ASSERT_OK(Put(Key(3), "val3")); - SyncPoint::GetInstance()->SetCallBack( - "BuildTable:BeforeCloseTableFile", - [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); - SyncPoint::GetInstance()->EnableProcessing(); - s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - SyncPoint::GetInstance()->DisableProcessing(); - fault_fs_->SetFilesystemActive(true); - s = dbfull()->Resume(); - ASSERT_OK(s); - Reopen(options); - ASSERT_EQ("val3", Get(Key(3))); + // not file scope, but retyrable set + error_msg.SetDataLoss(false); + error_msg.SetScope(IOStatus::IOErrorScope::kIOErrorScopeFileSystem); + error_msg.SetRetryable(true); + ASSERT_NO_FATAL_FAILURE( + fail_flush_and_recover("BuildTable:BeforeCloseTableFile", Key(4))); - // not file scope, but retyrable set - error_msg.SetDataLoss(false); - error_msg.SetScope( - ROCKSDB_NAMESPACE::IOStatus::IOErrorScope::kIOErrorScopeFileSystem); - error_msg.SetRetryable(true); - - ASSERT_OK(Put(Key(3), "val3")); - SyncPoint::GetInstance()->SetCallBack( - "BuildTable:BeforeCloseTableFile", - [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); - SyncPoint::GetInstance()->EnableProcessing(); - s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - SyncPoint::GetInstance()->DisableProcessing(); - fault_fs_->SetFilesystemActive(true); - s = dbfull()->Resume(); - ASSERT_OK(s); - Reopen(options); - ASSERT_EQ("val3", Get(Key(3))); - - Destroy(options); + Destroy(options); + } } TEST_F(DBErrorHandlingFSTest, FlushWALWriteRetryableError) { @@ -528,14 +528,15 @@ TEST_F(DBErrorHandlingFSTest, FlushWritNoWALRetryableError1) { [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - ASSERT_OK(Put(Key(2), "val2", wo)); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - ASSERT_EQ("val2", Get(Key(2))); + // Without auto resume, the error stops writes until a manual Resume() + ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kHardError); + ASSERT_NOK(Put(Key(2), "val2", wo)); SyncPoint::GetInstance()->DisableProcessing(); fault_fs_->SetFilesystemActive(true); s = dbfull()->Resume(); ASSERT_OK(s); ASSERT_EQ("val1", Get(Key(1))); + ASSERT_OK(Put(Key(2), "val2", wo)); ASSERT_EQ("val2", Get(Key(2))); ASSERT_OK(Put(Key(3), "val3", wo)); ASSERT_EQ("val3", Get(Key(3))); @@ -584,14 +585,15 @@ TEST_F(DBErrorHandlingFSTest, FlushWriteNoWALRetryableError2) { [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - ASSERT_OK(Put(Key(2), "val2", wo)); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - ASSERT_EQ("val2", Get(Key(2))); + // Without auto resume, the error stops writes until a manual Resume() + ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kHardError); + ASSERT_NOK(Put(Key(2), "val2", wo)); SyncPoint::GetInstance()->DisableProcessing(); fault_fs_->SetFilesystemActive(true); s = dbfull()->Resume(); ASSERT_OK(s); ASSERT_EQ("val1", Get(Key(1))); + ASSERT_OK(Put(Key(2), "val2", wo)); ASSERT_EQ("val2", Get(Key(2))); ASSERT_OK(Put(Key(3), "val3", wo)); ASSERT_EQ("val3", Get(Key(3))); @@ -628,14 +630,15 @@ TEST_F(DBErrorHandlingFSTest, FlushWriteNoWALRetryableError3) { [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - ASSERT_OK(Put(Key(2), "val2", wo)); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - ASSERT_EQ("val2", Get(Key(2))); + // Without auto resume, the error stops writes until a manual Resume() + ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kHardError); + ASSERT_NOK(Put(Key(2), "val2", wo)); SyncPoint::GetInstance()->DisableProcessing(); fault_fs_->SetFilesystemActive(true); s = dbfull()->Resume(); ASSERT_OK(s); ASSERT_EQ("val1", Get(Key(1))); + ASSERT_OK(Put(Key(2), "val2", wo)); ASSERT_EQ("val2", Get(Key(2))); ASSERT_OK(Put(Key(3), "val3", wo)); ASSERT_EQ("val3", Get(Key(3))); @@ -716,7 +719,7 @@ TEST_F(DBErrorHandlingFSTest, ManifestWriteRetryableError) { [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); + ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kHardError); ASSERT_FALSE(dbfull()->TEST_GetFilesToQuarantine().empty()); SyncPoint::GetInstance()->ClearAllCallBacks(); SyncPoint::GetInstance()->DisableProcessing(); @@ -734,52 +737,52 @@ TEST_F(DBErrorHandlingFSTest, ManifestWriteRetryableError) { Close(); } +// Without auto resume, the error stops writes until a manual Resume(). With +// auto resume, it is a soft error cleared by automatic recovery. TEST_F(DBErrorHandlingFSTest, ManifestWriteFileScopeError) { - std::shared_ptr listener = - std::make_shared(); - Options options = GetDefaultOptions(); - options.env = fault_env_.get(); - options.create_if_missing = true; - options.listeners.emplace_back(listener); - options.max_bgerror_resume_count = 0; - Status s; - std::string old_manifest; - std::string new_manifest; + for (int resume_count : {0, 2}) { + SCOPED_TRACE("max_bgerror_resume_count=" + std::to_string(resume_count)); + std::shared_ptr listener = + std::make_shared(); + Options options = GetDefaultOptions(); + options.env = fault_env_.get(); + options.create_if_missing = true; + options.listeners.emplace_back(listener); + options.max_bgerror_resume_count = resume_count; + const Status::Severity expected_severity = + resume_count > 0 ? Status::Severity::kSoftError + : Status::Severity::kHardError; - listener->EnableAutoRecovery(false); - DestroyAndReopen(options); - old_manifest = GetManifestNameFromLiveFiles(); + listener->EnableAutoRecovery(false); + DestroyAndReopen(options); + const std::string old_manifest = GetManifestNameFromLiveFiles(); - IOStatus error_msg = IOStatus::IOError("File Scope Data Loss Error"); - error_msg.SetDataLoss(true); - error_msg.SetScope( - ROCKSDB_NAMESPACE::IOStatus::IOErrorScope::kIOErrorScopeFile); - error_msg.SetRetryable(false); + IOStatus error_msg = IOStatus::IOError("File Scope Data Loss Error"); + error_msg.SetDataLoss(true); + error_msg.SetScope(IOStatus::IOErrorScope::kIOErrorScopeFile); + error_msg.SetRetryable(false); - ASSERT_OK(Put(Key(0), "val")); - ASSERT_OK(Flush()); - ASSERT_OK(Put(Key(1), "val")); - SyncPoint::GetInstance()->SetCallBack( - "VersionSet::LogAndApply:WriteManifest", - [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); - SyncPoint::GetInstance()->EnableProcessing(); - s = Flush(); - ASSERT_FALSE(dbfull()->TEST_GetFilesToQuarantine().empty()); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - SyncPoint::GetInstance()->ClearAllCallBacks(); - SyncPoint::GetInstance()->DisableProcessing(); - fault_fs_->SetFilesystemActive(true); - s = dbfull()->Resume(); - ASSERT_OK(s); + ASSERT_OK(Put(Key(0), "val")); + ASSERT_OK(Flush()); + ASSERT_OK(Put(Key(1), "val")); + HoldAutoRecovery(); + SyncPoint::GetInstance()->SetCallBack( + "VersionSet::LogAndApply:WriteManifest", + [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); + SyncPoint::GetInstance()->EnableProcessing(); + Status s = Flush(); + ASSERT_FALSE(dbfull()->TEST_GetFilesToQuarantine().empty()); + ASSERT_EQ(s.severity(), expected_severity); + ASSERT_NO_FATAL_FAILURE(ActivateFSAndRecover()); - new_manifest = GetManifestNameFromLiveFiles(); - ASSERT_NE(new_manifest, old_manifest); - ASSERT_TRUE(dbfull()->TEST_GetFilesToQuarantine().empty()); + ASSERT_NE(GetManifestNameFromLiveFiles(), old_manifest); + ASSERT_TRUE(dbfull()->TEST_GetFilesToQuarantine().empty()); - Reopen(options); - ASSERT_EQ("val", Get(Key(0))); - ASSERT_EQ("val", Get(Key(1))); - Close(); + Reopen(options); + ASSERT_EQ("val", Get(Key(0))); + ASSERT_EQ("val", Get(Key(1))); + Close(); + } } TEST_F(DBErrorHandlingFSTest, ManifestWriteNoWALRetryableError) { @@ -811,7 +814,7 @@ TEST_F(DBErrorHandlingFSTest, ManifestWriteNoWALRetryableError) { [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); + ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kHardError); ASSERT_FALSE(dbfull()->TEST_GetFilesToQuarantine().empty()); SyncPoint::GetInstance()->ClearAllCallBacks(); SyncPoint::GetInstance()->DisableProcessing(); @@ -1076,102 +1079,77 @@ TEST_F(DBErrorHandlingFSTest, CompactionWriteError) { Destroy(options); } -TEST_F(DBErrorHandlingFSTest, DISABLED_CompactionWriteRetryableError) { - std::shared_ptr listener = - std::make_shared(); - Options options = GetDefaultOptions(); - options.env = fault_env_.get(); - options.create_if_missing = true; - options.level0_file_num_compaction_trigger = 2; - options.listeners.emplace_back(listener); - options.max_bgerror_resume_count = 0; - Status s; - DestroyAndReopen(options); +// Without auto resume, a retryable or file scope IO error during compaction +// must stop writes until a manual Resume(), rather than being silently +// retried by rescheduling the compaction. +TEST_F(DBErrorHandlingFSTest, CompactionWriteErrorNoAutoResume) { + IOStatus retryable_error = IOStatus::IOError("Retryable IO Error"); + retryable_error.SetRetryable(true); + IOStatus file_scope_error = IOStatus::IOError("File Scope Data Loss Error"); + file_scope_error.SetDataLoss(true); + file_scope_error.SetScope(IOStatus::IOErrorScope::kIOErrorScopeFile); + file_scope_error.SetRetryable(false); - IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); - error_msg.SetRetryable(true); + for (const IOStatus& error_msg : {retryable_error, file_scope_error}) { + SCOPED_TRACE(error_msg.ToString()); + Options options = GetDefaultOptions(); + options.env = fault_env_.get(); + options.create_if_missing = true; + options.level0_file_num_compaction_trigger = 2; + options.max_bgerror_resume_count = 0; + options.statistics = CreateDBStatistics(); + DestroyAndReopen(options); - ASSERT_OK(Put(Key(0), "va;")); - ASSERT_OK(Put(Key(2), "va;")); - s = Flush(); - ASSERT_OK(s); + ASSERT_OK(Put(Key(0), "val")); + ASSERT_OK(Put(Key(2), "val")); + ASSERT_OK(Flush()); - listener->OverrideBGError(Status(error_msg, Status::Severity::kHardError)); - listener->EnableAutoRecovery(false); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( - {{"DBImpl::FlushMemTable:FlushMemTableFinished", - "BackgroundCallCompaction:0"}}); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - "CompactionJob::OpenCompactionOutputFile", - [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - "DBImpl::BackgroundCompaction:Finish", - [&](void*) { CancelAllBackgroundWork(dbfull()); }); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); + SyncPoint::GetInstance()->LoadDependency( + {{"DBImpl::FlushMemTable:FlushMemTableFinished", + "BackgroundCallCompaction:0"}}); + // Fail only the first attempt, so a rescheduled compaction would succeed + std::atomic injected(false); + SyncPoint::GetInstance()->SetCallBack( + "DBImpl::BackgroundCompaction:Start", [&](void*) { + if (injected.load()) { + fault_fs_->SetFilesystemActive(true); + } + }); + SyncPoint::GetInstance()->SetCallBack( + "CompactionJob::OpenCompactionOutputFile", [&](void*) { + if (!injected.exchange(true)) { + fault_fs_->SetFilesystemActive(false, error_msg); + } + }); + SyncPoint::GetInstance()->EnableProcessing(); - ASSERT_OK(Put(Key(1), "val")); - s = Flush(); - ASSERT_OK(s); + ASSERT_OK(Put(Key(1), "val")); + ASSERT_OK(Flush()); - s = dbfull()->TEST_GetBGError(); - ASSERT_OK(s); - fault_fs_->SetFilesystemActive(true); - SyncPoint::GetInstance()->ClearAllCallBacks(); - SyncPoint::GetInstance()->DisableProcessing(); - s = dbfull()->Resume(); - ASSERT_OK(s); - Destroy(options); -} + Status s = dbfull()->TEST_WaitForCompact(); + ASSERT_EQ(s.severity(), Status::Severity::kHardError); + ASSERT_EQ(dbfull()->TEST_GetBGError().severity(), + Status::Severity::kHardError); + ASSERT_NOK(Put(Key(3), "val")); -TEST_F(DBErrorHandlingFSTest, DISABLED_CompactionWriteFileScopeError) { - std::shared_ptr listener = - std::make_shared(); - Options options = GetDefaultOptions(); - options.env = fault_env_.get(); - options.create_if_missing = true; - options.level0_file_num_compaction_trigger = 2; - options.listeners.emplace_back(listener); - options.max_bgerror_resume_count = 0; - Status s; - DestroyAndReopen(options); + SyncPoint::GetInstance()->DisableProcessing(); + SyncPoint::GetInstance()->ClearAllCallBacks(); + fault_fs_->SetFilesystemActive(true); + // Reads are still served while writes are stopped + ASSERT_EQ("val", Get(Key(1))); + ASSERT_OK(dbfull()->Resume()); + ASSERT_OK(Put(Key(3), "val")); + ASSERT_OK(dbfull()->TEST_WaitForCompact()); + ASSERT_EQ(0, NumTableFilesAtLevel(0)); + ASSERT_EQ(0, options.statistics->getAndResetTickerCount( + ERROR_HANDLER_AUTORESUME_COUNT)); - IOStatus error_msg = IOStatus::IOError("File Scope Data Loss Error"); - error_msg.SetDataLoss(true); - error_msg.SetScope( - ROCKSDB_NAMESPACE::IOStatus::IOErrorScope::kIOErrorScopeFile); - error_msg.SetRetryable(false); - - ASSERT_OK(Put(Key(0), "va;")); - ASSERT_OK(Put(Key(2), "va;")); - s = Flush(); - ASSERT_OK(s); - - listener->OverrideBGError(Status(error_msg, Status::Severity::kHardError)); - listener->EnableAutoRecovery(false); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( - {{"DBImpl::FlushMemTable:FlushMemTableFinished", - "BackgroundCallCompaction:0"}}); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - "CompactionJob::OpenCompactionOutputFile", - [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - "DBImpl::BackgroundCompaction:Finish", - [&](void*) { CancelAllBackgroundWork(dbfull()); }); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - - ASSERT_OK(Put(Key(1), "val")); - s = Flush(); - ASSERT_OK(s); - - s = dbfull()->TEST_GetBGError(); - ASSERT_OK(s); - - fault_fs_->SetFilesystemActive(true); - SyncPoint::GetInstance()->ClearAllCallBacks(); - SyncPoint::GetInstance()->DisableProcessing(); - s = dbfull()->Resume(); - ASSERT_OK(s); - Destroy(options); + Reopen(options); + for (int i = 0; i < 4; ++i) { + ASSERT_EQ("val", Get(Key(i))); + } + Destroy(options); + } } TEST_F(DBErrorHandlingFSTest, CorruptionError) { @@ -2196,13 +2174,15 @@ TEST_F(DBErrorHandlingFSTest, ManualResumePreservesStrongerRecoveryContext) { options.env = fault_env_.get(); options.create_if_missing = true; options.listeners.emplace_back(listener); - options.max_bgerror_resume_count = 0; + // A soft no-WAL error is only possible with auto resume enabled. Use a + // single attempt that fails, so the error is left for a manual Resume(). + options.max_bgerror_resume_count = 1; + options.bgerror_resume_retry_interval = 1000; options.avoid_flush_during_shutdown = true; listener->EnableAutoRecovery(false); DestroyAndReopen(options); ASSERT_OK(Put(Key(0), "value")); - const uint64_t wal_before_recovery = dbfull()->TEST_GetCurrentLogNumber(); IOStatus soft_error = IOStatus::IOError("injected soft error"); dbfull()->TEST_SetBGError(soft_error, BackgroundErrorReason::kAsyncFileOpen); @@ -2211,11 +2191,36 @@ TEST_F(DBErrorHandlingFSTest, ManualResumePreservesStrongerRecoveryContext) { IOStatus retryable_error = IOStatus::IOError("injected retryable error"); retryable_error.SetRetryable(true); + // Let the flush thread finish reporting its error before auto recovery gives + // up. Otherwise the report can start a new recovery after the old one ends. + SyncPoint::GetInstance()->LoadDependency( + {{"DBImpl::BackgroundCallFlush:FlushFinish:0", + "RecoverFromRetryableBGIOError:BeforeWait0"}, + {"RecoverFromRetryableBGIOError:LoopOut", + "ManualResumePreservesStrongerRecoveryContext:AutoResumeDone"}}); + std::atomic injected(false); + SyncPoint::GetInstance()->SetCallBack( + "FlushJob::WriteLevel0Table:s", [&](void* arg) { + if (!injected.exchange(true)) { + *static_cast(arg) = retryable_error; + } + }); + SyncPoint::GetInstance()->EnableProcessing(); dbfull()->TEST_SetBGError(retryable_error, BackgroundErrorReason::kFlushNoWAL); + TEST_SYNC_POINT( + "ManualResumePreservesStrongerRecoveryContext:AutoResumeDone"); + SyncPoint::GetInstance()->DisableProcessing(); + SyncPoint::GetInstance()->ClearAllCallBacks(); + ASSERT_TRUE(injected.load()); + ASSERT_FALSE(dbfull()->TEST_IsRecoveryInProgress()); ASSERT_EQ(Status::Severity::kSoftError, dbfull()->TEST_GetBGError().severity()); + // Writes continue under the soft error. The manual resume must flush them, + // which switches the WAL, rather than clear the error directly. + ASSERT_OK(Put(Key(1), "value")); + const uint64_t wal_before_recovery = dbfull()->TEST_GetCurrentLogNumber(); ASSERT_OK(dbfull()->Resume()); const uint64_t wal_after_recovery = dbfull()->TEST_GetCurrentLogNumber(); ASSERT_GT(wal_after_recovery, wal_before_recovery) @@ -2223,11 +2228,10 @@ TEST_F(DBErrorHandlingFSTest, ManualResumePreservesStrongerRecoveryContext) { Reopen(options); ASSERT_EQ("value", Get(Key(0))); + ASSERT_EQ("value", Get(Key(1))); Destroy(options); } -// Auto resume fromt the flush retryable IO error. Activate the FS before the -// first resume. Resume is successful TEST_F(DBErrorHandlingFSTest, FlushWritRetryableErrorAutoRecover1) { // Activate the FS before the first resume std::shared_ptr listener = @@ -4533,7 +4537,8 @@ TEST_F(DBErrorHandlingFSTest, AtomicFlushErrorRecoveryTwoCFs) { options.create_if_missing = true; options.atomic_flush = true; options.listeners.emplace_back(listener); - options.max_bgerror_resume_count = 0; + // Auto resume keeps the no-WAL error soft, so recovery retries the flushes + options.max_bgerror_resume_count = 2; listener->EnableAutoRecovery(false); CreateAndReopenWithCF({"pikachu"}, options); @@ -4552,20 +4557,18 @@ TEST_F(DBErrorHandlingFSTest, AtomicFlushErrorRecoveryTwoCFs) { ASSERT_OK(Put(0, Key(1), "v0b", wo)); ASSERT_OK(Put(1, Key(0), "v1", wo)); + HoldAutoRecovery(); SyncPoint::GetInstance()->SetCallBack( "VersionSet::LogAndApply:WriteManifest", [&](void*) { fault_fs_->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); Status s = Flush({0, 1}); ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); - SyncPoint::GetInstance()->ClearAllCallBacks(); - SyncPoint::GetInstance()->DisableProcessing(); - fault_fs_->SetFilesystemActive(true); - // Resume routes through RetryFlushesForErrorRecovery(wait=true); the pairing - // invariant in that function fires under a debug build if the ids are - // misaligned with cfds. - ASSERT_OK(dbfull()->Resume()); + // Auto recovery routes through RetryFlushesForErrorRecovery(wait=true); the + // pairing invariant in that function fires under a debug build if the ids + // are misaligned with cfds. + ASSERT_NO_FATAL_FAILURE(ActivateFSAndRecover()); ASSERT_EQ("v0", Get(0, Key(0))); ASSERT_EQ("v0b", Get(0, Key(1))); diff --git a/include/rocksdb/options.h b/include/rocksdb/options.h index 34d9ca7bfa..ebfc4f7d8e 100644 --- a/include/rocksdb/options.h +++ b/include/rocksdb/options.h @@ -1749,7 +1749,10 @@ struct DBOptions { // Error happens, SetBGError is called to deal with the error. If the error // can be auto-recovered (e.g., retryable IO Error during Flush or WAL write), // then db resume is called in background to recover from the error. If this - // value is 0 or negative, DB::Resume() will not be called automatically. + // value is 0 or negative, DB::Resume() will not be called automatically, and + // any background retryable IO error (including one during compaction, which + // would otherwise be retried by rescheduling the compaction) stops writes + // until the user calls DB::Resume(). // // Default: INT_MAX int max_bgerror_resume_count = INT_MAX; diff --git a/unreleased_history/behavior_changes/no_auto_resume_stops_writes.md b/unreleased_history/behavior_changes/no_auto_resume_stops_writes.md new file mode 100644 index 0000000000..5a237bc97b --- /dev/null +++ b/unreleased_history/behavior_changes/no_auto_resume_stops_writes.md @@ -0,0 +1 @@ +With `max_bgerror_resume_count <= 0` (auto resume disabled), a retryable or file-scoped IO error during compaction or flush is now a hard background error that stops writes until `DB::Resume()` is called. Previously, compaction errors were silently retried, and flush errors with a healthy WAL were soft errors that let writes continue while flushes were stopped.