update metrics library after review

* move ticker kill
* use relaxed JSON in test
* rewrite loops in MockTick
* no need for atomics when protected by mutex
* add alive() to Tick API
* hook up total metrics
* add some documentation comments

3 files changed, 49 insertions, 40 deletions

diff --git a/staging_vespalib/src/tests/metrics/mock_tick.cpp b/staging_vespalib/src/tests/metrics/mock_tick.cpp
index e78b7c4644f..2178b5364e9 100644
--- a/staging_vespalib/src/tests/metrics/mock_tick.cpp
+++ b/staging_vespalib/src/tests/metrics/mock_tick.cpp
@@ -3,43 +3,49 @@
 
 namespace vespalib::metrics {
 
-const std::chrono::seconds oneSec{1};
-
 TimeStamp
 MockTick::next(TimeStamp prev)
 {
     std::unique_lock<std::mutex> locker(_lock);
     _prevValue = prev;
-    while (_runFlag) {
-        if (_provided) {
-            _blocked.store(false);
-            _provided.store(false);
-            return _nextValue;
-        }
-        _blocked.store(true);
-        _blockedCond.notify_all();
-        auto r = _providedCond.wait_for(locker, oneSec);
-        (void)r;
+    _blocked = true;
+    _blockedCond.notify_all();
+    while (_runFlag && !_provided) {
+        _providedCond.wait(locker);
+    }
+    _blocked = false;
+    if (_provided) {
+        _provided = false;
+        return _nextValue;
+    } else {
+        // killed
+        return TimeStamp(0);
     }
-    return TimeStamp(0);
 }
 
 void
 MockTick::kill()
 {
     std::unique_lock<std::mutex> locker(_lock);
-    _runFlag.store(false);
+    _runFlag = false;
     _blockedCond.notify_all();
     _providedCond.notify_all();
 }
 
+bool
+MockTick::alive()
+{
+    std::unique_lock<std::mutex> locker(_lock);
+    return _runFlag;
+}
+
 void
 MockTick::provide(TimeStamp value)
 {
     std::unique_lock<std::mutex> locker(_lock);
     _nextValue = value;
-    _blocked.store(false);
-    _provided.store(true);
+    _blocked = false;
+    _provided = true;
     _providedCond.notify_all();
 }
 
@@ -47,14 +53,15 @@ TimeStamp
 MockTick::waitUntilBlocked()
 {
     std::unique_lock<std::mutex> locker(_lock);
-    while (_runFlag) {
-        if (_blocked) {
-            return _prevValue;
-        }
-        auto r = _blockedCond.wait_for(locker, oneSec);
-        (void)r;
+    while (_runFlag && !_blocked) {
+        _blockedCond.wait(locker);
+    }
+    if (_blocked) {
+        return _prevValue;
+    } else {
+        // killed
+        return TimeStamp(0);
     }
-    return TimeStamp(0);
 }
 
 MockTick::MockTick()
diff --git a/staging_vespalib/src/tests/metrics/mock_tick.h b/staging_vespalib/src/tests/metrics/mock_tick.h
index 2f69f69fc91..e0336260880 100644
--- a/staging_vespalib/src/tests/metrics/mock_tick.h
+++ b/staging_vespalib/src/tests/metrics/mock_tick.h
@@ -12,9 +12,9 @@ namespace vespalib::metrics {
 class MockTick : public Tick {
 private:
     std::mutex _lock;
-    std::atomic<bool> _runFlag;
-    std::atomic<bool> _provided;
-    std::atomic<bool> _blocked;
+    bool _runFlag;
+    bool _provided;
+    bool _blocked;
     std::condition_variable _providedCond;
     std::condition_variable _blockedCond;
     TimeStamp _nextValue;
@@ -23,6 +23,7 @@ public:
     MockTick();
     TimeStamp next(TimeStamp prev) override;
     void kill() override;
+    bool alive() override;
 
     void provide(TimeStamp value);
     TimeStamp waitUntilBlocked();
@@ -33,6 +34,7 @@ struct TickProxy : Tick {
     TickProxy(std::shared_ptr<Tick> tick_in) : tick(std::move(tick_in)) {}
     TimeStamp next(TimeStamp prev) override { return tick->next(prev); }
     void kill() override { tick->kill(); }
+    bool alive() override { return tick->alive(); }
 };
 
 } // namespace vespalib::metrics
diff --git a/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp b/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp
index 80030cb89b2..463a285e7b3 100644
--- a/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp
+++ b/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp
@@ -90,24 +90,24 @@ fprintf(stderr, "bad json b\n");
 void check_json(const vespalib::string &actual)
 {
     vespalib::string expect = "{"
-    "   \"snapshot\": { \"from\": 1, \"to\": 4 },"
-    "   \"values\": [ { \"name\": \"foo\","
-    "       \"values\": { \"count\": 17, \"rate\": 4.85714 }"
+    "   snapshot: { from: 1, to: 4 },"
+    "   values: [ { name: 'foo',"
+    "       values: { count: 17, rate: 4.85714 }"
     "   }, {"
-    "       \"name\": \"foo\","
-    "       \"dimensions\": { \"chain\": \"default\", \"documenttype\": \"music\", \"thread\": \"0\" },"
-    "       \"values\": { \"count\": 4, \"rate\": 1.14286 }"
+    "       name: 'foo',"
+    "       dimensions: { chain: 'default', documenttype: 'music', thread: '0' },"
+    "       values: { count: 4, rate: 1.14286 }"
     "   }, {"
-    "       \"name\": \"bar\","
-    "       \"values\": { \"count\": 4, \"rate\": 1.14286, \"average\": 42, \"min\": 41, \"max\": 43, \"last\": 42 }"
+    "       name: 'bar',"
+    "       values: { count: 4, rate: 1.14286, average: 42, min: 41, max: 43, last: 42 }"
     "   }, {"
-    "       \"name\": \"bar\","
-    "       \"dimensions\": { \"chain\": \"vespa\", \"documenttype\": \"blogpost\", \"thread\": \"1\" },"
-    "       \"values\": { \"count\": 1, \"rate\": 0.285714, \"average\": 14, \"min\": 14, \"max\": 14, \"last\": 14 }"
+    "       name: 'bar',"
+    "       dimensions: { chain: 'vespa', documenttype: 'blogpost', thread: '1' },"
+    "       values: { count: 1, rate: 0.285714, average: 14, min: 14, max: 14, last: 14 }"
     "   }, {"
-    "       \"name\": \"bar\","
-    "       \"dimensions\": { \"chain\": \"vespa\", \"documenttype\": \"blogpost\", \"thread\": \"2\" },"
-    "       \"values\": { \"count\": 1, \"rate\": 0.285714, \"average\": 11, \"min\": 11, \"max\": 11, \"last\": 11 }"
+    "       name: 'bar',"
+    "       dimensions: { chain: 'vespa', documenttype: 'blogpost', thread: '2' },"
+    "       values: { count: 1, rate: 0.285714, average: 11, min: 11, max: 11, last: 11 }"
     "   } ]"
     "}";
     EXPECT_TRUE(compare_json(expect, actual));