From b7062f0b2cf05917edb21ff4e2e9fed4fc44284f Mon Sep 17 00:00:00 2001 From: Zhichao Cao Date: Fri, 2 Oct 2020 16:39:17 -0700 Subject: [PATCH] Status check enforcement for error_handler_fs_test (#7342) Summary: Added status check enforcement for error_test_fs_test Pull Request resolved: https://github.com/facebook/rocksdb/pull/7342 Test Plan: ASSERT_STATUS_CHECKED=1 make -j48 error_test_fs_test Reviewed By: akankshamahajan15 Differential Revision: D23972231 Pulled By: zhichao-cao fbshipit-source-id: fa41bfe440012e0c55f2c9507c1d0104e5e93f84 --- Makefile | 2 + db/compaction/compaction_job.cc | 4 +- db/db_impl/db_impl.cc | 6 +- db/db_impl/db_impl_write.cc | 19 ++- db/error_handler_fs_test.cc | 205 ++++++++++++++++---------------- db/event_helpers.cc | 2 + db/version_set.cc | 3 +- db/write_thread.h | 1 + utilities/fault_injection_env.h | 2 + utilities/fault_injection_fs.h | 8 +- 10 files changed, 140 insertions(+), 112 deletions(-) diff --git a/Makefile b/Makefile index f0534de2e..b061436ac 100644 --- a/Makefile +++ b/Makefile @@ -600,8 +600,10 @@ ifdef ASSERT_STATUS_CHECKED env_test \ env_logger_test \ event_logger_test \ + error_handler_fs_test \ auto_roll_logger_test \ file_indexer_test \ + flush_job_test \ hash_table_test \ hash_test \ heap_test \ diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc index 366c493ab..bcb90a2d1 100644 --- a/db/compaction/compaction_job.cc +++ b/db/compaction/compaction_job.cc @@ -1465,7 +1465,9 @@ Status CompactionJob::FinishCompactionOutputFile( "CompactionJob::FinishCompactionOutputFile:" "MaxAllowedSpaceReached"); InstrumentedMutexLock l(db_mutex_); - db_error_handler_->SetBGError(s, BackgroundErrorReason::kCompaction); + // Should handle return error? + db_error_handler_->SetBGError(s, BackgroundErrorReason::kCompaction) + .PermitUncheckedError(); } } #endif diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index d15a59b91..4bb62ed16 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -407,7 +407,7 @@ Status DBImpl::ResumeImpl(DBRecoverContext context) { // during previous error handling. if (file_deletion_disabled) { // Always return ok - EnableFileDeletions(/*force=*/true); + s = EnableFileDeletions(/*force=*/true); } ROCKS_LOG_INFO(immutable_db_options_.info_log, "Successfully resumed DB"); } @@ -4553,7 +4553,9 @@ Status DBImpl::IngestExternalFiles( // be pessimistic and try write to a new MANIFEST. // TODO: distinguish between MANIFEST write and CURRENT renaming const IOStatus& io_s = versions_->io_status(); - error_handler_.SetBGError(io_s, BackgroundErrorReason::kManifestWrite); + // Should handle return error? + error_handler_.SetBGError(io_s, BackgroundErrorReason::kManifestWrite) + .PermitUncheckedError(); } // Resume writes to the DB diff --git a/db/db_impl/db_impl_write.cc b/db/db_impl/db_impl_write.cc index 475b7608a..f120a0d9f 100644 --- a/db/db_impl/db_impl_write.cc +++ b/db/db_impl/db_impl_write.cc @@ -840,7 +840,9 @@ void DBImpl::WriteStatusCheckOnLocked(const Status& status) { mutex_.AssertHeld(); if (immutable_db_options_.paranoid_checks && !status.ok() && !status.IsBusy() && !status.IsIncomplete()) { - error_handler_.SetBGError(status, BackgroundErrorReason::kWriteCallback); + // Maybe change the return status to void? + error_handler_.SetBGError(status, BackgroundErrorReason::kWriteCallback) + .PermitUncheckedError(); } } @@ -851,7 +853,9 @@ void DBImpl::WriteStatusCheck(const Status& status) { if (immutable_db_options_.paranoid_checks && !status.ok() && !status.IsBusy() && !status.IsIncomplete()) { mutex_.Lock(); - error_handler_.SetBGError(status, BackgroundErrorReason::kWriteCallback); + // Maybe change the return status to void? + error_handler_.SetBGError(status, BackgroundErrorReason::kWriteCallback) + .PermitUncheckedError(); mutex_.Unlock(); } } @@ -863,7 +867,7 @@ void DBImpl::IOStatusCheck(const IOStatus& io_status) { !io_status.IsBusy() && !io_status.IsIncomplete()) || io_status.IsIOFenced()) { mutex_.Lock(); - // May be change the return status to void? + // Maybe change the return status to void? error_handler_.SetBGError(io_status, BackgroundErrorReason::kWriteCallback) .PermitUncheckedError(); mutex_.Unlock(); @@ -879,7 +883,7 @@ void DBImpl::MemTableInsertStatusCheck(const Status& status) { if (!status.ok()) { mutex_.Lock(); assert(!error_handler_.IsBGWorkStopped()); - // May be change the return status to void? + // Maybe change the return status to void? error_handler_.SetBGError(status, BackgroundErrorReason::kMemTable) .PermitUncheckedError(); mutex_.Unlock(); @@ -1775,10 +1779,13 @@ Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context) { } // We may have lost data from the WritableFileBuffer in-memory buffer for // the current log, so treat it as a fatal error and set bg_error + // Should handle return error? if (!io_s.ok()) { - error_handler_.SetBGError(io_s, BackgroundErrorReason::kMemTable); + error_handler_.SetBGError(io_s, BackgroundErrorReason::kMemTable) + .PermitUncheckedError(); } else { - error_handler_.SetBGError(s, BackgroundErrorReason::kMemTable); + error_handler_.SetBGError(s, BackgroundErrorReason::kMemTable) + .PermitUncheckedError(); } // Read back bg_error in order to get the right severity s = error_handler_.GetBGError(); diff --git a/db/error_handler_fs_test.cc b/db/error_handler_fs_test.cc index 4336043af..895c878ab 100644 --- a/db/error_handler_fs_test.cc +++ b/db/error_handler_fs_test.cc @@ -31,7 +31,10 @@ class DBErrorHandlingFSTest : public DBTestBase { std::vector live_files; uint64_t manifest_size; - dbfull()->GetLiveFiles(live_files, &manifest_size, false); + Status s = dbfull()->GetLiveFiles(live_files, &manifest_size, false); + if (!s.ok()) { + return ""; + } for (auto& file : live_files) { uint64_t num = 0; FileType type; @@ -69,6 +72,10 @@ class ErrorHandlerFSListener : public EventListener { override_bg_error_(false), file_count_(0), fault_fs_(nullptr) {} + ~ErrorHandlerFSListener() { + file_creation_error_.PermitUncheckedError(); + bg_error_.PermitUncheckedError(); + } void OnTableFileCreationStarted( const TableFileCreationBriefInfo& /*ti*/) override { @@ -83,17 +90,19 @@ class ErrorHandlerFSListener : public EventListener { cv_.SignalAll(); } - void OnErrorRecoveryBegin(BackgroundErrorReason /*reason*/, - Status /*bg_error*/, bool* auto_recovery) override { + void OnErrorRecoveryBegin(BackgroundErrorReason /*reason*/, Status bg_error, + bool* auto_recovery) override { + bg_error.PermitUncheckedError(); if (*auto_recovery && no_auto_recovery_) { *auto_recovery = false; } } - void OnErrorRecoveryCompleted(Status /*old_bg_error*/) override { + void OnErrorRecoveryCompleted(Status old_bg_error) override { InstrumentedMutexLock l(&mutex_); recovery_complete_ = true; cv_.SignalAll(); + old_bg_error.PermitUncheckedError(); } bool WaitForRecovery(uint64_t /*abs_time_us*/) { @@ -166,7 +175,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWriteError) { listener->EnableAutoRecovery(false); DestroyAndReopen(options); - Put(Key(0), "val"); + ASSERT_OK(Put(Key(0), "val")); SyncPoint::GetInstance()->SetCallBack("FlushJob::Start", [&](void*) { fault_fs->SetFilesystemActive(false, IOStatus::NoSpace("Out of space")); }); @@ -202,7 +211,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableError) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(1), "val1"); + ASSERT_OK(Put(Key(1), "val1")); SyncPoint::GetInstance()->SetCallBack( "BuildTable:BeforeFinishBuildTable", [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); @@ -216,7 +225,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableError) { Reopen(options); ASSERT_EQ("val1", Get(Key(1))); - Put(Key(2), "val2"); + ASSERT_OK(Put(Key(2), "val2")); SyncPoint::GetInstance()->SetCallBack( "BuildTable:BeforeSyncTable", [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); @@ -230,7 +239,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableError) { Reopen(options); ASSERT_EQ("val2", Get(Key(2))); - Put(Key(3), "val3"); + ASSERT_OK(Put(Key(3), "val3")); SyncPoint::GetInstance()->SetCallBack( "BuildTable:BeforeCloseTableFile", [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); @@ -268,13 +277,13 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableError1) { WriteOptions wo = WriteOptions(); wo.disableWAL = true; - Put(Key(1), "val1", wo); + ASSERT_OK(Put(Key(1), "val1", wo)); SyncPoint::GetInstance()->SetCallBack( "BuildTable:BeforeFinishBuildTable", [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - Put(Key(2), "val2", wo); + ASSERT_OK(Put(Key(2), "val2", wo)); ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); ASSERT_EQ("val2", Get(Key(2))); SyncPoint::GetInstance()->DisableProcessing(); @@ -283,7 +292,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableError1) { ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val1", Get(Key(1))); ASSERT_EQ("val2", Get(Key(2))); - Put(Key(3), "val3", wo); + ASSERT_OK(Put(Key(3), "val3", wo)); ASSERT_EQ("val3", Get(Key(3))); s = Flush(); ASSERT_OK(s); @@ -314,13 +323,13 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableError2) { WriteOptions wo = WriteOptions(); wo.disableWAL = true; - Put(Key(1), "val1", wo); + ASSERT_OK(Put(Key(1), "val1", wo)); SyncPoint::GetInstance()->SetCallBack( "BuildTable:BeforeSyncTable", [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - Put(Key(2), "val2", wo); + ASSERT_OK(Put(Key(2), "val2", wo)); ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); ASSERT_EQ("val2", Get(Key(2))); SyncPoint::GetInstance()->DisableProcessing(); @@ -329,7 +338,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableError2) { ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val1", Get(Key(1))); ASSERT_EQ("val2", Get(Key(2))); - Put(Key(3), "val3", wo); + ASSERT_OK(Put(Key(3), "val3", wo)); ASSERT_EQ("val3", Get(Key(3))); s = Flush(); ASSERT_OK(s); @@ -360,13 +369,13 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableError3) { WriteOptions wo = WriteOptions(); wo.disableWAL = true; - Put(Key(1), "val1", wo); + ASSERT_OK(Put(Key(1), "val1", wo)); SyncPoint::GetInstance()->SetCallBack( "BuildTable:BeforeCloseTableFile", [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); SyncPoint::GetInstance()->EnableProcessing(); s = Flush(); - Put(Key(2), "val2", wo); + ASSERT_OK(Put(Key(2), "val2", wo)); ASSERT_EQ(s.severity(), ROCKSDB_NAMESPACE::Status::Severity::kSoftError); ASSERT_EQ("val2", Get(Key(2))); SyncPoint::GetInstance()->DisableProcessing(); @@ -375,7 +384,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableError3) { ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val1", Get(Key(1))); ASSERT_EQ("val2", Get(Key(2))); - Put(Key(3), "val3", wo); + ASSERT_OK(Put(Key(3), "val3", wo)); ASSERT_EQ("val3", Get(Key(3))); s = Flush(); ASSERT_OK(s); @@ -402,9 +411,9 @@ TEST_F(DBErrorHandlingFSTest, ManifestWriteError) { DestroyAndReopen(options); old_manifest = GetManifestNameFromLiveFiles(); - Put(Key(0), "val"); - Flush(); - Put(Key(1), "val"); + 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, IOStatus::NoSpace("Out of space")); @@ -449,9 +458,9 @@ TEST_F(DBErrorHandlingFSTest, ManifestWriteRetryableError) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(0), "val"); - Flush(); - Put(Key(1), "val"); + 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); }); @@ -491,9 +500,9 @@ TEST_F(DBErrorHandlingFSTest, DoubleManifestWriteError) { DestroyAndReopen(options); old_manifest = GetManifestNameFromLiveFiles(); - Put(Key(0), "val"); - Flush(); - Put(Key(1), "val"); + 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, IOStatus::NoSpace("Out of space")); @@ -541,8 +550,8 @@ TEST_F(DBErrorHandlingFSTest, CompactionManifestWriteError) { DestroyAndReopen(options); old_manifest = GetManifestNameFromLiveFiles(); - Put(Key(0), "val"); - Put(Key(2), "val"); + ASSERT_OK(Put(Key(0), "val")); + ASSERT_OK(Put(Key(2), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -570,7 +579,7 @@ TEST_F(DBErrorHandlingFSTest, CompactionManifestWriteError) { }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); // This Flush will trigger a compaction, which will fail when appending to // the manifest s = Flush(); @@ -618,8 +627,8 @@ TEST_F(DBErrorHandlingFSTest, CompactionManifestWriteRetryableError) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(0), "val"); - Put(Key(2), "val"); + ASSERT_OK(Put(Key(0), "val")); + ASSERT_OK(Put(Key(2), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -645,7 +654,7 @@ TEST_F(DBErrorHandlingFSTest, CompactionManifestWriteRetryableError) { }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -685,8 +694,8 @@ TEST_F(DBErrorHandlingFSTest, CompactionWriteError) { Status s; DestroyAndReopen(options); - Put(Key(0), "va;"); - Put(Key(2), "va;"); + ASSERT_OK(Put(Key(0), "va;")); + ASSERT_OK(Put(Key(2), "va;")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -702,7 +711,7 @@ TEST_F(DBErrorHandlingFSTest, CompactionWriteError) { }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -733,8 +742,8 @@ TEST_F(DBErrorHandlingFSTest, CompactionWriteRetryableError) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(0), "va;"); - Put(Key(2), "va;"); + ASSERT_OK(Put(Key(0), "va;")); + ASSERT_OK(Put(Key(2), "va;")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -748,7 +757,7 @@ TEST_F(DBErrorHandlingFSTest, CompactionWriteRetryableError) { [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -774,8 +783,8 @@ TEST_F(DBErrorHandlingFSTest, CorruptionError) { Status s; DestroyAndReopen(options); - Put(Key(0), "va;"); - Put(Key(2), "va;"); + ASSERT_OK(Put(Key(0), "va;")); + ASSERT_OK(Put(Key(2), "va;")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -789,7 +798,7 @@ TEST_F(DBErrorHandlingFSTest, CorruptionError) { }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -818,7 +827,7 @@ TEST_F(DBErrorHandlingFSTest, AutoRecoverFlushError) { listener->EnableAutoRecovery(); DestroyAndReopen(options); - Put(Key(0), "val"); + ASSERT_OK(Put(Key(0), "val")); SyncPoint::GetInstance()->SetCallBack("FlushJob::Start", [&](void*) { fault_fs->SetFilesystemActive(false, IOStatus::NoSpace("Out of space")); }); @@ -853,7 +862,7 @@ TEST_F(DBErrorHandlingFSTest, FailRecoverFlushError) { listener->EnableAutoRecovery(); DestroyAndReopen(options); - Put(Key(0), "val"); + ASSERT_OK(Put(Key(0), "val")); SyncPoint::GetInstance()->SetCallBack("FlushJob::Start", [&](void*) { fault_fs->SetFilesystemActive(false, IOStatus::NoSpace("Out of space")); }); @@ -887,7 +896,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteError) { WriteBatch batch; for (auto i = 0; i < 100; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; @@ -900,7 +909,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteError) { int write_error = 0; for (auto i = 100; i < 199; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } SyncPoint::GetInstance()->SetCallBack( @@ -964,7 +973,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableError) { WriteBatch batch; for (auto i = 0; i < 100; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; @@ -979,7 +988,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableError) { int write_error = 0; for (auto i = 100; i < 200; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } SyncPoint::GetInstance()->SetCallBack( @@ -1015,7 +1024,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableError) { WriteBatch batch; for (auto i = 200; i < 300; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; @@ -1056,7 +1065,7 @@ TEST_F(DBErrorHandlingFSTest, MultiCFWALWriteError) { for (auto i = 1; i < 4; ++i) { for (auto j = 0; j < 100; ++j) { - batch.Put(handles_[i], Key(j), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(handles_[i], Key(j), rnd.RandomString(1024))); } } @@ -1071,7 +1080,7 @@ TEST_F(DBErrorHandlingFSTest, MultiCFWALWriteError) { // Write to one CF for (auto i = 100; i < 199; ++i) { - batch.Put(handles_[2], Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(handles_[2], Key(i), rnd.RandomString(1024))); } SyncPoint::GetInstance()->SetCallBack( @@ -1160,7 +1169,7 @@ TEST_F(DBErrorHandlingFSTest, MultiDBCompactionError) { WriteBatch batch; for (auto j = 0; j <= 100; ++j) { - batch.Put(Key(j), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(j), rnd.RandomString(1024))); } WriteOptions wopts; @@ -1175,7 +1184,7 @@ TEST_F(DBErrorHandlingFSTest, MultiDBCompactionError) { // Write to one CF for (auto j = 100; j < 199; ++j) { - batch.Put(Key(j), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(j), rnd.RandomString(1024))); } WriteOptions wopts; @@ -1273,7 +1282,7 @@ TEST_F(DBErrorHandlingFSTest, MultiDBVariousErrors) { WriteBatch batch; for (auto j = 0; j <= 100; ++j) { - batch.Put(Key(j), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(j), rnd.RandomString(1024))); } WriteOptions wopts; @@ -1288,7 +1297,7 @@ TEST_F(DBErrorHandlingFSTest, MultiDBVariousErrors) { // Write to one CF for (auto j = 100; j < 199; ++j) { - batch.Put(Key(j), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(j), rnd.RandomString(1024))); } WriteOptions wopts; @@ -1378,7 +1387,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableeErrorAutoRecover1) { WriteOptions wo = WriteOptions(); wo.disableWAL = true; - Put(Key(1), "val1", wo); + ASSERT_OK(Put(Key(1), "val1", wo)); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"RecoverFromRetryableBGIOError:LoopOut", "FLushWritNoWALRetryableeErrorAutoRecover1:1"}}); @@ -1395,7 +1404,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableeErrorAutoRecover1) { ASSERT_EQ("val1", Get(Key(1))); SyncPoint::GetInstance()->DisableProcessing(); fault_fs->SetFilesystemActive(true); - Put(Key(2), "val2", wo); + ASSERT_OK(Put(Key(2), "val2", wo)); s = Flush(); // Since auto resume fails, the bg error is not cleand, flush will // return the bg_error set before. @@ -1405,7 +1414,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableeErrorAutoRecover1) { // call auto resume s = dbfull()->Resume(); ASSERT_EQ(s, Status::OK()); - Put(Key(3), "val3", wo); + ASSERT_OK(Put(Key(3), "val3", wo)); s = Flush(); // After resume is successful, the flush should be ok. ASSERT_EQ(s, Status::OK()); @@ -1436,7 +1445,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableeErrorAutoRecover2) { WriteOptions wo = WriteOptions(); wo.disableWAL = true; - Put(Key(1), "val1", wo); + ASSERT_OK(Put(Key(1), "val1", wo)); SyncPoint::GetInstance()->SetCallBack( "BuildTable:BeforeFinishBuildTable", [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); @@ -1449,7 +1458,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritNoWALRetryableeErrorAutoRecover2) { fault_fs->SetFilesystemActive(true); ASSERT_EQ(listener->WaitForRecovery(5000000), true); ASSERT_EQ("val1", Get(Key(1))); - Put(Key(2), "val2", wo); + ASSERT_OK(Put(Key(2), "val2", wo)); s = Flush(); // Since auto resume is successful, the bg error is cleaned, flush will // be successful. @@ -1479,7 +1488,7 @@ TEST_F(DBErrorHandlingFSTest, DISABLED_FLushWritRetryableeErrorAutoRecover1) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(1), "val1"); + ASSERT_OK(Put(Key(1), "val1")); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"RecoverFromRetryableBGIOError:BeforeWait0", "FLushWritRetryableeErrorAutoRecover1:0"}, @@ -1503,7 +1512,7 @@ TEST_F(DBErrorHandlingFSTest, DISABLED_FLushWritRetryableeErrorAutoRecover1) { ASSERT_EQ("val1", Get(Key(1))); Reopen(options); ASSERT_EQ("val1", Get(Key(1))); - Put(Key(2), "val2"); + ASSERT_OK(Put(Key(2), "val2")); s = Flush(); ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val2", Get(Key(2))); @@ -1532,7 +1541,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableeErrorAutoRecover2) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(1), "val1"); + ASSERT_OK(Put(Key(1), "val1")); SyncPoint::GetInstance()->SetCallBack( "BuildTable:BeforeFinishBuildTable", [&](void*) { fault_fs->SetFilesystemActive(false, error_msg); }); @@ -1547,7 +1556,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableeErrorAutoRecover2) { ASSERT_EQ("val1", Get(Key(1))); Reopen(options); ASSERT_EQ("val1", Get(Key(1))); - Put(Key(2), "val2"); + ASSERT_OK(Put(Key(2), "val2")); s = Flush(); ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val2", Get(Key(2))); @@ -1576,7 +1585,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableeErrorAutoRecover3) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(1), "val1"); + ASSERT_OK(Put(Key(1), "val1")); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"FLushWritRetryableeErrorAutoRecover3:0", "RecoverFromRetryableBGIOError:BeforeStart"}, @@ -1600,7 +1609,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableeErrorAutoRecover3) { s = dbfull()->Resume(); ASSERT_EQ("val1", Get(Key(1))); ASSERT_EQ(s, Status::OK()); - Put(Key(2), "val2"); + ASSERT_OK(Put(Key(2), "val2")); s = Flush(); ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val2", Get(Key(2))); @@ -1632,7 +1641,7 @@ TEST_F(DBErrorHandlingFSTest, DISABLED_FLushWritRetryableeErrorAutoRecover4) { IOStatus nr_msg = IOStatus::IOError("No Retryable Fatal IO Error"); nr_msg.SetRetryable(false); - Put(Key(1), "val1"); + ASSERT_OK(Put(Key(1), "val1")); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"RecoverFromRetryableBGIOError:BeforeStart", "FLushWritRetryableeErrorAutoRecover4:0"}, @@ -1659,14 +1668,14 @@ TEST_F(DBErrorHandlingFSTest, DISABLED_FLushWritRetryableeErrorAutoRecover4) { s = dbfull()->Resume(); ASSERT_NE(s, Status::OK()); ASSERT_EQ("val1", Get(Key(1))); - Put(Key(2), "val2"); + ASSERT_OK(Put(Key(2), "val2")); s = Flush(); ASSERT_NE(s, Status::OK()); ASSERT_EQ("NOT_FOUND", Get(Key(2))); Reopen(options); ASSERT_EQ("val1", Get(Key(1))); - Put(Key(2), "val2"); + ASSERT_OK(Put(Key(2), "val2")); s = Flush(); ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val2", Get(Key(2))); @@ -1696,10 +1705,8 @@ TEST_F(DBErrorHandlingFSTest, DISABLED_FLushWritRetryableeErrorAutoRecover5) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - IOStatus nr_msg = IOStatus::IOError("No Retryable Fatal IO Error"); - nr_msg.SetRetryable(false); - Put(Key(1), "val1"); + ASSERT_OK(Put(Key(1), "val1")); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"RecoverFromRetryableBGIOError:BeforeStart", "FLushWritRetryableeErrorAutoRecover5:0"}}); @@ -1720,7 +1727,7 @@ TEST_F(DBErrorHandlingFSTest, DISABLED_FLushWritRetryableeErrorAutoRecover5) { Reopen(options); ASSERT_NE("val1", Get(Key(1))); - Put(Key(2), "val2"); + ASSERT_OK(Put(Key(2), "val2")); s = Flush(); ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val2", Get(Key(2))); @@ -1750,10 +1757,8 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableeErrorAutoRecover6) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - IOStatus nr_msg = IOStatus::IOError("No Retryable Fatal IO Error"); - nr_msg.SetRetryable(false); - Put(Key(1), "val1"); + ASSERT_OK(Put(Key(1), "val1")); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"FLushWritRetryableeErrorAutoRecover6:0", "RecoverFromRetryableBGIOError:BeforeStart"}, @@ -1783,7 +1788,7 @@ TEST_F(DBErrorHandlingFSTest, FLushWritRetryableeErrorAutoRecover6) { Reopen(options); ASSERT_EQ("val1", Get(Key(1))); - Put(Key(2), "val2"); + ASSERT_OK(Put(Key(2), "val2")); s = Flush(); ASSERT_EQ(s, Status::OK()); ASSERT_EQ("val2", Get(Key(2))); @@ -1815,9 +1820,9 @@ TEST_F(DBErrorHandlingFSTest, ManifestWriteRetryableErrorAutoRecover) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(0), "val"); - Flush(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(0), "val")); + ASSERT_OK(Flush()); + ASSERT_OK(Put(Key(1), "val")); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"RecoverFromRetryableBGIOError:BeforeStart", "ManifestWriteRetryableErrorAutoRecover:0"}, @@ -1871,8 +1876,8 @@ TEST_F(DBErrorHandlingFSTest, IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(0), "val"); - Put(Key(2), "val"); + ASSERT_OK(Put(Key(0), "val")); + ASSERT_OK(Put(Key(2), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -1909,7 +1914,7 @@ TEST_F(DBErrorHandlingFSTest, }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -1961,8 +1966,8 @@ TEST_F(DBErrorHandlingFSTest, CompactionWriteRetryableErrorAutoRecover) { IOStatus error_msg = IOStatus::IOError("Retryable IO Error"); error_msg.SetRetryable(true); - Put(Key(0), "va;"); - Put(Key(2), "va;"); + ASSERT_OK(Put(Key(0), "va;")); + ASSERT_OK(Put(Key(2), "va;")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -1987,7 +1992,7 @@ TEST_F(DBErrorHandlingFSTest, CompactionWriteRetryableErrorAutoRecover) { }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -2027,7 +2032,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableErrorAutoRecover1) { WriteBatch batch; for (auto i = 0; i < 100; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; @@ -2042,7 +2047,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableErrorAutoRecover1) { int write_error = 0; for (auto i = 100; i < 200; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"RecoverFromRetryableBGIOError:BeforeResume0", "WALWriteError1:0"}, @@ -2084,7 +2089,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableErrorAutoRecover1) { WriteBatch batch; for (auto i = 200; i < 300; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; @@ -2131,7 +2136,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableErrorAutoRecover2) { WriteBatch batch; for (auto i = 0; i < 100; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; @@ -2146,7 +2151,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableErrorAutoRecover2) { int write_error = 0; for (auto i = 100; i < 200; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( {{"RecoverFromRetryableBGIOError:BeforeWait0", "WALWriteError2:0"}, @@ -2188,7 +2193,7 @@ TEST_F(DBErrorHandlingFSTest, WALWriteRetryableErrorAutoRecover2) { WriteBatch batch; for (auto i = 200; i < 300; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; @@ -2226,7 +2231,7 @@ TEST_P(DBErrorHandlingFencingTest, FLushWriteFenced) { listener->EnableAutoRecovery(true); DestroyAndReopen(options); - Put(Key(0), "val"); + ASSERT_OK(Put(Key(0), "val")); SyncPoint::GetInstance()->SetCallBack("FlushJob::Start", [&](void*) { fault_fs->SetFilesystemActive(false, IOStatus::IOFenced("IO fenced")); }); @@ -2260,9 +2265,9 @@ TEST_P(DBErrorHandlingFencingTest, ManifestWriteFenced) { DestroyAndReopen(options); old_manifest = GetManifestNameFromLiveFiles(); - Put(Key(0), "val"); + ASSERT_OK(Put(Key(0), "val")); Flush(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); SyncPoint::GetInstance()->SetCallBack( "VersionSet::LogAndApply:WriteManifest", [&](void*) { fault_fs->SetFilesystemActive(false, IOStatus::IOFenced("IO fenced")); @@ -2294,8 +2299,8 @@ TEST_P(DBErrorHandlingFencingTest, CompactionWriteFenced) { Status s; DestroyAndReopen(options); - Put(Key(0), "va;"); - Put(Key(2), "va;"); + ASSERT_OK(Put(Key(0), "va;")); + ASSERT_OK(Put(Key(2), "va;")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -2309,7 +2314,7 @@ TEST_P(DBErrorHandlingFencingTest, CompactionWriteFenced) { }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - Put(Key(1), "val"); + ASSERT_OK(Put(Key(1), "val")); s = Flush(); ASSERT_EQ(s, Status::OK()); @@ -2345,7 +2350,7 @@ TEST_P(DBErrorHandlingFencingTest, WALWriteFenced) { WriteBatch batch; for (auto i = 0; i < 100; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; @@ -2358,7 +2363,7 @@ TEST_P(DBErrorHandlingFencingTest, WALWriteFenced) { int write_error = 0; for (auto i = 100; i < 199; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } SyncPoint::GetInstance()->SetCallBack( @@ -2381,7 +2386,7 @@ TEST_P(DBErrorHandlingFencingTest, WALWriteFenced) { WriteBatch batch; for (auto i = 0; i < 100; ++i) { - batch.Put(Key(i), rnd.RandomString(1024)); + ASSERT_OK(batch.Put(Key(i), rnd.RandomString(1024))); } WriteOptions wopts; diff --git a/db/event_helpers.cc b/db/event_helpers.cc index 968f18f08..b9fa35e33 100644 --- a/db/event_helpers.cc +++ b/db/event_helpers.cc @@ -51,6 +51,7 @@ void EventHelpers::NotifyOnBackgroundError( db_mutex->Unlock(); for (auto& listener : listeners) { listener->OnBackgroundError(reason, bg_error); + bg_error->PermitUncheckedError(); if (*auto_recovery) { listener->OnErrorRecoveryBegin(reason, *bg_error, auto_recovery); } @@ -221,6 +222,7 @@ void EventHelpers::NotifyOnErrorRecoveryCompleted( for (auto& listener : listeners) { listener->OnErrorRecoveryCompleted(old_bg_error); } + old_bg_error.PermitUncheckedError(); db_mutex->Lock(); #else (void)listeners; diff --git a/db/version_set.cc b/db/version_set.cc index 4eef86cb7..7b0413943 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -4134,7 +4134,8 @@ Status VersionSet::ProcessManifestWrites( "\n", manifest_file_number_, pending_manifest_file_number_); env_->DeleteFile( - DescriptorFileName(dbname_, pending_manifest_file_number_)); + DescriptorFileName(dbname_, pending_manifest_file_number_)) + .PermitUncheckedError(); } } diff --git a/db/write_thread.h b/db/write_thread.h index 800712569..8c2d431dc 100644 --- a/db/write_thread.h +++ b/db/write_thread.h @@ -75,6 +75,7 @@ class WriteThread { struct Writer; struct WriteGroup { + ~WriteGroup() { status.PermitUncheckedError(); } Writer* leader = nullptr; Writer* last_writer = nullptr; SequenceNumber last_sequence; diff --git a/utilities/fault_injection_env.h b/utilities/fault_injection_env.h index 30e900328..4629c0ab9 100644 --- a/utilities/fault_injection_env.h +++ b/utilities/fault_injection_env.h @@ -217,12 +217,14 @@ class FaultInjectionTestEnv : public EnvWrapper { if (!active) { error_ = error; } + error.PermitUncheckedError(); } void SetFilesystemActive(bool active, Status error = Status::Corruption("Not active")) { error.PermitUncheckedError(); MutexLock l(&mutex_); SetFilesystemActiveNoLock(active, error); + error.PermitUncheckedError(); } void AssertNoOpenFile() { assert(open_files_.empty()); } Status GetError() { return error_; } diff --git a/utilities/fault_injection_fs.h b/utilities/fault_injection_fs.h index 4c4a1612b..737ce2379 100644 --- a/utilities/fault_injection_fs.h +++ b/utilities/fault_injection_fs.h @@ -214,12 +214,13 @@ class FaultInjectionTestFS : public FileSystemWrapper { virtual IOStatus GetFreeSpace(const std::string& path, const IOOptions& options, uint64_t* disk_free, IODebugContext* dbg) override { + IOStatus io_s; if (!IsFilesystemActive() && error_ == IOStatus::NoSpace()) { *disk_free = 0; - return IOStatus::OK(); } else { - return target()->GetFreeSpace(path, options, disk_free, dbg); + io_s = target()->GetFreeSpace(path, options, disk_free, dbg); } + return io_s; } void WritableFileClosed(const FSFileState& state); @@ -262,6 +263,7 @@ class FaultInjectionTestFS : public FileSystemWrapper { } void SetFilesystemActiveNoLock( bool active, IOStatus error = IOStatus::Corruption("Not active")) { + error.PermitUncheckedError(); filesystem_active_ = active; if (!active) { error_ = error; @@ -270,6 +272,7 @@ class FaultInjectionTestFS : public FileSystemWrapper { void SetFilesystemActive( bool active, IOStatus error = IOStatus::Corruption("Not active")) { MutexLock l(&mutex_); + error.PermitUncheckedError(); SetFilesystemActiveNoLock(active, error); } void SetFilesystemDirectWritable( @@ -283,6 +286,7 @@ class FaultInjectionTestFS : public FileSystemWrapper { void SetFileSystemIOError(IOStatus io_error) { MutexLock l(&mutex_); + io_error.PermitUncheckedError(); error_ = io_error; }