From ef15b9d17857c17c7e7c432d9b9c7efe7845b451 Mon Sep 17 00:00:00 2001
From: Haobo Xu <haobo@fb.com>
Date: Fri, 24 May 2013 12:52:45 -0700
Subject: [PATCH] [RocksDB] Fix MaybeDumpStats

Summary: MaybeDumpStats was causing lock problem

Test Plan: make check; db_stress

Reviewers: dhruba

Reviewed By: dhruba

Differential Revision: https://reviews.facebook.net/D10935
---
 db/db_impl.cc | 31 +++++++++++++++++--------------
 db/db_impl.h  |  3 ++-
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/db/db_impl.cc b/db/db_impl.cc
index 1cec13464..563ed99eb 100644
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -305,20 +305,21 @@ const Status DBImpl::CreateArchivalDirectory() {
 }
 
 void DBImpl::MaybeDumpStats() {
-  mutex_.AssertHeld();
+  if (options_.stats_dump_period_sec == 0) return;
 
-  if (options_.stats_dump_period_sec != 0) {
-    const uint64_t now_micros = env_->NowMicros();
-    if (last_stats_dump_time_microsec_ +
-        options_.stats_dump_period_sec * 1000000
-        <= now_micros) {
-      last_stats_dump_time_microsec_ = now_micros;
-      mutex_.Unlock();
-      std::string stats;
-      GetProperty("leveldb.stats", &stats);
-      Log(options_.info_log, "%s", stats.c_str());
-      mutex_.Lock();
-    }
+  const uint64_t now_micros = env_->NowMicros();
+
+  if (last_stats_dump_time_microsec_ +
+      options_.stats_dump_period_sec * 1000000
+      <= now_micros) {
+    // Multiple threads could race in here simultaneously.
+    // However, the last one will update last_stats_dump_time_microsec_
+    // atomically. We could see more than one dump during one dump
+    // period in rare cases.
+    last_stats_dump_time_microsec_ = now_micros;
+    std::string stats;
+    GetProperty("leveldb.stats", &stats);
+    Log(options_.info_log, "%s", stats.c_str());
   }
 }
 
@@ -1233,6 +1234,9 @@ void DBImpl::TEST_PurgeObsoleteteWAL() {
 void DBImpl::BackgroundCall() {
   bool madeProgress = false;
   DeletionState deletion_state;
+
+  MaybeDumpStats();
+
   MutexLock l(&mutex_);
   // Log(options_.info_log, "XXX BG Thread %llx process new work item", pthread_self());
   assert(bg_compaction_scheduled_);
@@ -1273,7 +1277,6 @@ void DBImpl::BackgroundCall() {
   }
   bg_cv_.SignalAll();
 
-  MaybeDumpStats();
 }
 
 Status DBImpl::BackgroundCompaction(bool* madeProgress,
diff --git a/db/db_impl.h b/db/db_impl.h
index ba166b9ac..5a8d4fdbe 100644
--- a/db/db_impl.h
+++ b/db/db_impl.h
@@ -5,6 +5,7 @@
 #ifndef STORAGE_LEVELDB_DB_DB_IMPL_H_
 #define STORAGE_LEVELDB_DB_DB_IMPL_H_
 
+#include <atomic>
 #include <deque>
 #include <set>
 #include "db/dbformat.h"
@@ -277,7 +278,7 @@ class DBImpl : public DB {
   uint64_t purge_wal_files_last_run_;
 
   // last time stats were dumped to LOG
-  uint64_t last_stats_dump_time_microsec_;
+  std::atomic<uint64_t> last_stats_dump_time_microsec_;
 
   // These count the number of microseconds for which MakeRoomForWrite stalls.
   uint64_t stall_level0_slowdown_;