From c4e90c79ed1fe848b2a468f39d92fb9a047ab971 Mon Sep 17 00:00:00 2001 From: Lei Jin Date: Thu, 19 Jun 2014 09:31:14 -0700 Subject: [PATCH] bug fix: iteration over ColumnFamilySet needs to be under mutex Summary: asan_crash_test is failing on segfault Test Plan: running asan_crash_test Reviewers: sdong, igor Reviewed By: igor Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D19149 --- db/db_impl.cc | 20 +++++++++++--------- db/db_impl.h | 1 + 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index d2a9d0da8..a9706ab6d 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -2458,9 +2458,6 @@ Status DBImpl::InstallCompactionResults(CompactionState* compact, inline SequenceNumber DBImpl::findEarliestVisibleSnapshot( SequenceNumber in, std::vector& snapshots, SequenceNumber* prev_snapshot) { - if (!IsSnapshotSupported()) { - return 0; - } SequenceNumber prev __attribute__((unused)) = 0; for (const auto cur : snapshots) { assert(prev <= cur); @@ -2499,6 +2496,7 @@ uint64_t DBImpl::CallFlushDuringCompaction(ColumnFamilyData* cfd, } Status DBImpl::ProcessKeyValueCompaction( + bool is_snapshot_supported, SequenceNumber visible_at_tip, SequenceNumber earliest_snapshot, SequenceNumber latest_snapshot, @@ -2627,11 +2625,10 @@ Status DBImpl::ProcessKeyValueCompaction( // Otherwise, search though all existing snapshots to find // the earlist snapshot that is affected by this kv. SequenceNumber prev_snapshot = 0; // 0 means no previous snapshot - SequenceNumber visible = visible_at_tip ? - visible_at_tip : - findEarliestVisibleSnapshot(ikey.sequence, - compact->existing_snapshots, - &prev_snapshot); + SequenceNumber visible = visible_at_tip ? visible_at_tip : + is_snapshot_supported ? findEarliestVisibleSnapshot(ikey.sequence, + compact->existing_snapshots, &prev_snapshot) + : 0; if (visible_in_snapshot == visible) { // If the earliest snapshot is which this key is visible in @@ -2897,6 +2894,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact, // Allocate the output file numbers before we release the lock AllocateCompactionOutputFileNumbers(compact); + bool is_snapshot_supported = IsSnapshotSupported(); // Release mutex while we're actually doing the compaction work mutex_.Unlock(); log_buffer->FlushBufferToLog(); @@ -2983,6 +2981,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact, // Done buffering for the current prefix. Spit it out to disk // Now just iterate through all the kv-pairs status = ProcessKeyValueCompaction( + is_snapshot_supported, visible_at_tip, earliest_snapshot, latest_snapshot, @@ -3018,6 +3017,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact, compact->MergeKeyValueSliceBuffer(&cfd->internal_comparator()); status = ProcessKeyValueCompaction( + is_snapshot_supported, visible_at_tip, earliest_snapshot, latest_snapshot, @@ -3039,6 +3039,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact, } compact->MergeKeyValueSliceBuffer(&cfd->internal_comparator()); status = ProcessKeyValueCompaction( + is_snapshot_supported, visible_at_tip, earliest_snapshot, latest_snapshot, @@ -3053,6 +3054,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact, if (!compaction_filter_v2) { status = ProcessKeyValueCompaction( + is_snapshot_supported, visible_at_tip, earliest_snapshot, latest_snapshot, @@ -3692,9 +3694,9 @@ bool DBImpl::IsSnapshotSupported() const { } const Snapshot* DBImpl::GetSnapshot() { + MutexLock l(&mutex_); // returns null if the underlying memtable does not support snapshot. if (!IsSnapshotSupported()) return nullptr; - MutexLock l(&mutex_); return snapshots_.New(versions_->LastSequence()); } diff --git a/db/db_impl.h b/db/db_impl.h index dd7b6f88c..797cb0484 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -376,6 +376,7 @@ class DBImpl : public DB { // Call compaction filter if is_compaction_v2 is not true. Then iterate // through input and compact the kv-pairs Status ProcessKeyValueCompaction( + bool is_snapshot_supported, SequenceNumber visible_at_tip, SequenceNumber earliest_snapshot, SequenceNumber latest_snapshot,