diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index 50f9a8ca5..6303ed5f2 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -2310,7 +2310,7 @@ class DBImpl : public DB { // logfile_number_. With two_write_queues it also protects alive_log_files_, // and log_empty_. Refer to the definition of each variable below for more // details. - // Note: to avoid dealock, if needed to acquire both log_write_mutex_ and + // Note: to avoid deadlock, if needed to acquire both log_write_mutex_ and // mutex_, the order should be first mutex_ and then log_write_mutex_. InstrumentedMutex log_write_mutex_; diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index 06cc2e6a8..b8ab5eb7e 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/db/db_impl/db_impl_compaction_flush.cc @@ -3569,7 +3569,7 @@ Status DBImpl::BackgroundCompaction(bool* made_progress, mutex_.Unlock(); TEST_SYNC_POINT_CALLBACK( "DBImpl::BackgroundCompaction:NonTrivial:BeforeRun", nullptr); - // Should handle erorr? + // Should handle error? compaction_job.Run().PermitUncheckedError(); TEST_SYNC_POINT("DBImpl::BackgroundCompaction:NonTrivial:AfterRun"); mutex_.Lock(); diff --git a/db/db_impl/db_impl_files.cc b/db/db_impl/db_impl_files.cc index 414c1270c..0998c9455 100644 --- a/db/db_impl/db_impl_files.cc +++ b/db/db_impl/db_impl_files.cc @@ -286,6 +286,7 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force, return; } + bool mutex_unlocked = false; if (!alive_log_files_.empty() && !logs_.empty()) { uint64_t min_log_number = job_context->log_number; size_t num_alive_log_files = alive_log_files_.size(); @@ -315,6 +316,7 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force, } log_write_mutex_.Unlock(); mutex_.Unlock(); + mutex_unlocked = true; TEST_SYNC_POINT_CALLBACK("FindObsoleteFiles::PostMutexUnlock", nullptr); log_write_mutex_.Lock(); while (!logs_.empty() && logs_.front().number < min_log_number) { @@ -337,7 +339,9 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force, logs_to_free_.clear(); log_write_mutex_.Unlock(); - mutex_.Lock(); + if (mutex_unlocked) { + mutex_.Lock(); + } job_context->log_recycle_files.assign(log_recycle_files_.begin(), log_recycle_files_.end()); } diff --git a/db/db_test_util.cc b/db/db_test_util.cc index f169034fc..6c4a1db94 100644 --- a/db/db_test_util.cc +++ b/db/db_test_util.cc @@ -1074,6 +1074,24 @@ size_t DBTestBase::TotalLiveFiles(int cf) { return num_files; } +size_t DBTestBase::TotalLiveFilesAtPath(int cf, const std::string& path) { + ColumnFamilyMetaData cf_meta; + if (cf == 0) { + db_->GetColumnFamilyMetaData(&cf_meta); + } else { + db_->GetColumnFamilyMetaData(handles_[cf], &cf_meta); + } + size_t num_files = 0; + for (auto& level : cf_meta.levels) { + for (auto& f : level.files) { + if (f.directory == path) { + num_files++; + } + } + } + return num_files; +} + size_t DBTestBase::CountLiveFiles() { std::vector metadata; db_->GetLiveFilesMetaData(&metadata); diff --git a/db/db_test_util.h b/db/db_test_util.h index a4986d665..50bfc78ee 100644 --- a/db/db_test_util.h +++ b/db/db_test_util.h @@ -1192,6 +1192,8 @@ class DBTestBase : public testing::Test { size_t TotalLiveFiles(int cf = 0); + size_t TotalLiveFilesAtPath(int cf, const std::string& path); + size_t CountLiveFiles(); int NumTableFilesAtLevel(int level, int cf = 0); diff --git a/db/db_universal_compaction_test.cc b/db/db_universal_compaction_test.cc index bb6b67d9b..84f01b3d1 100644 --- a/db/db_universal_compaction_test.cc +++ b/db/db_universal_compaction_test.cc @@ -1850,31 +1850,31 @@ TEST_P(DBTestUniversalManualCompactionOutputPathId, compact_options.exclusive_manual_compaction = exclusive_manual_compaction_; ASSERT_OK(db_->CompactRange(compact_options, handles_[1], nullptr, nullptr)); ASSERT_EQ(1, TotalLiveFiles(1)); - ASSERT_EQ(0, GetSstFileCount(options.db_paths[0].path)); - ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path)); + ASSERT_EQ(0, TotalLiveFilesAtPath(1, options.db_paths[0].path)); + ASSERT_EQ(1, TotalLiveFilesAtPath(1, options.db_paths[1].path)); ReopenWithColumnFamilies({kDefaultColumnFamilyName, "pikachu"}, options); ASSERT_EQ(1, TotalLiveFiles(1)); - ASSERT_EQ(0, GetSstFileCount(options.db_paths[0].path)); - ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path)); + ASSERT_EQ(0, TotalLiveFilesAtPath(1, options.db_paths[0].path)); + ASSERT_EQ(1, TotalLiveFilesAtPath(1, options.db_paths[1].path)); MakeTables(1, "p", "q", 1); ASSERT_EQ(2, TotalLiveFiles(1)); - ASSERT_EQ(1, GetSstFileCount(options.db_paths[0].path)); - ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path)); + ASSERT_EQ(1, TotalLiveFilesAtPath(1, options.db_paths[0].path)); + ASSERT_EQ(1, TotalLiveFilesAtPath(1, options.db_paths[1].path)); ReopenWithColumnFamilies({kDefaultColumnFamilyName, "pikachu"}, options); ASSERT_EQ(2, TotalLiveFiles(1)); - ASSERT_EQ(1, GetSstFileCount(options.db_paths[0].path)); - ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path)); + ASSERT_EQ(1, TotalLiveFilesAtPath(1, options.db_paths[0].path)); + ASSERT_EQ(1, TotalLiveFilesAtPath(1, options.db_paths[1].path)); // Full compaction to DB path 0 compact_options.target_path_id = 0; compact_options.exclusive_manual_compaction = exclusive_manual_compaction_; ASSERT_OK(db_->CompactRange(compact_options, handles_[1], nullptr, nullptr)); ASSERT_EQ(1, TotalLiveFiles(1)); - ASSERT_EQ(1, GetSstFileCount(options.db_paths[0].path)); - ASSERT_EQ(0, GetSstFileCount(options.db_paths[1].path)); + ASSERT_EQ(1, TotalLiveFilesAtPath(1, options.db_paths[0].path)); + ASSERT_EQ(0, TotalLiveFilesAtPath(1, options.db_paths[1].path)); // Fail when compacting to an invalid path ID compact_options.target_path_id = 2;