diff --git a/HISTORY.md b/HISTORY.md index 408a4ce818b874e7bd02f5c04192b0feeb6e471e..6fa5dc855ccad72ac628c14a4ace96920cf3d3dc 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -15,6 +15,7 @@ ### Bug Fixes * Delete an empty WAL file on DB open if the log number is less than the min log number to keep +* Delete temp OPTIONS file on DB open if there is a failure to write it out or rename it ### Performance Improvements * Improved the I/O efficiency of prefetching SST metadata by recording more information in the DB manifest. Opening files written with previous versions will still rely on heuristics for how much to prefetch (#11406). diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index bf4f9232a08f6a11fbd377a4df409e45fb0d5a42..07aee535db5fa4f6becd751188bb2c3fc9871bbc 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -4913,6 +4913,15 @@ Status DBImpl::WriteOptionsFile(bool need_mutex_lock, if (s.ok()) { s = RenameTempFileToOptionsFile(file_name); } + + if (!s.ok() && GetEnv()->FileExists(file_name).ok()) { + if (!GetEnv()->DeleteFile(file_name).ok()) { + ROCKS_LOG_WARN(immutable_db_options_.info_log, + "Unable to delete temp options file %s", + file_name.c_str()); + } + } + // restore lock if (!need_mutex_lock) { mutex_.Lock(); diff --git a/db/db_options_test.cc b/db/db_options_test.cc index 777e2fe2216a6c5544b84ca3a703ffa8679337b4..729afdf3db9a922ced2a89e13cc44627d6c5b94d 100644 --- a/db/db_options_test.cc +++ b/db/db_options_test.cc @@ -22,6 +22,7 @@ #include "test_util/sync_point.h" #include "test_util/testutil.h" #include "util/random.h" +#include "utilities/fault_injection_fs.h" namespace ROCKSDB_NAMESPACE { @@ -1287,6 +1288,39 @@ TEST_F(DBOptionsTest, FIFOTemperatureAgeThresholdValidation) { "supported when num_levels = 1.")); } +TEST_F(DBOptionsTest, TempOptionsFailTest) { + std::shared_ptr fs; + std::unique_ptr env; + + fs.reset(new FaultInjectionTestFS(env_->GetFileSystem())); + env = NewCompositeEnv(fs); + Options options = CurrentOptions(); + options.env = env.get(); + + SyncPoint::GetInstance()->SetCallBack( + "PersistRocksDBOptions:create", + [&](void* /*arg*/) { fs->SetFilesystemActive(false); }); + SyncPoint::GetInstance()->SetCallBack( + "PersistRocksDBOptions:written", + [&](void* /*arg*/) { fs->SetFilesystemActive(true); }); + + SyncPoint::GetInstance()->EnableProcessing(); + TryReopen(options); + SyncPoint::GetInstance()->DisableProcessing(); + + std::vector filenames; + ASSERT_OK(env_->GetChildren(dbname_, &filenames)); + uint64_t number; + FileType type; + bool found_temp_file = false; + for (size_t i = 0; i < filenames.size(); i++) { + if (ParseFileName(filenames[i], &number, &type) && type == kTempFile) { + found_temp_file = true; + } + } + ASSERT_FALSE(found_temp_file); +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) { diff --git a/options/options_parser.cc b/options/options_parser.cc index d423f76ff9776c1b57a18f105163d26b767d382b..a8c855d6e22afdce02fff2f86a6851813b4dc7b7 100644 --- a/options/options_parser.cc +++ b/options/options_parser.cc @@ -75,6 +75,7 @@ Status PersistRocksDBOptions(const ConfigOptions& config_options_in, std::unique_ptr writable; writable.reset(new WritableFileWriter(std::move(wf), file_name, EnvOptions(), nullptr /* statistics */)); + TEST_SYNC_POINT("PersistRocksDBOptions:create"); std::string options_file_content; @@ -135,6 +136,7 @@ Status PersistRocksDBOptions(const ConfigOptions& config_options_in, if (s.ok()) { s = writable->Close(); } + TEST_SYNC_POINT("PersistRocksDBOptions:written"); if (s.ok()) { return RocksDBOptionsParser::VerifyRocksDBOptionsFromFile( config_options, db_opt, cf_names, cf_opts, file_name, fs);