MB-19246: Fix potentially incorrect persist_time in OBSERVE response

There is a data race in updating EPStore::lastTransTimePerItem. This
could result in an incorrect value for the persist_time field in an
OBSERVE response.

WARNING: ThreadSanitizer: data race (pid=4590)

  Write of size 8 at 0x7d540000fe88 by thread T8 (mutexes: write M11599):
    #0 EventuallyPersistentStore::flushVBucket(unsigned short) /home/abhinav/couchbase/ep-engine/src/ep.cc:3307 (ep.so+0x00000009954f)
    #1 Flusher::flushVB() /home/abhinav/couchbase/ep-engine/src/flusher.cc:296 (ep.so+0x0000000ff32f)
    #2 Flusher::step(GlobalTask*) /home/abhinav/couchbase/ep-engine/src/flusher.cc:186 (ep.so+0x0000000fd825)
    #3 FlusherTask::run() /home/abhinav/couchbase/ep-engine/src/tasks.cc:63 (ep.so+0x00000013bbb2)
    #4 ExecutorThread::run() /home/abhinav/couchbase/ep-engine/src/executorthread.cc:112 (ep.so+0x0000000f89b6)
    #5 launch_executor_thread(void*) /home/abhinav/couchbase/ep-engine/src/executorthread.cc:33 (ep.so+0x0000000f8555)
    #6 platform_thread_wrap /home/abhinav/couchbase/platform/src/cb_pthreads.c:23 (libplatform.so.0.1.0+0x000000003d31)

  Previous write of size 8 at 0x7d540000fe88 by thread T6 (mutexes: write M11602):
    #0 EventuallyPersistentStore::flushVBucket(unsigned short) /home/abhinav/couchbase/ep-engine/src/ep.cc:3307 (ep.so+0x00000009954f)
    #1 Flusher::flushVB() /home/abhinav/couchbase/ep-engine/src/flusher.cc:296 (ep.so+0x0000000ff32f)
    #2 Flusher::step(GlobalTask*) /home/abhinav/couchbase/ep-engine/src/flusher.cc:186 (ep.so+0x0000000fd825)
    #3 FlusherTask::run() /home/abhinav/couchbase/ep-engine/src/tasks.cc:63 (ep.so+0x00000013bbb2)
    #4 ExecutorThread::run() /home/abhinav/couchbase/ep-engine/src/executorthread.cc:112 (ep.so+0x0000000f89b6)
    #5 launch_executor_thread(void*) /home/abhinav/couchbase/ep-engine/src/executorthread.cc:33 (ep.so+0x0000000f8555)
    #6 platform_thread_wrap /home/abhinav/couchbase/platform/src/cb_pthreads.c:23 (libplatform.so.0.1.0+0x000000003d31)

Change-Id: Iccf1b0eacba495a8147fe81922361d566cb1d6a0
Reviewed-on: http://review.couchbase.org/62967
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Tested-by: buildbot <build@couchbase.com>

diff --git a/src/ep.cc b/src/ep.cc
index 050a396..76ca30c 100644 (file)
--- a/src/ep.cc
+++ b/src/ep.cc
@@ -2769,9 +2769,9 @@ int EventuallyPersistentStore::flushVBucket(uint16_t vbid) {
             uint64_t commit_time = (end - start) / 1000000;
             uint64_t trans_time = (end - flush_start) / 1000000;
 
-            lastTransTimePerItem = (items_flushed == 0) ? 0 :
-                static_cast<double>(trans_time) /
-                static_cast<double>(items_flushed);
+            lastTransTimePerItem.store((items_flushed == 0) ? 0 :
+                                       static_cast<double>(trans_time) /
+                                       static_cast<double>(items_flushed));
             stats.commit_time.store(commit_time);
             stats.cumulativeCommitTime.fetch_add(commit_time);
             stats.cumulativeFlushTime.fetch_add(ep_current_time()

diff --git a/src/ep.h b/src/ep.h
index d7702da..510d937 100644 (file)
--- a/src/ep.h
+++ b/src/ep.h
@@ -625,7 +625,7 @@ public:
     }
 
     size_t getTransactionTimePerItem() {
-        return lastTransTimePerItem;
+        return lastTransTimePerItem.load();
     }
 
     bool isFlushAllScheduled() {
@@ -878,7 +878,7 @@ private:
         AtomicValue<size_t> replicaRatio;
     } cachedResidentRatio;
     size_t statsSnapshotTaskId;
-    size_t lastTransTimePerItem;
+    AtomicValue<size_t> lastTransTimePerItem;
     item_eviction_policy_t eviction_policy;
 
     Mutex compactionLock;