Skip to content

Commit c4d8abb

Browse files
abhinavdangetichiyoung
authored andcommitted
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 <[email protected]> Reviewed-by: Will Gardner <[email protected]> Tested-by: buildbot <[email protected]>
1 parent 2e4ce11 commit c4d8abb

File tree

2 files changed

+5
-5
lines changed

2 files changed

+5
-5
lines changed

src/ep.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2769,9 +2769,9 @@ int EventuallyPersistentStore::flushVBucket(uint16_t vbid) {
27692769
uint64_t commit_time = (end - start) / 1000000;
27702770
uint64_t trans_time = (end - flush_start) / 1000000;
27712771

2772-
lastTransTimePerItem = (items_flushed == 0) ? 0 :
2773-
static_cast<double>(trans_time) /
2774-
static_cast<double>(items_flushed);
2772+
lastTransTimePerItem.store((items_flushed == 0) ? 0 :
2773+
static_cast<double>(trans_time) /
2774+
static_cast<double>(items_flushed));
27752775
stats.commit_time.store(commit_time);
27762776
stats.cumulativeCommitTime.fetch_add(commit_time);
27772777
stats.cumulativeFlushTime.fetch_add(ep_current_time()

src/ep.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ class EventuallyPersistentStore {
625625
}
626626

627627
size_t getTransactionTimePerItem() {
628-
return lastTransTimePerItem;
628+
return lastTransTimePerItem.load();
629629
}
630630

631631
bool isFlushAllScheduled() {
@@ -878,7 +878,7 @@ class EventuallyPersistentStore {
878878
AtomicValue<size_t> replicaRatio;
879879
} cachedResidentRatio;
880880
size_t statsSnapshotTaskId;
881-
size_t lastTransTimePerItem;
881+
AtomicValue<size_t> lastTransTimePerItem;
882882
item_eviction_policy_t eviction_policy;
883883

884884
Mutex compactionLock;

0 commit comments

Comments
 (0)