diff --git a/db/db_impl.cc b/db/db_impl.cc index 70a14d2c5e..67c20191c5 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -2154,7 +2154,7 @@ ColumnFamilyHandle* DBImpl::GetColumnFamilyHandle(uint32_t column_family_id) { } // REQUIRED: mutex is NOT held. -ColumnFamilyHandle* DBImpl::GetColumnFamilyHandleUnlocked( +std::unique_ptr DBImpl::GetColumnFamilyHandleUnlocked( uint32_t column_family_id) { ColumnFamilyMemTables* cf_memtables = column_family_memtables_.get(); @@ -2164,7 +2164,8 @@ ColumnFamilyHandle* DBImpl::GetColumnFamilyHandleUnlocked( return nullptr; } - return cf_memtables->GetColumnFamilyHandle(); + return std::unique_ptr( + new ColumnFamilyHandleImpl(cf_memtables->current(), this, &mutex_)); } void DBImpl::GetApproximateMemTableStats(ColumnFamilyHandle* column_family, diff --git a/db/db_impl.h b/db/db_impl.h index dceffedffd..2e90d298ee 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -545,7 +545,8 @@ class DBImpl : public DB { ColumnFamilyHandle* GetColumnFamilyHandle(uint32_t column_family_id); // Same as above, should called without mutex held and not on write thread. - ColumnFamilyHandle* GetColumnFamilyHandleUnlocked(uint32_t column_family_id); + std::unique_ptr GetColumnFamilyHandleUnlocked( + uint32_t column_family_id); // Returns the number of currently running flushes. // REQUIREMENT: mutex_ must be held when calling this function. diff --git a/db/db_test2.cc b/db/db_test2.cc index cc30ef85b5..0e345f8b24 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -2835,6 +2835,44 @@ TEST_F(DBTest2, TestBBTTailPrefetch) { rocksdb::SyncPoint::GetInstance()->ClearAllCallBacks(); } +TEST_F(DBTest2, TestGetColumnFamilyHandleUnlocked) { + // Setup sync point dependency to reproduce the race condition of + // DBImpl::GetColumnFamilyHandleUnlocked + rocksdb::SyncPoint::GetInstance()->LoadDependency( + { {"TestGetColumnFamilyHandleUnlocked::GetColumnFamilyHandleUnlocked1", + "TestGetColumnFamilyHandleUnlocked::PreGetColumnFamilyHandleUnlocked2"}, + {"TestGetColumnFamilyHandleUnlocked::GetColumnFamilyHandleUnlocked2", + "TestGetColumnFamilyHandleUnlocked::ReadColumnFamilyHandle1"}, + }); + SyncPoint::GetInstance()->EnableProcessing(); + + CreateColumnFamilies({"test1", "test2"}, Options()); + ASSERT_EQ(handles_.size(), 2); + + DBImpl* dbi = reinterpret_cast(db_); + port::Thread user_thread1([&]() { + auto cfh = dbi->GetColumnFamilyHandleUnlocked(handles_[0]->GetID()); + ASSERT_EQ(cfh->GetID(), handles_[0]->GetID()); + TEST_SYNC_POINT("TestGetColumnFamilyHandleUnlocked::GetColumnFamilyHandleUnlocked1"); + TEST_SYNC_POINT("TestGetColumnFamilyHandleUnlocked::ReadColumnFamilyHandle1"); + ASSERT_EQ(cfh->GetID(), handles_[0]->GetID()); + }); + + port::Thread user_thread2([&]() { + TEST_SYNC_POINT("TestGetColumnFamilyHandleUnlocked::PreGetColumnFamilyHandleUnlocked2"); + auto cfh = dbi->GetColumnFamilyHandleUnlocked(handles_[1]->GetID()); + ASSERT_EQ(cfh->GetID(), handles_[1]->GetID()); + TEST_SYNC_POINT("TestGetColumnFamilyHandleUnlocked::GetColumnFamilyHandleUnlocked2"); + ASSERT_EQ(cfh->GetID(), handles_[1]->GetID()); + }); + + user_thread1.join(); + user_thread2.join(); + + rocksdb::SyncPoint::GetInstance()->DisableProcessing(); + rocksdb::SyncPoint::GetInstance()->ClearAllCallBacks(); +} + } // namespace rocksdb int main(int argc, char** argv) { diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc index 1a32bd562e..e1045aa9e2 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -1459,8 +1459,7 @@ Status BlobDBImpl::GCFileAndUpdateLSM(const std::shared_ptr& bfptr, return s; } - auto* cfh = - db_impl_->GetColumnFamilyHandleUnlocked(bfptr->column_family_id()); + auto cfh = db_impl_->DefaultColumnFamily(); auto* cfd = reinterpret_cast(cfh)->cfd(); auto column_family_id = cfd->GetID(); bool has_ttl = header.has_ttl;