From 4d51feab0bc0d0a209a0606afa6b2857737bb0f5 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 22 Mar 2018 15:56:52 -0700 Subject: [PATCH] Rename function for handling WAL write error Summary: It was misnamed. It actually updates `bg_error_` if `PreprocessWrite()` or `WriteToWAL()` fail, not related to the user callback. Closes https://github.com/facebook/rocksdb/pull/3485 Differential Revision: D6955787 Pulled By: ajkr fbshipit-source-id: bd7afc3fdb7a52830c021cbfc25fcbc3ab7d5e10 --- db/db_impl.h | 2 +- db/db_impl_write.cc | 8 ++++---- db/db_write_test.cc | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/db/db_impl.h b/db/db_impl.h index c97d407bf..0089ece15 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -875,7 +875,7 @@ class DBImpl : public DB { size_t seq_inc); // Used by WriteImpl to update bg_error_ if paranoid check is enabled. - void WriteCallbackStatusCheck(const Status& status); + void WriteStatusCheck(const Status& status); // Used by WriteImpl to update bg_error_ in case of memtable insert error. void MemTableInsertStatusCheck(const Status& memtable_insert_status); diff --git a/db/db_impl_write.cc b/db/db_impl_write.cc index 8b26ac511..ee66a9a8b 100644 --- a/db/db_impl_write.cc +++ b/db/db_impl_write.cc @@ -340,7 +340,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options, PERF_TIMER_START(write_pre_and_post_process_time); if (!w.CallbackFailed()) { - WriteCallbackStatusCheck(status); + WriteStatusCheck(status); } if (need_log_sync) { @@ -464,7 +464,7 @@ Status DBImpl::PipelinedWriteImpl(const WriteOptions& write_options, } if (!w.CallbackFailed()) { - WriteCallbackStatusCheck(w.status); + WriteStatusCheck(w.status); } if (need_log_sync) { @@ -625,7 +625,7 @@ Status DBImpl::WriteImplWALOnly(const WriteOptions& write_options, PERF_TIMER_START(write_pre_and_post_process_time); if (!w.CallbackFailed()) { - WriteCallbackStatusCheck(status); + WriteStatusCheck(status); } if (status.ok()) { for (auto* writer : write_group) { @@ -651,7 +651,7 @@ Status DBImpl::WriteImplWALOnly(const WriteOptions& write_options, return status; } -void DBImpl::WriteCallbackStatusCheck(const Status& status) { +void DBImpl::WriteStatusCheck(const Status& status) { // Is setting bg_error_ enough here? This will at least stop // compaction and fail any further writes. if (immutable_db_options_.paranoid_checks && !status.ok() && diff --git a/db/db_write_test.cc b/db/db_write_test.cc index 1a27f470e..917aef550 100644 --- a/db/db_write_test.cc +++ b/db/db_write_test.cc @@ -80,6 +80,22 @@ TEST_P(DBWriteTest, IOErrorOnWALWritePropagateToWriteThreadFollower) { Close(); } +TEST_P(DBWriteTest, IOErrorOnWALWriteTriggersReadOnlyMode) { + std::unique_ptr mock_env( + new FaultInjectionTestEnv(Env::Default())); + Options options = GetOptions(); + options.env = mock_env.get(); + Reopen(options); + for (int i = 0; i < 2; i++) { + // Forcibly fail WAL write for the first Put only. Subsequent Puts should + // fail due to read-only mode + mock_env->SetFilesystemActive(i != 0); + ASSERT_FALSE(Put("key" + ToString(i), "value").ok()); + } + // Close before mock_env destruct. + Close(); +} + INSTANTIATE_TEST_CASE_P(DBWriteTestInstance, DBWriteTest, testing::Values(DBTestBase::kDefault, DBTestBase::kConcurrentWALWrites,