diff --git a/HISTORY.md b/HISTORY.md index 804673199..72a7fd588 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -9,6 +9,7 @@ * Fixed an optimistic transaction validation bug caused by DBImpl::GetLatestSequenceForKey() returning non-latest seq for merge (#10724). * Fixed a bug in iterator refresh which could segfault for DeleteRange users (#10739). * Fixed a bug causing manual flush with `flush_opts.wait=false` to stall when database has stopped all writes (#10001). +* Fixed a bug in iterator refresh that was not freeing up SuperVersion, which could cause excessive resource pinniung (#10770). ### Performance Improvements * Try to align the compaction output file boundaries to the next level ones, which can reduce more than 10% compaction load for the default level compaction. The feature is enabled by default, to disable, set `AdvancedColumnFamilyOptions.level_compaction_dynamic_file_size` to false. As a side effect, it can create SSTs larger than the target_file_size (capped at 2x target_file_size) or smaller files. diff --git a/db/arena_wrapped_db_iter.cc b/db/arena_wrapped_db_iter.cc index b7959491c..d3161c228 100644 --- a/db/arena_wrapped_db_iter.cc +++ b/db/arena_wrapped_db_iter.cc @@ -90,6 +90,7 @@ Status ArenaWrappedDBIter::Refresh() { // Refresh range-tombstones in MemTable if (!read_options_.ignore_range_deletions) { SuperVersion* sv = cfd_->GetThreadLocalSuperVersion(db_impl_); + TEST_SYNC_POINT_CALLBACK("ArenaWrappedDBIter::Refresh:SV", nullptr); auto t = sv->mem->NewRangeTombstoneIterator( read_options_, latest_seq, false /* immutable_memtable */); if (!t || t->empty()) { @@ -107,7 +108,7 @@ Status ArenaWrappedDBIter::Refresh() { } else { // current mutable memtable has range tombstones if (!memtable_range_tombstone_iter_) { delete t; - cfd_->ReturnThreadLocalSuperVersion(sv); + db_impl_->ReturnAndCleanupSuperVersion(cfd_, sv); // The memtable under DBIter did not have range tombstone before // refresh. reinit_internal_iter(); @@ -119,7 +120,7 @@ Status ArenaWrappedDBIter::Refresh() { &cfd_->internal_comparator(), nullptr, nullptr); } } - cfd_->ReturnThreadLocalSuperVersion(sv); + db_impl_->ReturnAndCleanupSuperVersion(cfd_, sv); } // Refresh latest sequence number db_iter_->set_sequence(latest_seq); diff --git a/db/db_iterator_test.cc b/db/db_iterator_test.cc index b2d549250..51c81585c 100644 --- a/db/db_iterator_test.cc +++ b/db/db_iterator_test.cc @@ -3232,6 +3232,28 @@ TEST_F(DBIteratorTest, BackwardIterationOnInplaceUpdateMemtable) { } } +TEST_F(DBIteratorTest, IteratorRefreshReturnSV) { + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + DestroyAndReopen(options); + ASSERT_OK( + db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z")); + std::unique_ptr iter{db_->NewIterator(ReadOptions())}; + SyncPoint::GetInstance()->SetCallBack( + "ArenaWrappedDBIter::Refresh:SV", [&](void*) { + ASSERT_OK(db_->Put(WriteOptions(), "dummy", "new SV")); + // This makes the local SV obselete. + ASSERT_OK(Flush()); + SyncPoint::GetInstance()->DisableProcessing(); + }); + SyncPoint::GetInstance()->EnableProcessing(); + ASSERT_OK(iter->Refresh()); + iter.reset(); + // iter used to not cleanup SV, so the Close() below would hit an assertion + // error. + Close(); +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) {