diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index df75c7c04..8d25659a8 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -235,7 +235,8 @@ DBImpl::DBImpl(const DBOptions& options, const std::string& dbname, // !batch_per_trx_ implies seq_per_batch_ because it is only unset for // WriteUnprepared, which should use seq_per_batch_. assert(batch_per_txn_ || seq_per_batch_); - env_->GetAbsolutePath(dbname, &db_absolute_path_); + // TODO: Check for an error here + env_->GetAbsolutePath(dbname, &db_absolute_path_).PermitUncheckedError(); // Reserve ten files or so for other uses and give the rest to TableCache. // Give a large number for setting of "infinite" open files. @@ -501,7 +502,7 @@ Status DBImpl::CloseHelper() { env_->UnSchedule(this, Env::Priority::BOTTOM); int compactions_unscheduled = env_->UnSchedule(this, Env::Priority::LOW); int flushes_unscheduled = env_->UnSchedule(this, Env::Priority::HIGH); - Status ret; + Status ret = Status::OK(); mutex_.Lock(); bg_bottom_compaction_scheduled_ -= bottom_compactions_unscheduled; bg_compaction_scheduled_ -= compactions_unscheduled; @@ -613,7 +614,8 @@ Status DBImpl::CloseHelper() { versions_.reset(); mutex_.Unlock(); if (db_lock_ != nullptr) { - env_->UnlockFile(db_lock_); + // TODO: Check for unlock error + env_->UnlockFile(db_lock_).PermitUncheckedError(); } ROCKS_LOG_INFO(immutable_db_options_.info_log, "Shutdown complete"); @@ -632,7 +634,7 @@ Status DBImpl::CloseHelper() { if (immutable_db_options_.info_log && own_info_log_) { Status s = immutable_db_options_.info_log->Close(); - if (ret.ok()) { + if (!s.ok() && ret.ok()) { ret = s; } } @@ -651,7 +653,7 @@ Status DBImpl::CloseImpl() { return CloseHelper(); } DBImpl::~DBImpl() { if (!closed_) { closed_ = true; - CloseHelper(); + CloseHelper().PermitUncheckedError(); } } @@ -3769,7 +3771,7 @@ Status DestroyDB(const std::string& dbname, const Options& options, // log file and prevents cleanup and directory removal soptions.info_log.reset(); // Ignore error in case directory does not exist - env->GetChildren(dbname, &filenames); + env->GetChildren(dbname, &filenames).PermitUncheckedError(); FileLock* lock; const std::string lockname = LockFileName(dbname); @@ -3791,7 +3793,7 @@ Status DestroyDB(const std::string& dbname, const Options& options, } else { del = env->DeleteFile(path_to_delete); } - if (result.ok() && !del.ok()) { + if (!del.ok() && result.ok()) { result = del; } } @@ -3814,7 +3816,7 @@ Status DestroyDB(const std::string& dbname, const Options& options, std::string table_path = path + "/" + fname; Status del = DeleteDBFile(&soptions, table_path, dbname, /*force_bg=*/false, /*force_fg=*/false); - if (result.ok() && !del.ok()) { + if (!del.ok() && result.ok()) { result = del; } } @@ -3842,12 +3844,13 @@ Status DestroyDB(const std::string& dbname, const Options& options, Status del = DeleteDBFile(&soptions, archivedir + "/" + file, archivedir, /*force_bg=*/false, /*force_fg=*/!wal_in_db_path); - if (result.ok() && !del.ok()) { + if (!del.ok() && result.ok()) { result = del; } } } - env->DeleteDir(archivedir); + // TODO: Should we check for errors here? + env->DeleteDir(archivedir).PermitUncheckedError(); } // Delete log files in the WAL dir @@ -3858,7 +3861,7 @@ Status DestroyDB(const std::string& dbname, const Options& options, DeleteDBFile(&soptions, LogFileName(soptions.wal_dir, number), soptions.wal_dir, /*force_bg=*/false, /*force_fg=*/!wal_in_db_path); - if (result.ok() && !del.ok()) { + if (!del.ok() && result.ok()) { result = del; } } @@ -3866,14 +3869,17 @@ Status DestroyDB(const std::string& dbname, const Options& options, env->DeleteDir(soptions.wal_dir); } - env->UnlockFile(lock); // Ignore error since state is already gone - env->DeleteFile(lockname); + // Ignore error since state is already gone + env->UnlockFile(lock).PermitUncheckedError(); + env->DeleteFile(lockname).PermitUncheckedError(); // sst_file_manager holds a ref to the logger. Make sure the logger is // gone before trying to remove the directory. soptions.sst_file_manager.reset(); - env->DeleteDir(dbname); // Ignore error in case dir contains other files + // Ignore error in case dir contains other files + env->DeleteDir(dbname).PermitUncheckedError(); + ; } return result; } @@ -4008,7 +4014,8 @@ Status DBImpl::RenameTempFileToOptionsFile(const std::string& file_name) { } if (0 == disable_delete_obsolete_files_) { - DeleteObsoleteOptionsFiles(); + // TODO: Should we check for errors here? + DeleteObsoleteOptionsFiles().PermitUncheckedError(); } return s; #else diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index b0d7d401f..73f6f79c0 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/db/db_impl/db_impl_compaction_flush.cc @@ -167,7 +167,7 @@ Status DBImpl::FlushMemTableToOutputFile( #endif // ROCKSDB_LITE Status s; - IOStatus io_s; + IOStatus io_s = IOStatus::OK(); if (logfile_number_ > 0 && versions_->GetColumnFamilySet()->NumberOfColumnFamilies() > 1) { // If there are more than one column families, we need to make sure that @@ -225,6 +225,10 @@ Status DBImpl::FlushMemTableToOutputFile( Status new_bg_error = s; error_handler_.SetBGError(new_bg_error, BackgroundErrorReason::kFlush); } + } else { + // If we got here, then we decided not to care about the i_os status (either + // from never needing it or ignoring the flush job status + io_s.PermitUncheckedError(); } if (s.ok()) { #ifndef ROCKSDB_LITE diff --git a/db/db_impl/db_impl_files.cc b/db/db_impl/db_impl_files.cc index 5fd769776..7e51d2c3c 100644 --- a/db/db_impl/db_impl_files.cc +++ b/db/db_impl/db_impl_files.cc @@ -447,7 +447,8 @@ void DBImpl::PurgeObsoleteFiles(JobContext& state, bool schedule_only) { // Close WALs before trying to delete them. for (const auto w : state.logs_to_free) { // TODO: maybe check the return value of Close. - w->Close(); + auto s = w->Close(); + s.PermitUncheckedError(); } bool own_files = OwnTablesAndLogs(); @@ -566,7 +567,6 @@ void DBImpl::PurgeObsoleteFiles(JobContext& state, bool schedule_only) { if (!own_files) { continue; } - Status file_deletion_status; if (schedule_only) { InstrumentedMutexLock guard_lock(&mutex_); SchedulePendingPurge(fname, dir_to_sync, type, number, state.job_id); diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index 75e823972..5978839ba 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -1684,7 +1684,9 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname, paths.erase(std::unique(paths.begin(), paths.end()), paths.end()); for (auto& path : paths) { std::vector existing_files; - impl->immutable_db_options_.env->GetChildren(path, &existing_files); + // TODO: Check for errors here? + impl->immutable_db_options_.env->GetChildren(path, &existing_files) + .PermitUncheckedError(); for (auto& file_name : existing_files) { uint64_t file_number; FileType file_type; diff --git a/db/error_handler.h b/db/error_handler.h index a2f1a7ec0..6ede5559e 100644 --- a/db/error_handler.h +++ b/db/error_handler.h @@ -29,7 +29,11 @@ class ErrorHandler { db_mutex_(db_mutex), auto_recovery_(false), recovery_in_prog_(false) {} - ~ErrorHandler() {} + ~ErrorHandler() { + bg_error_.PermitUncheckedError(); + recovery_error_.PermitUncheckedError(); + recovery_io_error_.PermitUncheckedError(); + } void EnableAutoRecovery() { auto_recovery_ = true; } diff --git a/db/version_set.cc b/db/version_set.cc index 1c8242c23..bc446dc03 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -3563,6 +3563,7 @@ struct VersionSet::ManifestWriter { cfd(_cfd), mutable_cf_options(cf_options), edit_list(e) {} + ~ManifestWriter() { status.PermitUncheckedError(); } }; Status AtomicGroupReadBuffer::AddEdit(VersionEdit* edit) { diff --git a/db/write_batch.cc b/db/write_batch.cc index 76f59d55a..0d0efc5c0 100644 --- a/db/write_batch.cc +++ b/db/write_batch.cc @@ -1428,6 +1428,7 @@ class MemTableInserter : public WriteBatch::Handler { MaybeAdvanceSeq(batch_boundry); return seek_status; } + seek_status.PermitUncheckedError(); // Ignore errors Status ret_status; MemTable* mem = cf_mems_->GetMemTable(); diff --git a/db/write_thread.h b/db/write_thread.h index 878199714..800712569 100644 --- a/db/write_thread.h +++ b/db/write_thread.h @@ -179,6 +179,8 @@ class WriteThread { StateMutex().~mutex(); StateCV().~condition_variable(); } + status.PermitUncheckedError(); + callback_status.PermitUncheckedError(); } bool CheckCallback(DB* db) { diff --git a/env/env_test.cc b/env/env_test.cc index dae14f015..1caa29453 100644 --- a/env/env_test.cc +++ b/env/env_test.cc @@ -2111,8 +2111,6 @@ class EnvFSTestWithParam std::string dbname2_; }; -#ifndef ROCKSDB_ASSERT_STATUS_CHECKED // Database tests do not do well with - // this flag TEST_P(EnvFSTestWithParam, OptionsTest) { Options opts; opts.env = env_; @@ -2132,11 +2130,11 @@ TEST_P(EnvFSTestWithParam, OptionsTest) { ASSERT_OK(s); WriteOptions wo; - db->Put(wo, "a", "a"); - db->Flush(FlushOptions()); - db->Put(wo, "b", "b"); - db->Flush(FlushOptions()); - db->CompactRange(CompactRangeOptions(), nullptr, nullptr); + ASSERT_OK(db->Put(wo, "a", "a")); + ASSERT_OK(db->Flush(FlushOptions())); + ASSERT_OK(db->Put(wo, "b", "b")); + ASSERT_OK(db->Flush(FlushOptions())); + ASSERT_OK(db->CompactRange(CompactRangeOptions(), nullptr, nullptr)); std::string val; ASSERT_OK(db->Get(ReadOptions(), "a", &val)); @@ -2144,14 +2142,13 @@ TEST_P(EnvFSTestWithParam, OptionsTest) { ASSERT_OK(db->Get(ReadOptions(), "b", &val)); ASSERT_EQ("b", val); - db->Close(); + ASSERT_OK(db->Close()); delete db; DestroyDB(dbname, opts); dbname = dbname2_; } } -#endif // ROCKSDB_ASSERT_STATUS_CHECKED // The parameters are as follows - // 1. True means Options::env is non-null, false means null diff --git a/file/delete_scheduler.cc b/file/delete_scheduler.cc index 475d8883e..5a032837e 100644 --- a/file/delete_scheduler.cc +++ b/file/delete_scheduler.cc @@ -65,7 +65,7 @@ Status DeleteScheduler::DeleteFile(const std::string& file_path, TEST_SYNC_POINT("DeleteScheduler::DeleteFile"); s = fs_->DeleteFile(file_path, IOOptions(), nullptr); if (s.ok()) { - sst_file_manager_->OnDeleteFile(file_path); + s = sst_file_manager_->OnDeleteFile(file_path); ROCKS_LOG_INFO(info_log_, "Deleted file %s immediately, rate_bytes_per_sec %" PRIi64 ", total_trash_size %" PRIu64 " max_trash_db_ratio %lf", @@ -88,7 +88,7 @@ Status DeleteScheduler::DeleteFile(const std::string& file_path, file_path.c_str(), s.ToString().c_str()); s = fs_->DeleteFile(file_path, IOOptions(), nullptr); if (s.ok()) { - sst_file_manager_->OnDeleteFile(file_path); + s = sst_file_manager_->OnDeleteFile(file_path); ROCKS_LOG_INFO(info_log_, "Deleted file %s immediately", trash_file.c_str()); InstrumentedMutexLock l(&mu_); @@ -146,7 +146,7 @@ Status DeleteScheduler::CleanupDirectory(Env* env, SstFileManagerImpl* sfm, std::string trash_file = path + "/" + current_file; if (sfm) { // We have an SstFileManager that will schedule the file delete - sfm->OnAddFile(trash_file); + s = sfm->OnAddFile(trash_file); file_delete = sfm->ScheduleFileDeletion(trash_file, path); } else { // Delete the file immediately @@ -354,8 +354,10 @@ Status DeleteScheduler::DeleteTrashFile(const std::string& path_in_trash, reinterpret_cast(const_cast(&dir_to_sync))); } } - *deleted_bytes = file_size; - sst_file_manager_->OnDeleteFile(path_in_trash); + if (s.ok()) { + *deleted_bytes = file_size; + s = sst_file_manager_->OnDeleteFile(path_in_trash); + } } } if (!s.ok()) { diff --git a/file/sst_file_manager_impl.cc b/file/sst_file_manager_impl.cc index 494deaf63..baf58d6b8 100644 --- a/file/sst_file_manager_impl.cc +++ b/file/sst_file_manager_impl.cc @@ -43,6 +43,7 @@ SstFileManagerImpl::SstFileManagerImpl(Env* env, std::shared_ptr fs, SstFileManagerImpl::~SstFileManagerImpl() { Close(); + bg_err_.PermitUncheckedError(); } void SstFileManagerImpl::Close() { @@ -183,12 +184,13 @@ bool SstFileManagerImpl::EnoughRoomForCompaction( // seen a NoSpace() error. This is tin order to contain a single potentially // misbehaving DB instance and prevent it from slowing down compactions of // other DB instances - if (CheckFreeSpace() && bg_error == Status::NoSpace()) { + if (bg_error == Status::NoSpace() && CheckFreeSpace()) { auto fn = TableFileName(cfd->ioptions()->cf_paths, inputs[0][0]->fd.GetNumber(), inputs[0][0]->fd.GetPathId()); uint64_t free_space = 0; - fs_->GetFreeSpace(fn, IOOptions(), &free_space, nullptr); + Status s = fs_->GetFreeSpace(fn, IOOptions(), &free_space, nullptr); + s.PermitUncheckedError(); // TODO: Check the status // needed_headroom is based on current size reserved by compactions, // minus any files created by running compactions as they would count // against the reserved size. If user didn't specify any compaction diff --git a/include/rocksdb/env.h b/include/rocksdb/env.h index 4af2171f0..955d591c3 100644 --- a/include/rocksdb/env.h +++ b/include/rocksdb/env.h @@ -901,8 +901,10 @@ class WritableFile { if (new_last_preallocated_block > last_preallocated_block_) { size_t num_spanned_blocks = new_last_preallocated_block - last_preallocated_block_; + // TODO: Don't ignore errors from allocate Allocate(block_size * last_preallocated_block_, - block_size * num_spanned_blocks); + block_size * num_spanned_blocks) + .PermitUncheckedError(); last_preallocated_block_ = new_last_preallocated_block; } } diff --git a/logging/auto_roll_logger.cc b/logging/auto_roll_logger.cc index fc498521b..adfe9cc89 100644 --- a/logging/auto_roll_logger.cc +++ b/logging/auto_roll_logger.cc @@ -257,11 +257,15 @@ Status CreateLoggerFromOptions(const std::string& dbname, Env* env = options.env; std::string db_absolute_path; - env->GetAbsolutePath(dbname, &db_absolute_path); + Status s = env->GetAbsolutePath(dbname, &db_absolute_path); + if (!s.ok()) { + return s; + } std::string fname = InfoLogFileName(dbname, db_absolute_path, options.db_log_dir); - env->CreateDirIfMissing(dbname); // In case it does not exist + env->CreateDirIfMissing(dbname) + .PermitUncheckedError(); // In case it does not exist // Currently we only support roll by time-to-roll and log size #ifndef ROCKSDB_LITE if (options.log_file_time_to_roll > 0 || options.max_log_file_size > 0) { @@ -269,7 +273,7 @@ Status CreateLoggerFromOptions(const std::string& dbname, env, dbname, options.db_log_dir, options.max_log_file_size, options.log_file_time_to_roll, options.keep_log_file_num, options.info_log_level); - Status s = result->GetStatus(); + s = result->GetStatus(); if (!s.ok()) { delete result; } else { @@ -281,8 +285,9 @@ Status CreateLoggerFromOptions(const std::string& dbname, // Open a log file in the same directory as the db env->RenameFile(fname, OldInfoLogFileName(dbname, env->NowMicros(), db_absolute_path, - options.db_log_dir)); - auto s = env->NewLogger(fname, logger); + options.db_log_dir)) + .PermitUncheckedError(); + s = env->NewLogger(fname, logger); if (logger->get() != nullptr) { (*logger)->SetInfoLogLevel(options.info_log_level); } diff --git a/logging/posix_logger.h b/logging/posix_logger.h index fbd297c68..0bf14c3b6 100644 --- a/logging/posix_logger.h +++ b/logging/posix_logger.h @@ -68,7 +68,7 @@ class PosixLogger : public Logger { virtual ~PosixLogger() { if (!closed_) { closed_ = true; - PosixCloseHelper(); + PosixCloseHelper().PermitUncheckedError(); } } virtual void Flush() override { diff --git a/monitoring/stats_dump_scheduler.cc b/monitoring/stats_dump_scheduler.cc index 1dd641aa3..69fb6cf39 100644 --- a/monitoring/stats_dump_scheduler.cc +++ b/monitoring/stats_dump_scheduler.cc @@ -56,7 +56,8 @@ StatsDumpScheduler* StatsDumpScheduler::Default() { std::string StatsDumpScheduler::GetTaskName(DBImpl* dbi, const std::string& func_name) { std::string db_session_id; - dbi->GetDbSessionId(db_session_id); + // TODO: Should this error be ignored? + dbi->GetDbSessionId(db_session_id).PermitUncheckedError(); return db_session_id + ":" + func_name; } diff --git a/table/block_based/block.h b/table/block_based/block.h index 651bdc739..bc712b170 100644 --- a/table/block_based/block.h +++ b/table/block_based/block.h @@ -332,6 +332,7 @@ class BlockIter : public InternalIteratorBase { // Assert that the BlockIter is never deleted while Pinning is Enabled. assert(!pinned_iters_mgr_ || (pinned_iters_mgr_ && !pinned_iters_mgr_->PinningEnabled())); + status_.PermitUncheckedError(); } void SetPinnedItersMgr(PinnedIteratorsManager* pinned_iters_mgr) override { pinned_iters_mgr_ = pinned_iters_mgr; diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index 3d827838f..73a744122 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -1729,7 +1729,7 @@ TableProperties BlockBasedTableBuilder::GetTableProperties() const { for (const auto& prop : collector->GetReadableProperties()) { ret.readable_properties.insert(prop); } - collector->Finish(&ret.user_collected_properties); + collector->Finish(&ret.user_collected_properties).PermitUncheckedError(); } return ret; } diff --git a/table/block_based/block_based_table_reader.h b/table/block_based/block_based_table_reader.h index bf47dd34d..b705489ba 100644 --- a/table/block_based/block_based_table_reader.h +++ b/table/block_based/block_based_table_reader.h @@ -520,7 +520,7 @@ struct BlockBasedTable::Rep { file_size(_file_size), level(_level), immortal_table(_immortal_table) {} - + ~Rep() { status.PermitUncheckedError(); } const ImmutableCFOptions& ioptions; const EnvOptions& env_options; const BlockBasedTableOptions table_options; diff --git a/utilities/fault_injection_env.h b/utilities/fault_injection_env.h index 0de73cbe2..ded02083f 100644 --- a/utilities/fault_injection_env.h +++ b/utilities/fault_injection_env.h @@ -122,7 +122,7 @@ class FaultInjectionTestEnv : public EnvWrapper { public: explicit FaultInjectionTestEnv(Env* base) : EnvWrapper(base), filesystem_active_(true) {} - virtual ~FaultInjectionTestEnv() {} + virtual ~FaultInjectionTestEnv() { error_.PermitUncheckedError(); } Status NewDirectory(const std::string& name, std::unique_ptr* result) override; diff --git a/utilities/fault_injection_fs.cc b/utilities/fault_injection_fs.cc index 3f7be199e..5d4f94e0c 100644 --- a/utilities/fault_injection_fs.cc +++ b/utilities/fault_injection_fs.cc @@ -86,7 +86,7 @@ TestFSWritableFile::TestFSWritableFile(const std::string& fname, TestFSWritableFile::~TestFSWritableFile() { if (writable_file_opened_) { - Close(IOOptions(), nullptr); + Close(IOOptions(), nullptr).PermitUncheckedError(); } } @@ -112,7 +112,8 @@ IOStatus TestFSWritableFile::Close(const IOOptions& options, io_s = target_->Append(state_.buffer_, options, dbg); if (io_s.ok()) { state_.buffer_.resize(0); - target_->Sync(options, dbg); + // Ignore sync errors + target_->Sync(options, dbg).PermitUncheckedError(); io_s = target_->Close(options, dbg); } if (io_s.ok()) { @@ -125,11 +126,10 @@ IOStatus TestFSWritableFile::Flush(const IOOptions&, IODebugContext*) { if (!fs_->IsFilesystemActive()) { return fs_->GetError(); } - IOStatus io_s; - if (io_s.ok() && fs_->IsFilesystemActive()) { + if (fs_->IsFilesystemActive()) { state_.pos_at_last_flush_ = state_.pos_; } - return io_s; + return IOStatus::OK(); } IOStatus TestFSWritableFile::Sync(const IOOptions& options, @@ -139,7 +139,8 @@ IOStatus TestFSWritableFile::Sync(const IOOptions& options, } IOStatus io_s = target_->Append(state_.buffer_, options, dbg); state_.buffer_.resize(0); - target_->Sync(options, dbg); + // Ignore sync errors + target_->Sync(options, dbg).PermitUncheckedError(); state_.pos_at_last_sync_ = state_.pos_; fs_->WritableFileSynced(state_); return io_s; @@ -154,7 +155,7 @@ TestFSRandomRWFile::TestFSRandomRWFile(const std::string& /*fname*/, TestFSRandomRWFile::~TestFSRandomRWFile() { if (file_opened_) { - Close(IOOptions(), nullptr); + Close(IOOptions(), nullptr).PermitUncheckedError(); } }