From 7ce4741cf4c83244689c3e89713be84f8c18f40b Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Thu, 28 Dec 2023 01:42:22 -0800 Subject: [PATCH 1/8] Add copy constructor for ColumnFamilyHandleImpl Signed-off-by: Yang Zhang --- db/column_family.cc | 32 ++++++++++++++++++++------------ db/column_family.h | 19 ++++++++++--------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index df1c8434a4b..d6814b3ab76 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -49,6 +49,13 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( cfd_->Ref(); } } +ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( + const ColumnFamilyHandleImpl& other) + : cfd_(other.cfd_), db_(other.db_), mutex_(other.mutex_) { + if (cfd_ != nullptr) { + cfd_->Ref(); + } +} ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() { if (cfd_ != nullptr) { @@ -188,8 +195,7 @@ Status CheckCFPathsSupported(const DBOptions& db_options, return Status::NotSupported( "More than one CF paths are only supported in " "universal and level compaction styles. "); - } else if (cf_options.cf_paths.empty() && - db_options.db_paths.size() > 1) { + } else if (cf_options.cf_paths.empty() && db_options.db_paths.size() > 1) { return Status::NotSupported( "More than one DB paths are only supported in " "universal and level compaction styles. "); @@ -335,7 +341,8 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, // were not deleted yet, when we open the DB we will find these .trash files // and schedule them to be deleted (or delete immediately if SstFileManager // was not used) - auto sfm = static_cast(db_options.sst_file_manager.get()); + auto sfm = + static_cast(db_options.sst_file_manager.get()); for (size_t i = 0; i < result.cf_paths.size(); i++) { DeleteScheduler::CleanupDirectory(db_options.env, sfm, result.cf_paths[i].path) @@ -588,8 +595,8 @@ ColumnFamilyData::ColumnFamilyData( compaction_picker_.reset( new FIFOCompactionPicker(ioptions_, &internal_comparator_)); } else if (ioptions_.compaction_style == kCompactionStyleNone) { - compaction_picker_.reset(new NullCompactionPicker( - ioptions_, &internal_comparator_)); + compaction_picker_.reset( + new NullCompactionPicker(ioptions_, &internal_comparator_)); ROCKS_LOG_WARN(ioptions_.logger, "Column family %s does not use any background compaction. " "Compactions can only be done via CompactFiles\n", @@ -977,7 +984,8 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions( mutable_cf_options.hard_pending_compaction_bytes_limit > 0 && (compaction_needed_bytes - mutable_cf_options.soft_pending_compaction_bytes_limit) > - 3 * (mutable_cf_options.hard_pending_compaction_bytes_limit - + 3 * + (mutable_cf_options.hard_pending_compaction_bytes_limit - mutable_cf_options.soft_pending_compaction_bytes_limit) / 4; @@ -1379,8 +1387,8 @@ bool ColumnFamilyData::ReturnThreadLocalSuperVersion(SuperVersion* sv) { return false; } -void ColumnFamilyData::InstallSuperVersion( - SuperVersionContext* sv_context, InstrumentedMutex* db_mutex) { +void ColumnFamilyData::InstallSuperVersion(SuperVersionContext* sv_context, + InstrumentedMutex* db_mutex) { db_mutex->AssertHeld(); return InstallSuperVersion(sv_context, mutable_cf_options_); } @@ -1546,8 +1554,8 @@ Env::WriteLifeTimeHint ColumnFamilyData::CalculateSSTWriteHint(int level) { // than base_level. return Env::WLTH_MEDIUM; } - return static_cast(level - base_level + - static_cast(Env::WLTH_MEDIUM)); + return static_cast( + level - base_level + static_cast(Env::WLTH_MEDIUM)); } Status ColumnFamilyData::AddDirectories( @@ -1641,8 +1649,8 @@ ColumnFamilyData* ColumnFamilySet::GetColumnFamily(uint32_t id) const { } } -ColumnFamilyData* ColumnFamilySet::GetColumnFamily(const std::string& name) - const { +ColumnFamilyData* ColumnFamilySet::GetColumnFamily( + const std::string& name) const { auto cfd_iter = column_families_.find(name); if (cfd_iter != column_families_.end()) { auto cfd = GetColumnFamily(cfd_iter->second); diff --git a/db/column_family.h b/db/column_family.h index 897084477e3..9888eb27a97 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -9,10 +9,10 @@ #pragma once -#include +#include #include +#include #include -#include #include "db/memtable_list.h" #include "db/table_cache.h" @@ -160,8 +160,9 @@ extern const double kIncSlowdownRatio; class ColumnFamilyHandleImpl : public ColumnFamilyHandle { public: // create while holding the mutex - ColumnFamilyHandleImpl( - ColumnFamilyData* cfd, DBImpl* db, InstrumentedMutex* mutex); + ColumnFamilyHandleImpl(ColumnFamilyData* cfd, DBImpl* db, + InstrumentedMutex* mutex); + ColumnFamilyHandleImpl(const ColumnFamilyHandleImpl& other); // destroy without mutex virtual ~ColumnFamilyHandleImpl(); virtual ColumnFamilyData* cfd() const { return cfd_; } @@ -186,7 +187,8 @@ class ColumnFamilyHandleImpl : public ColumnFamilyHandle { class ColumnFamilyHandleInternal : public ColumnFamilyHandleImpl { public: ColumnFamilyHandleInternal() - : ColumnFamilyHandleImpl(nullptr, nullptr, nullptr), internal_cfd_(nullptr) {} + : ColumnFamilyHandleImpl(nullptr, nullptr, nullptr), + internal_cfd_(nullptr) {} void SetCFD(ColumnFamilyData* _cfd) { internal_cfd_ = _cfd; } virtual ColumnFamilyData* cfd() const override { return internal_cfd_; } @@ -354,7 +356,7 @@ class ColumnFamilyData { Version* current() { return current_; } Version* dummy_versions() { return dummy_versions_; } void SetCurrent(Version* _current); - uint64_t GetNumLiveVersions() const; // REQUIRE: DB mutex held + uint64_t GetNumLiveVersions() const; // REQUIRE: DB mutex held uint64_t GetTotalSstFilesSize() const; // REQUIRE: DB mutex held uint64_t GetLiveSstFilesSize() const; // REQUIRE: DB mutex held uint64_t GetTotalBlobFileSize() const; // REQUIRE: DB mutex held @@ -550,7 +552,7 @@ class ColumnFamilyData { Version* dummy_versions_; // Head of circular doubly-linked list of versions. Version* current_; // == dummy_versions->prev_ - std::atomic refs_; // outstanding references to ColumnFamilyData + std::atomic refs_; // outstanding references to ColumnFamilyData std::atomic initialized_; std::atomic dropped_; // true if client dropped it @@ -648,8 +650,7 @@ class ColumnFamilySet { // ColumnFamilySet supports iteration class iterator { public: - explicit iterator(ColumnFamilyData* cfd) - : current_(cfd) {} + explicit iterator(ColumnFamilyData* cfd) : current_(cfd) {} // NOTE: minimum operators for for-loop iteration iterator& operator++() { current_ = current_->next_; From 852acdfb6bd85df60019f770955ffc40044d6959 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Thu, 28 Dec 2023 04:13:47 -0800 Subject: [PATCH 2/8] Revert format changes Signed-off-by: Yang Zhang --- db/column_family.cc | 26 +++++++++++++------------- db/column_family.h | 18 +++++++++--------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index d6814b3ab76..a0e66e7e5ce 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -49,6 +49,7 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( cfd_->Ref(); } } + ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( const ColumnFamilyHandleImpl& other) : cfd_(other.cfd_), db_(other.db_), mutex_(other.mutex_) { @@ -195,7 +196,8 @@ Status CheckCFPathsSupported(const DBOptions& db_options, return Status::NotSupported( "More than one CF paths are only supported in " "universal and level compaction styles. "); - } else if (cf_options.cf_paths.empty() && db_options.db_paths.size() > 1) { + } else if (cf_options.cf_paths.empty() && + db_options.db_paths.size() > 1) { return Status::NotSupported( "More than one DB paths are only supported in " "universal and level compaction styles. "); @@ -341,8 +343,7 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, // were not deleted yet, when we open the DB we will find these .trash files // and schedule them to be deleted (or delete immediately if SstFileManager // was not used) - auto sfm = - static_cast(db_options.sst_file_manager.get()); + auto sfm = static_cast(db_options.sst_file_manager.get()); for (size_t i = 0; i < result.cf_paths.size(); i++) { DeleteScheduler::CleanupDirectory(db_options.env, sfm, result.cf_paths[i].path) @@ -595,8 +596,8 @@ ColumnFamilyData::ColumnFamilyData( compaction_picker_.reset( new FIFOCompactionPicker(ioptions_, &internal_comparator_)); } else if (ioptions_.compaction_style == kCompactionStyleNone) { - compaction_picker_.reset( - new NullCompactionPicker(ioptions_, &internal_comparator_)); + compaction_picker_.reset(new NullCompactionPicker( + ioptions_, &internal_comparator_)); ROCKS_LOG_WARN(ioptions_.logger, "Column family %s does not use any background compaction. " "Compactions can only be done via CompactFiles\n", @@ -984,8 +985,7 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions( mutable_cf_options.hard_pending_compaction_bytes_limit > 0 && (compaction_needed_bytes - mutable_cf_options.soft_pending_compaction_bytes_limit) > - 3 * - (mutable_cf_options.hard_pending_compaction_bytes_limit - + 3 * (mutable_cf_options.hard_pending_compaction_bytes_limit - mutable_cf_options.soft_pending_compaction_bytes_limit) / 4; @@ -1387,8 +1387,8 @@ bool ColumnFamilyData::ReturnThreadLocalSuperVersion(SuperVersion* sv) { return false; } -void ColumnFamilyData::InstallSuperVersion(SuperVersionContext* sv_context, - InstrumentedMutex* db_mutex) { +void ColumnFamilyData::InstallSuperVersion( + SuperVersionContext* sv_context, InstrumentedMutex* db_mutex) { db_mutex->AssertHeld(); return InstallSuperVersion(sv_context, mutable_cf_options_); } @@ -1554,8 +1554,8 @@ Env::WriteLifeTimeHint ColumnFamilyData::CalculateSSTWriteHint(int level) { // than base_level. return Env::WLTH_MEDIUM; } - return static_cast( - level - base_level + static_cast(Env::WLTH_MEDIUM)); + return static_cast(level - base_level + + static_cast(Env::WLTH_MEDIUM)); } Status ColumnFamilyData::AddDirectories( @@ -1649,8 +1649,8 @@ ColumnFamilyData* ColumnFamilySet::GetColumnFamily(uint32_t id) const { } } -ColumnFamilyData* ColumnFamilySet::GetColumnFamily( - const std::string& name) const { +ColumnFamilyData* ColumnFamilySet::GetColumnFamily(const std::string& name) + const { auto cfd_iter = column_families_.find(name); if (cfd_iter != column_families_.end()) { auto cfd = GetColumnFamily(cfd_iter->second); diff --git a/db/column_family.h b/db/column_family.h index 9888eb27a97..1a3ca3797dd 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -9,10 +9,10 @@ #pragma once -#include -#include #include +#include #include +#include #include "db/memtable_list.h" #include "db/table_cache.h" @@ -160,8 +160,8 @@ extern const double kIncSlowdownRatio; class ColumnFamilyHandleImpl : public ColumnFamilyHandle { public: // create while holding the mutex - ColumnFamilyHandleImpl(ColumnFamilyData* cfd, DBImpl* db, - InstrumentedMutex* mutex); + ColumnFamilyHandleImpl( + ColumnFamilyData* cfd, DBImpl* db, InstrumentedMutex* mutex); ColumnFamilyHandleImpl(const ColumnFamilyHandleImpl& other); // destroy without mutex virtual ~ColumnFamilyHandleImpl(); @@ -187,8 +187,7 @@ class ColumnFamilyHandleImpl : public ColumnFamilyHandle { class ColumnFamilyHandleInternal : public ColumnFamilyHandleImpl { public: ColumnFamilyHandleInternal() - : ColumnFamilyHandleImpl(nullptr, nullptr, nullptr), - internal_cfd_(nullptr) {} + : ColumnFamilyHandleImpl(nullptr, nullptr, nullptr), internal_cfd_(nullptr) {} void SetCFD(ColumnFamilyData* _cfd) { internal_cfd_ = _cfd; } virtual ColumnFamilyData* cfd() const override { return internal_cfd_; } @@ -356,7 +355,7 @@ class ColumnFamilyData { Version* current() { return current_; } Version* dummy_versions() { return dummy_versions_; } void SetCurrent(Version* _current); - uint64_t GetNumLiveVersions() const; // REQUIRE: DB mutex held + uint64_t GetNumLiveVersions() const; // REQUIRE: DB mutex held uint64_t GetTotalSstFilesSize() const; // REQUIRE: DB mutex held uint64_t GetLiveSstFilesSize() const; // REQUIRE: DB mutex held uint64_t GetTotalBlobFileSize() const; // REQUIRE: DB mutex held @@ -552,7 +551,7 @@ class ColumnFamilyData { Version* dummy_versions_; // Head of circular doubly-linked list of versions. Version* current_; // == dummy_versions->prev_ - std::atomic refs_; // outstanding references to ColumnFamilyData + std::atomic refs_; // outstanding references to ColumnFamilyData std::atomic initialized_; std::atomic dropped_; // true if client dropped it @@ -650,7 +649,8 @@ class ColumnFamilySet { // ColumnFamilySet supports iteration class iterator { public: - explicit iterator(ColumnFamilyData* cfd) : current_(cfd) {} + explicit iterator(ColumnFamilyData* cfd) + : current_(cfd) {} // NOTE: minimum operators for for-loop iteration iterator& operator++() { current_ = current_->next_; From 72db33367bc9b74fa1c3c1c882da0f5b1f9418ef Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Tue, 2 Jan 2024 20:02:36 -0800 Subject: [PATCH 3/8] Add move instead of copy constructor Signed-off-by: Yang Zhang --- db/column_family.cc | 12 +++++++----- db/column_family.h | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index a0e66e7e5ce..9fe6276d6fe 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -51,11 +51,13 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( } ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( - const ColumnFamilyHandleImpl& other) - : cfd_(other.cfd_), db_(other.db_), mutex_(other.mutex_) { - if (cfd_ != nullptr) { - cfd_->Ref(); - } + ColumnFamilyHandleImpl&& other) { + other.cfd_ = cfd_; + other.db_ = db_; + other.mutex_ = mutex_; + cfd_ = nullptr; + db_ = nullptr; + mutex_ = nullptr; } ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() { diff --git a/db/column_family.h b/db/column_family.h index 1a3ca3797dd..07692e5dadc 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -162,7 +162,7 @@ class ColumnFamilyHandleImpl : public ColumnFamilyHandle { // create while holding the mutex ColumnFamilyHandleImpl( ColumnFamilyData* cfd, DBImpl* db, InstrumentedMutex* mutex); - ColumnFamilyHandleImpl(const ColumnFamilyHandleImpl& other); + ColumnFamilyHandleImpl(ColumnFamilyHandleImpl&& other); // destroy without mutex virtual ~ColumnFamilyHandleImpl(); virtual ColumnFamilyData* cfd() const { return cfd_; } From 12aacd2bd9ad74e1b2d27b4b6bb88e289f3b0a3d Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Tue, 2 Jan 2024 20:05:06 -0800 Subject: [PATCH 4/8] Fix format Signed-off-by: Yang Zhang --- db/column_family.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index 9fe6276d6fe..f3f1f6c22df 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -50,8 +50,7 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( } } -ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( - ColumnFamilyHandleImpl&& other) { +ColumnFamilyHandleImpl::ColumnFamilyHandleImpl(ColumnFamilyHandleImpl&& other) { other.cfd_ = cfd_; other.db_ = db_; other.mutex_ = mutex_; From 83d29393bbd98781b39e8cd89169926549eee41f Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Tue, 2 Jan 2024 20:43:00 -0800 Subject: [PATCH 5/8] Apply comments Signed-off-by: Yang Zhang --- db/column_family.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index f3f1f6c22df..76eac06a417 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -51,12 +51,12 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( } ColumnFamilyHandleImpl::ColumnFamilyHandleImpl(ColumnFamilyHandleImpl&& other) { - other.cfd_ = cfd_; - other.db_ = db_; - other.mutex_ = mutex_; - cfd_ = nullptr; - db_ = nullptr; - mutex_ = nullptr; + cfd_ = other.cfd_; + db_ = other.db_; + mutex_ = other.mutex_; + other.cfd_ = nullptr; + other.db_ = nullptr; + other.mutex_ = nullptr; } ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() { From cb9cb1c4b41755e2487456632acc61501c21aeb3 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Tue, 2 Jan 2024 21:07:55 -0800 Subject: [PATCH 6/8] Revert to copy Signed-off-by: Yang Zhang --- db/column_family.cc | 11 ++++------- db/column_family.h | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index 76eac06a417..8d26cf89862 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -50,13 +50,10 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( } } -ColumnFamilyHandleImpl::ColumnFamilyHandleImpl(ColumnFamilyHandleImpl&& other) { - cfd_ = other.cfd_; - db_ = other.db_; - mutex_ = other.mutex_; - other.cfd_ = nullptr; - other.db_ = nullptr; - other.mutex_ = nullptr; +ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( + const ColumnFamilyHandleImpl& other) + : cfd_(other.cfd_), db_(other.db_), mutex_(other.mutex_) { + cfd_->Ref(); } ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() { diff --git a/db/column_family.h b/db/column_family.h index 07692e5dadc..1a3ca3797dd 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -162,7 +162,7 @@ class ColumnFamilyHandleImpl : public ColumnFamilyHandle { // create while holding the mutex ColumnFamilyHandleImpl( ColumnFamilyData* cfd, DBImpl* db, InstrumentedMutex* mutex); - ColumnFamilyHandleImpl(ColumnFamilyHandleImpl&& other); + ColumnFamilyHandleImpl(const ColumnFamilyHandleImpl& other); // destroy without mutex virtual ~ColumnFamilyHandleImpl(); virtual ColumnFamilyData* cfd() const { return cfd_; } From 5230fddb9a00c932a5d5b65d53df6f15933a51e2 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Tue, 2 Jan 2024 21:09:51 -0800 Subject: [PATCH 7/8] Fix format Signed-off-by: Yang Zhang --- db/column_family.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index 8d26cf89862..88f673f446d 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -51,9 +51,9 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( } ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( - const ColumnFamilyHandleImpl& other) - : cfd_(other.cfd_), db_(other.db_), mutex_(other.mutex_) { - cfd_->Ref(); + const ColumnFamilyHandleImpl& other) + : cfd_(other.cfd_), db_(other.db_), mutex_(other.mutex_) { + cfd_->Ref(); } ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() { From cc56f164b192f97726d94cd2dea70e5be3dd7b34 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Tue, 2 Jan 2024 21:41:22 -0800 Subject: [PATCH 8/8] Ref cfd when it is not null Signed-off-by: Yang Zhang --- db/column_family.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/db/column_family.cc b/db/column_family.cc index 88f673f446d..a0e66e7e5ce 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -53,7 +53,9 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( ColumnFamilyHandleImpl::ColumnFamilyHandleImpl( const ColumnFamilyHandleImpl& other) : cfd_(other.cfd_), db_(other.db_), mutex_(other.mutex_) { - cfd_->Ref(); + if (cfd_ != nullptr) { + cfd_->Ref(); + } } ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() {