common/perf_counters: use second atomic to make counters safe to read

Add a second counter so that we can detect a race with an add/inc during
read, and retry.

Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/common/perf_counters.cc b/src/common/perf_counters.cc
index 06edb41991038bfa4d59c4e56f013db9fa33cda3..61d893454edff636d8363568f7ecf91541ba2a7b 100644 (file)
--- a/src/common/perf_counters.cc
+++ b/src/common/perf_counters.cc
@@ -106,9 +106,13 @@ void PerfCounters::inc(int idx, uint64_t amt)
   perf_counter_data_any_d& data(m_data[idx - m_lower_bound - 1]);
   if (!(data.type & PERFCOUNTER_U64))
     return;
-  data.u64.add(amt);
-  if (data.type & PERFCOUNTER_LONGRUNAVG)
+  if (data.type & PERFCOUNTER_LONGRUNAVG) {
     data.avgcount.inc();
+    data.u64.add(amt);
+    data.avgcount2.inc();
+  } else {
+    data.u64.add(amt);
+  }
 }
 
 void PerfCounters::dec(int idx, uint64_t amt)
@@ -136,8 +140,13 @@ void PerfCounters::set(int idx, uint64_t amt)
   if (!(data.type & PERFCOUNTER_U64))
     return;
   data.u64.set(amt);
-  if (data.type & PERFCOUNTER_LONGRUNAVG)
+  if (data.type & PERFCOUNTER_LONGRUNAVG) {
     data.avgcount.inc();
+    data.u64.set(amt);
+    data.avgcount2.inc();
+  } else {
+    data.u64.set(amt);
+  }
 }
 
 uint64_t PerfCounters::get(int idx) const
@@ -163,9 +172,13 @@ void PerfCounters::tinc(int idx, utime_t amt)
   perf_counter_data_any_d& data(m_data[idx - m_lower_bound - 1]);
   if (!(data.type & PERFCOUNTER_TIME))
     return;
-  data.u64.add(amt.to_nsec());
-  if (data.type & PERFCOUNTER_LONGRUNAVG)
+  if (data.type & PERFCOUNTER_LONGRUNAVG) {
     data.avgcount.inc();
+    data.u64.add(amt.to_nsec());
+    data.avgcount2.inc();
+  } else {
+    data.u64.add(amt.to_nsec());
+  }
 }
 
 void PerfCounters::tset(int idx, utime_t amt)
@@ -209,7 +222,8 @@ pair<uint64_t, uint64_t> PerfCounters::get_tavg_ms(int idx) const
     return make_pair(0, 0);
   if (!(data.type & PERFCOUNTER_LONGRUNAVG))
     return make_pair(0, 0);
-  return make_pair(data.avgcount.read(), data.u64.read()/1000000);
+  pair<uint64_t,uint64_t> a = data.read_avg();
+  return make_pair(a.second, a.first / 1000000ull);
 }
 
 void PerfCounters::dump_formatted(Formatter *f, bool schema)
@@ -229,15 +243,15 @@ void PerfCounters::dump_formatted(Formatter *f, bool schema)
     } else {
       if (d->type & PERFCOUNTER_LONGRUNAVG) {
        f->open_object_section(d->name);
+       pair<uint64_t,uint64_t> a = d->read_avg();
        if (d->type & PERFCOUNTER_U64) {
-         f->dump_unsigned("avgcount", d->avgcount.read());
-         f->dump_unsigned("sum", d->u64.read());
+         f->dump_unsigned("avgcount", a.second);
+         f->dump_unsigned("sum", a.first);
        } else if (d->type & PERFCOUNTER_TIME) {
-         f->dump_unsigned("avgcount", d->avgcount.read());
-         uint64_t v = d->u64.read();
+         f->dump_unsigned("avgcount", a.second);
          f->dump_format_unquoted("sum", "%"PRId64".%09"PRId64,
-                                 v / 1000000000ull,
-                                 v % 1000000000ull);
+                                 a.first / 1000000000ull,
+                                 a.first % 1000000000ull);
        } else {
          assert(0);
        }

diff --git a/src/common/perf_counters.h b/src/common/perf_counters.h
index f4f650771bccbc40a82c51addce7e144e119d7b4..78c8684a2a58f2c6d3d35592bbbde2ac27ccfe29 100644 (file)
--- a/src/common/perf_counters.h
+++ b/src/common/perf_counters.h
@@ -115,14 +115,18 @@ private:
       : name(NULL),
        type(PERFCOUNTER_NONE),
        u64(0),
-       avgcount(0)
+       avgcount(0),
+       avgcount2(0)
     {}
     perf_counter_data_any_d(const perf_counter_data_any_d& other)
       : name(other.name),
        type(other.type),
-       u64(other.u64.read()),
-       avgcount(other.avgcount.read())
-    {}
+       u64(other.u64.read()) {
+      pair<uint64_t,uint64_t> a = other.read_avg();
+      u64.set(a.first);
+      avgcount.set(a.second);
+      avgcount2.set(a.second);
+    }
 
     void write_schema_json(char *buf, size_t buf_sz) const;
     void  write_json(char *buf, size_t buf_sz) const;
@@ -131,14 +135,27 @@ private:
     enum perfcounter_type_d type;
     atomic64_t u64;
     atomic64_t avgcount;
+    atomic64_t avgcount2;
 
     perf_counter_data_any_d& operator=(const perf_counter_data_any_d& other) {
       name = other.name;
       type = other.type;
-      u64.set(other.u64.read());
-      avgcount.set(other.avgcount.read());
+      pair<uint64_t,uint64_t> a = other.read_avg();
+      u64.set(a.first);
+      avgcount.set(a.second);
+      avgcount2.set(a.second);
       return *this;
     }
+
+    /// read <sum, count> safely
+    pair<uint64_t,uint64_t> read_avg() const {
+      uint64_t sum, count;
+      do {
+       count = avgcount.read();
+       sum = u64.read();
+      } while (avgcount2.read() != count);
+      return make_pair(sum, count);
+    }
   };
   typedef std::vector<perf_counter_data_any_d> perf_counter_data_vec_t;