diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 98709dd92..e4b275e56 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -1889,6 +1889,8 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key, return s; } } + TEST_SYNC_POINT("DBImpl::GetImpl:PostMemTableGet:0"); + TEST_SYNC_POINT("DBImpl::GetImpl:PostMemTableGet:1"); PinnedIteratorsManager pinned_iters_mgr; if (!done) { PERF_TIMER_GUARD(get_from_output_files_time); @@ -1906,8 +1908,6 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key, { PERF_TIMER_GUARD(get_post_process_time); - ReturnAndCleanupSuperVersion(cfd, sv); - RecordTick(stats_, NUMBER_KEYS_READ); size_t size = 0; if (s.ok()) { @@ -1934,6 +1934,8 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key, PERF_COUNTER_ADD(get_read_bytes, size); } RecordInHistogram(stats_, BYTES_PER_READ, size); + + ReturnAndCleanupSuperVersion(cfd, sv); } return s; } diff --git a/db/db_merge_operand_test.cc b/db/db_merge_operand_test.cc index f00c6872f..bca35b258 100644 --- a/db/db_merge_operand_test.cc +++ b/db/db_merge_operand_test.cc @@ -47,7 +47,7 @@ class DBMergeOperandTest : public DBTestBase { : DBTestBase("db_merge_operand_test", /*env_do_fsync=*/true) {} }; -TEST_F(DBMergeOperandTest, MergeOperandReadAfterFreeBug) { +TEST_F(DBMergeOperandTest, CacheEvictedMergeOperandReadAfterFreeBug) { // There was a bug of reading merge operands after they are mistakely freed // in DB::GetMergeOperands, which is surfaced by cache full. // See PR#9507 for more. @@ -86,6 +86,42 @@ TEST_F(DBMergeOperandTest, MergeOperandReadAfterFreeBug) { ASSERT_EQ(values[3].ToString(), "v4"); } +TEST_F(DBMergeOperandTest, FlushedMergeOperandReadAfterFreeBug) { + // Repro for a bug where a memtable containing a merge operand could be + // deleted before the merge operand was saved to the result. + auto options = CurrentOptions(); + options.merge_operator = MergeOperators::CreateStringAppendOperator(); + Reopen(options); + + ASSERT_OK(Merge("key", "value")); + + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( + {{"DBImpl::GetImpl:PostMemTableGet:0", + "DBMergeOperandTest::FlushedMergeOperandReadAfterFreeBug:PreFlush"}, + {"DBMergeOperandTest::FlushedMergeOperandReadAfterFreeBug:PostFlush", + "DBImpl::GetImpl:PostMemTableGet:1"}}); + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); + + auto flush_thread = port::Thread([&]() { + TEST_SYNC_POINT( + "DBMergeOperandTest::FlushedMergeOperandReadAfterFreeBug:PreFlush"); + ASSERT_OK(Flush()); + TEST_SYNC_POINT( + "DBMergeOperandTest::FlushedMergeOperandReadAfterFreeBug:PostFlush"); + }); + + PinnableSlice value; + GetMergeOperandsOptions merge_operands_info; + merge_operands_info.expected_max_number_of_operands = 1; + int number_of_operands; + ASSERT_OK(db_->GetMergeOperands(ReadOptions(), db_->DefaultColumnFamily(), + "key", &value, &merge_operands_info, + &number_of_operands)); + ASSERT_EQ(1, number_of_operands); + + flush_thread.join(); +} + TEST_F(DBMergeOperandTest, GetMergeOperandsBasic) { Options options; options.create_if_missing = true;