From 87e82a41a98fbb89591b0599625235de0b60dafc Mon Sep 17 00:00:00 2001 From: Zhichao Cao Date: Tue, 20 Jul 2021 18:08:55 -0700 Subject: [PATCH] Fix incorrect Status::NoSpace() status check (#8504) Summary: If we want to check whether a Status s is NoSpace() or not, we should check the subcode instread of using s==Status::NoSpace(). Fix some of the incorrect check in the ErrorHandler. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8504 Test Plan: make check Reviewed By: anand1976 Differential Revision: D29601764 Pulled By: zhichao-cao fbshipit-source-id: cdab56a827891c23746bba9cbb53f169fe35f086 --- db/error_handler.cc | 6 ++++-- file/sst_file_manager_impl.cc | 2 +- utilities/fault_injection_env.h | 3 ++- utilities/fault_injection_fs.h | 3 ++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/db/error_handler.cc b/db/error_handler.cc index b5c353a69..38b4fa049 100644 --- a/db/error_handler.cc +++ b/db/error_handler.cc @@ -327,7 +327,8 @@ const Status& ErrorHandler::SetBGError(const Status& bg_err, } // Allow some error specific overrides - if (new_bg_err == Status::NoSpace()) { + if (new_bg_err.subcode() == IOStatus::SubCode::kNoSpace || + new_bg_err.subcode() == IOStatus::SubCode::kSpaceLimit) { new_bg_err = OverrideNoSpaceError(new_bg_err, &auto_recovery); } @@ -349,7 +350,8 @@ const Status& ErrorHandler::SetBGError(const Status& bg_err, recovery_in_prog_ = true; // Kick-off error specific recovery - if (bg_error_ == Status::NoSpace()) { + if (new_bg_err.subcode() == IOStatus::SubCode::kNoSpace || + new_bg_err.subcode() == IOStatus::SubCode::kSpaceLimit) { RecoverFromNoSpace(); } } diff --git a/file/sst_file_manager_impl.cc b/file/sst_file_manager_impl.cc index cc03e5444..161602fa7 100644 --- a/file/sst_file_manager_impl.cc +++ b/file/sst_file_manager_impl.cc @@ -317,7 +317,7 @@ void SstFileManagerImpl::ClearError() { // error is also a NoSpace() non-fatal error, leave the instance in // the list Status err = cur_instance_->GetBGError(); - if (s.ok() && err == Status::NoSpace() && + if (s.ok() && err.subcode() == IOStatus::SubCode::kNoSpace && err.severity() < Status::Severity::kFatalError) { s = err; } diff --git a/utilities/fault_injection_env.h b/utilities/fault_injection_env.h index ab232a1a2..fa1fa0d64 100644 --- a/utilities/fault_injection_env.h +++ b/utilities/fault_injection_env.h @@ -179,7 +179,8 @@ class FaultInjectionTestEnv : public EnvWrapper { #undef GetFreeSpace virtual Status GetFreeSpace(const std::string& path, uint64_t* disk_free) override { - if (!IsFilesystemActive() && error_ == Status::NoSpace()) { + if (!IsFilesystemActive() && + error_.subcode() == IOStatus::SubCode::kNoSpace) { *disk_free = 0; return Status::OK(); } else { diff --git a/utilities/fault_injection_fs.h b/utilities/fault_injection_fs.h index 625911af3..62c938ca0 100644 --- a/utilities/fault_injection_fs.h +++ b/utilities/fault_injection_fs.h @@ -240,7 +240,8 @@ class FaultInjectionTestFS : public FileSystemWrapper { const IOOptions& options, uint64_t* disk_free, IODebugContext* dbg) override { IOStatus io_s; - if (!IsFilesystemActive() && error_ == IOStatus::NoSpace()) { + if (!IsFilesystemActive() && + error_.subcode() == IOStatus::SubCode::kNoSpace) { *disk_free = 0; } else { io_s = target()->GetFreeSpace(path, options, disk_free, dbg);