Skip to content

Commit

Permalink
MB-36049: Make VBucket::purge_seqno atomic
Browse files Browse the repository at this point in the history
As reported by TSan, VBucket::purge_seqno is read and written without
a lock:

  Write of size 8 at 0x7b6800107728 by thread T41:
    #0 Monotonic::operator=(unsigned long const&) kv_engine/engines/ep/src/monotonic.h:134 (ep.so+0x0000001be09e)
    couchbase#1 VBucket::setPurgeSeqno(unsigned long) kv_engine/engines/ep/src/vbucket.h:218 (ep.so+0x0000001be09e)
    couchbase#2 EphemeralVBucket::purgeStaleItems(std::function) kv_engine/engines/ep/src/ephemeral_vb.cc:350 (ep.so+0x0000001be09e)
    couchbase#3 EphemeralVBucket::StaleItemDeleter::visit(VBucket&) kv_engine/engines/ep/src/ephemeral_tombstone_purger.cc:205 (ep.so+0x0000001b7571)
    couchbase#4 KVBucket::pauseResumeVisit(PauseResumeVBVisitor&, KVBucketIface::Position&) kv_engine/engines/ep/src/kv_bucket.cc:2278 (ep.so+0x000000209a09)
    couchbase#5 EphTombstoneStaleItemDeleter::run() kv_engine/engines/ep/src/ephemeral_tombstone_purger.cc:273 (ep.so+0x0000001b6d59)
    couchbase#6 ExecutorThread::run() kv_engine/engines/ep/src/executorthread.cc:153 (ep.so+0x0000001d0405)
    #7 launch_executor_thread kv_engine/engines/ep/src/executorthread.cc:34 (ep.so+0x0000001d1345)
    #8 CouchbaseThread::run() platform/src/cb_pthreads.cc:58 (libplatform_so.so.0.1.0+0x000000009c5f)
    #9 platform_thread_wrap platform/src/cb_pthreads.cc:71 (libplatform_so.so.0.1.0+0x000000009c5f)
    #10   (libtsan.so.0+0x000000024feb)

  Previous read of size 8 at 0x7b6800107728 by thread T6 (mutexes: write M1057918717805263064):
    #0 VBucket::_addStats(bool, std::function)> const&, void const*) kv_engine/engines/ep/src/vbucket.cc:3010 (ep.so+0x0000002796b6)
    couchbase#1 EphemeralVBucket::addStats(bool, std::function)> const&, void const*) kv_engine/engines/ep/src/ephemeral_vb.cc:166 (ep.so+0x0000001b8824)
    couchbase#2 addVBStats kv_engine/engines/ep/src/ep_engine.cc:3155 (ep.so+0x000000183e93)
    couchbase#3 visitBucket kv_engine/engines/ep/src/ep_engine.cc:3127 (ep.so+0x000000183e93)
    couchbase#4 KVBucket::visit(VBucketVisitor&) kv_engine/engines/ep/src/kv_bucket.cc:2254 (ep.so+0x000000209efd)
    couchbase#5 EventuallyPersistentEngine::doVBucketStats(void const*, std::function)> const&, char const*, int, bool, bool) kv_engine/engines/ep/src/ep_engine.cc:3193 (ep.so+0x000000183a7b)
    ...

Fix by changing to an Atomic WeaklyMonotonic type.

Change-Id: I014242268f913d31fdc0964c42f59aa952607ba4
Reviewed-on: http://review.couchbase.org/114950
Reviewed-by: James Harrison <[email protected]>
Tested-by: Build Bot <[email protected]>
  • Loading branch information
daverigby committed Sep 18, 2019
1 parent fb6944d commit 894eabd
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion engines/ep/src/vbucket.h
Original file line number Diff line number Diff line change
Expand Up @@ -2338,7 +2338,13 @@ class VBucket : public std::enable_shared_from_this<VBucket> {
std::mutex pendingOpLock;
std::vector<const void*> pendingOps;
std::chrono::steady_clock::time_point pendingOpsStart;
WeaklyMonotonic<uint64_t> purge_seqno;

/**
* Sequence number of the highest purged tombstone.
* - Weakly monotonic as this should not go backwards.
* - Atomic so it can be read without locks for stats printing.
*/
WeaklyAtomicMonotonic<uint64_t> purge_seqno;
std::atomic<bool> takeover_backed_up;

/* snapshotMutex is used to update/read the pair {start, end} atomically,
Expand Down

0 comments on commit 894eabd

Please sign in to comment.