MB-19280: Fix data race in CouchKVStore stats access 81/63081/6
authorDave Rigby <daver@couchbase.com>
Wed, 7 Oct 2015 15:27:03 +0000 (15:27 +0000)
committerChiyoung Seo <chiyoung@couchbase.com>
Sat, 23 Apr 2016 01:17:10 +0000 (01:17 +0000)
As reported by ThreadSanitizer. CouchKVStore maintains a map of
vBucketID to counter - dbFileRevMap. This is read by some of the stats
functions (e.g. getNumPersistedDeletes) without a lock and hence there
is a potential race.

Solve this by changing the type of these counters to RelaxedAtomic<>.

WARNING: ThreadSanitizer: data race (pid=10155)
  Read of size 8 at 0x7d9000008000 by main thread (mutexes: write M21730):
    #0 CouchKVStore::getNumPersistedDeletes(unsigned short) ep-engine/src/couch-kvstore/couch-kvstore.cc:2095 (ep.so+0x000000326779)
    #1 EventuallyPersistentEngine::doDcpVbTakeoverStats(void const*, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), std::string&, unsigned short) ep-engine/src/ep_engine.cc:5312 (ep.so+0x000000155ca5)
    #2 EventuallyPersistentEngine::getStats(void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)) ep-engine/src/ep_engine.cc:4462 (ep.so+0x000000154622)

  Previous write of size 8 at 0x7d9000008000 by thread T8 (mutexes: write M15079):
    #0 CouchKVStore::updateDbFileMap(unsigned short, unsigned long) ep-engine/src/couch-kvstore/couch-kvstore.cc:1306 (ep.so+0x000000311d3c)
    #1 CouchKVStore::openDB(unsigned short, unsigned long, _db**, unsigned long, unsigned long*) ep-engine/src/couch-kvstore/couch-kvstore.cc:1336 (ep.so+0x00000030f7ae)
    #2 CouchKVStore::setVBucketState(unsigned short, vbucket_state&, Callback<KVStatsCtx>*) ep-engine/src/couch-kvstore/couch-kvstore.cc:981 (ep.so+0x00000031a557)
    #3 CouchKVStore::snapshotVBucket(unsigned short, vbucket_state&, Callback<KVStatsCtx>*) ep-engine/src/couch-kvstore/couch-kvstore.cc:891 (ep.so+0x00000031a11c)
    #4 EventuallyPersistentStore::snapshotVBuckets(Priority const&, unsigned short) ep-engine/src/ep.cc:949 (ep.so+0x0000000dce69)

Change-Id: I83db17ffce0d0a49cfe80f23a34e5dac25ede719
Reviewed-on: http://review.couchbase.org/63081
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
src/couch-kvstore/couch-kvstore.h

index 0fe266f..349d839 100644 (file)
@@ -20,6 +20,7 @@
 
 #include "config.h"
 #include "libcouchstore/couch_db.h"
+#include <relaxed_atomic.h>
 
 #include <map>
 #include <string>
@@ -638,7 +639,12 @@ private:
     EPStats &epStats;
     Configuration &configuration;
     const std::string dbname;
-    std::vector<uint64_t>dbFileRevMap;
+
+    // Map of the fileRev for each vBucket. Using RelaxedAtomic so
+    // stats gathering (doDcpVbTakeoverStats) can get a snapshot
+    // without having to lock.
+    std::vector<Couchbase::RelaxedAtomic<uint64_t> > dbFileRevMap;
+
     uint16_t numDbFiles;
     std::vector<CouchRequest *> pendingReqsQ;
     bool intransaction;