diff --git a/db/column_family.cc b/db/column_family.cc index 4acbde55cfd..c7e0c69317a 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -435,11 +435,18 @@ SuperVersion::~SuperVersion() { SuperVersion* SuperVersion::Ref() { refs.fetch_add(1, std::memory_order_relaxed); + std::cout << "SV: " << this << " ref: " << refs << "\n"; return this; } +uint32_t SuperVersion::GetRef() const { + return refs.load(); +} + bool SuperVersion::Unref() { // fetch_sub returns the previous value of ref + std::cout << "SV: " << this << " unref: " << refs << "\n"; + uint32_t previous_refs = refs.fetch_sub(1); assert(previous_refs > 0); return previous_refs == 1; @@ -672,6 +679,8 @@ ColumnFamilyData::~ColumnFamilyData() { } bool ColumnFamilyData::UnrefAndTryDelete() { + std::cout << "CFD: " << this << ", refs_.sub: " << refs_ << "\n"; + int old_refs = refs_.fetch_sub(1); assert(old_refs > 0); @@ -1219,7 +1228,7 @@ SuperVersion* ColumnFamilyData::GetThreadLocalSuperVersion(DBImpl* db) { // local pointer to guarantee exclusive access. If the thread local pointer // is being used while a new SuperVersion is installed, the cached // SuperVersion can become stale. In that case, the background thread would - // have swapped in kSVObsolete. We re-check the value at when returning + // have swapped in kSVObsolete. We re-check the value when returning // SuperVersion back to thread local, with an atomic compare and swap. // The superversion will need to be released if detected to be stale. void* ptr = local_sv_->Swap(SuperVersion::kSVInUse); @@ -1228,8 +1237,8 @@ SuperVersion* ColumnFamilyData::GetThreadLocalSuperVersion(DBImpl* db) { // (2) the Swap above (always) installs kSVInUse, ThreadLocal storage // should only keep kSVInUse before ReturnThreadLocalSuperVersion call // (if no Scrape happens). - // assert(ptr != SuperVersion::kSVInUse); if (ptr == SuperVersion::kSVInUse) { + // FIXME: Check version number too. return super_version_->Ref(); } @@ -1260,23 +1269,38 @@ SuperVersion* ColumnFamilyData::GetThreadLocalSuperVersion(DBImpl* db) { delete sv_to_delete; } assert(sv != nullptr); + std::cout << "SV: " << sv << std::endl; return sv; } bool ColumnFamilyData::ReturnThreadLocalSuperVersion(SuperVersion* sv) { assert(sv != nullptr); + // Put the SuperVersion back void* expected = SuperVersion::kSVInUse; + if (local_sv_->CompareAndSwap(static_cast(sv), expected)) { // When we see kSVInUse in the ThreadLocal, we are sure ThreadLocal // storage has not been altered and no Scrape has happened. The // SuperVersion is still current. + std::cout << "SV: " << sv << " in use\n"; + return true; + } else if (expected != nullptr) { + auto ptr = static_cast(local_sv_->Swap(sv)); + if (sv != ptr) { + ptr->Unref(); + refs_.fetch_sub(1); + ptr->Cleanup(); + delete ptr; + } else { + sv->Unref(); + } return true; } else { // ThreadLocal scrape happened in the process of this GetImpl call (after // thread local Swap() at the beginning and before CompareAndSwap()). // This means the SuperVersion it holds is obsolete. - // assert(expected == SuperVersion::kSVObsolete); + std::cout << "SV: " << sv << " (" << expected << ") obsolete : " << "\n"; } return false; } @@ -1333,6 +1357,7 @@ void ColumnFamilyData::ResetThreadLocalSuperVersions() { continue; } auto sv = static_cast(ptr); + bool was_last_ref __attribute__((__unused__)); was_last_ref = sv->Unref(); // sv couldn't have been the last reference because diff --git a/db/column_family.h b/db/column_family.h index 2ab1f9e0458..4bce2373562 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -211,6 +211,9 @@ struct SuperVersion { // should be called outside the mutex SuperVersion() = default; ~SuperVersion(); + + uint32_t GetRef() const; + SuperVersion* Ref(); // If Unref() returns true, Cleanup() should be called with mutex held // before deleting this SuperVersion. @@ -274,7 +277,10 @@ class ColumnFamilyData { // Ref() can only be called from a context where the caller can guarantee // that ColumnFamilyData is alive (while holding a non-zero ref already, // holding a DB mutex, or as the leader in a write batch group). - void Ref() { refs_.fetch_add(1); } + void Ref() { + std::cout << "CFD: " << this << ", refs_.add: " << refs_ << "\n"; + refs_.fetch_add(1); + } // UnrefAndTryDelete() decreases the reference count and do free if needed, // return true if this is freed else false, UnrefAndTryDelete() can only diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 1637032ee26..cb41b08540b 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -4045,17 +4045,9 @@ bool DBImpl::GetAggregatedIntProperty(const Slice& property, return ret; } -SuperVersion* DBImpl::GetAndRefSuperVersion(ColumnFamilyData* cfd, - bool useThreadLocalCache) { - if (useThreadLocalCache) { - // TODO(ljin): consider using GetReferencedSuperVersion() directly - return cfd->GetThreadLocalSuperVersion(this); - } else { - this->mutex()->Lock(); - auto sv = cfd->GetSuperVersion(); - this->mutex()->Unlock(); - return sv; - } +SuperVersion* DBImpl::GetAndRefSuperVersion(ColumnFamilyData* cfd) { + // TODO(ljin): consider using GetReferencedSuperVersion() directly + return cfd->GetThreadLocalSuperVersion(this); } // REQUIRED: this function should only be called on the write thread or if the @@ -4072,6 +4064,7 @@ SuperVersion* DBImpl::GetAndRefSuperVersion(uint32_t column_family_id) { void DBImpl::CleanupSuperVersion(SuperVersion* sv) { // Release SuperVersion + std::cout << "SV: Cleanup: " << sv << "\n"; if (sv->Unref()) { bool defer_purge = immutable_db_options().avoid_unnecessary_blocking_io; diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index 5bd81155093..d37c51ee43e 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -785,8 +785,7 @@ class DBImpl : public DB { // Find Super version and reference it. Based on options, it might return // the thread local cached one. // Call ReturnAndCleanupSuperVersion() when it is no longer needed. - SuperVersion* GetAndRefSuperVersion(ColumnFamilyData* cfd, - bool useThreadLocalCache = true); + SuperVersion* GetAndRefSuperVersion(ColumnFamilyData* cfd); // Similar to the previous function but looks up based on a column family id. // nullptr will be returned if this column family no longer exists. diff --git a/utilities/transactions/transaction_util.cc b/utilities/transactions/transaction_util.cc index 61762a8ff50..f1d72ec074b 100644 --- a/utilities/transactions/transaction_util.cc +++ b/utilities/transactions/transaction_util.cc @@ -27,7 +27,7 @@ Status TransactionUtil::CheckKeyForConflicts( auto cfh = static_cast_with_check(column_family); auto cfd = cfh->cfd(); - SuperVersion* sv = db_impl->GetAndRefSuperVersion(cfd, true); + SuperVersion* sv = db_impl->GetAndRefSuperVersion(cfd); if (sv == nullptr) { result = Status::InvalidArgument("Could not access column family " +