diff --git a/db/arena_wrapped_db_iter.cc b/db/arena_wrapped_db_iter.cc index 20d4655bed2..33f36dc3f66 100644 --- a/db/arena_wrapped_db_iter.cc +++ b/db/arena_wrapped_db_iter.cc @@ -92,7 +92,7 @@ Status ArenaWrappedDBIter::Refresh() { range_del_iter.reset( sv->mem->NewRangeTombstoneIterator(read_options_, latest_seq)); range_del_agg->AddTombstones(std::move(range_del_iter)); - cfd_->ReturnThreadLocalSuperVersion(sv); + cfd_->ReturnThreadLocalSuperVersion(db_impl_, sv); } // Refresh latest sequence number db_iter_->set_sequence(latest_seq); diff --git a/db/column_family.cc b/db/column_family.cc index c7e0c69317a..bedecec923a 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -435,7 +435,6 @@ SuperVersion::~SuperVersion() { SuperVersion* SuperVersion::Ref() { refs.fetch_add(1, std::memory_order_relaxed); - std::cout << "SV: " << this << " ref: " << refs << "\n"; return this; } @@ -445,8 +444,6 @@ uint32_t SuperVersion::GetRef() const { 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; @@ -679,8 +676,6 @@ ColumnFamilyData::~ColumnFamilyData() { } bool ColumnFamilyData::UnrefAndTryDelete() { - std::cout << "CFD: " << this << ", refs_.sub: " << refs_ << "\n"; - int old_refs = refs_.fetch_sub(1); assert(old_refs > 0); @@ -1209,7 +1204,7 @@ Compaction* ColumnFamilyData::CompactRange( SuperVersion* ColumnFamilyData::GetReferencedSuperVersion(DBImpl* db) { SuperVersion* sv = GetThreadLocalSuperVersion(db); sv->Ref(); - if (!ReturnThreadLocalSuperVersion(sv)) { + if (!ReturnThreadLocalSuperVersion(db, sv)) { // This Unref() corresponds to the Ref() in GetThreadLocalSuperVersion() // when the thread-local pointer was populated. So, the Ref() earlier in // this function still prevents the returned SuperVersion* from being @@ -1269,11 +1264,10 @@ 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) { +bool ColumnFamilyData::ReturnThreadLocalSuperVersion(DBImpl* db, SuperVersion* sv) { assert(sv != nullptr); // Put the SuperVersion back @@ -1283,15 +1277,22 @@ bool ColumnFamilyData::ReturnThreadLocalSuperVersion(SuperVersion* sv) { // 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) { + // A scrape has happened, we have to adjust the refs in both + // the super version and CFD that it refers to. + // FIXME: This assumes the SVs "release" are not interleaved. auto ptr = static_cast(local_sv_->Swap(sv)); if (sv != ptr) { - ptr->Unref(); + db->mutex()->Lock(); + assert(ptr != super_version_); + bool last_ref __attribute__((__unused__)); + last_ref = ptr->Unref(); + assert(last_ref); refs_.fetch_sub(1); ptr->Cleanup(); - delete ptr; + db->mutex()->Unlock(); + // delete ptr; } else { sv->Unref(); } @@ -1300,9 +1301,8 @@ bool ColumnFamilyData::ReturnThreadLocalSuperVersion(SuperVersion* sv) { // 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. - std::cout << "SV: " << sv << " (" << expected << ") obsolete : " << "\n"; + return false; } - return false; } void ColumnFamilyData::InstallSuperVersion( diff --git a/db/column_family.h b/db/column_family.h index 4bce2373562..4d2e3017b1f 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -278,7 +278,6 @@ class ColumnFamilyData { // 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() { - std::cout << "CFD: " << this << ", refs_.add: " << refs_ << "\n"; refs_.fetch_add(1); } @@ -446,7 +445,7 @@ class ColumnFamilyData { // Try to return SuperVersion back to thread local storage. Return true on // success and false on failure. It fails when the thread local storage // contains anything other than SuperVersion::kSVInUse flag. - bool ReturnThreadLocalSuperVersion(SuperVersion* sv); + bool ReturnThreadLocalSuperVersion(DBImpl* db, SuperVersion* sv); // thread-safe uint64_t GetSuperVersionNumber() const { return super_version_number_.load(); diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index cb41b08540b..6d565ae82d9 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -4064,7 +4064,6 @@ 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; @@ -4086,7 +4085,7 @@ void DBImpl::CleanupSuperVersion(SuperVersion* sv) { void DBImpl::ReturnAndCleanupSuperVersion(ColumnFamilyData* cfd, SuperVersion* sv) { - if (!cfd->ReturnThreadLocalSuperVersion(sv)) { + if (!cfd->ReturnThreadLocalSuperVersion(this, sv)) { CleanupSuperVersion(sv); } }