From 70b757e310533b6ba97d53a6a13733b6ee480243 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Mon, 4 Dec 2017 11:10:55 +0000 Subject: 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 --- staging_vespalib/src/tests/metrics/mock_tick.cpp | 53 ++++++++++++---------- staging_vespalib/src/tests/metrics/mock_tick.h | 8 ++-- .../src/tests/metrics/simple_metrics_test.cpp | 28 ++++++------ 3 files changed, 49 insertions(+), 40 deletions(-) (limited to 'staging_vespalib/src/tests') 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 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 locker(_lock); - _runFlag.store(false); + _runFlag = false; _blockedCond.notify_all(); _providedCond.notify_all(); } +bool +MockTick::alive() +{ + std::unique_lock locker(_lock); + return _runFlag; +} + void MockTick::provide(TimeStamp value) { std::unique_lock 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 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 _runFlag; - std::atomic _provided; - std::atomic _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_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)); -- cgit v1.2.3