diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-03-01 06:38:06 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2023-03-01 17:02:23 +0000 |
commit | f2e5ba70fcacd7dfc6d5688479e220c975d26f58 (patch) | |
tree | 2b9470f0da3da72b6c1915db75e81b000a31b1e4 /metrics | |
parent | 87aa63c8c6f7544b648401e1836d8e66af000370 (diff) |
Set period in constructor only
Diffstat (limited to 'metrics')
-rw-r--r-- | metrics/src/tests/metricmanagertest.cpp | 28 | ||||
-rw-r--r-- | metrics/src/vespa/metrics/metricmanager.cpp | 27 | ||||
-rw-r--r-- | metrics/src/vespa/metrics/metricmanager.h | 4 | ||||
-rw-r--r-- | metrics/src/vespa/metrics/updatehook.h | 18 |
4 files changed, 38 insertions, 39 deletions
diff --git a/metrics/src/tests/metricmanagertest.cpp b/metrics/src/tests/metricmanagertest.cpp index e398e252a36..6674da98452 100644 --- a/metrics/src/tests/metricmanagertest.cpp +++ b/metrics/src/tests/metricmanagertest.cpp @@ -950,8 +950,8 @@ namespace { std::mutex& _output_mutex; FakeTimer& _timer; - MyUpdateHook(std::ostringstream& output, std::mutex& output_mutex, const char* name, FakeTimer& timer) - : UpdateHook(name), + MyUpdateHook(std::ostringstream& output, std::mutex& output_mutex, const char* name, vespalib::duration period, FakeTimer& timer) + : UpdateHook(name, period), _output(output), _output_mutex(output_mutex), _timer(timer) @@ -979,12 +979,12 @@ TEST_F(MetricManagerTest, test_update_hooks) mm.registerMetric(lockGuard, mySet.set); } - MyUpdateHook preInitShort(output, output_mutex, "BIS", timer); - MyUpdateHook preInitLong(output, output_mutex, "BIL", timer); - MyUpdateHook preInitInfinite(output, output_mutex, "BII", timer); - mm.addMetricUpdateHook(preInitShort, 5); - mm.addMetricUpdateHook(preInitLong, 300); - mm.addMetricUpdateHook(preInitInfinite, 0); + MyUpdateHook preInitShort(output, output_mutex, "BIS", 5s, timer); + MyUpdateHook preInitLong(output, output_mutex, "BIL", 300s, timer); + MyUpdateHook preInitInfinite(output, output_mutex, "BII", 0s, timer); + mm.addMetricUpdateHook(preInitShort); + mm.addMetricUpdateHook(preInitLong); + mm.addMetricUpdateHook(preInitInfinite); // All hooks should get called during initialization @@ -1000,12 +1000,12 @@ TEST_F(MetricManagerTest, test_update_hooks) "consumer[1].tags[0] snaptest\n")); output << "Init done\n"; - MyUpdateHook postInitShort(output, output_mutex, "AIS", timer); - MyUpdateHook postInitLong(output, output_mutex, "AIL", timer); - MyUpdateHook postInitInfinite(output, output_mutex, "AII", timer); - mm.addMetricUpdateHook(postInitShort, 5); - mm.addMetricUpdateHook(postInitLong, 400); - mm.addMetricUpdateHook(postInitInfinite, 0); + MyUpdateHook postInitShort(output, output_mutex, "AIS", 5s, timer); + MyUpdateHook postInitLong(output, output_mutex, "AIL", 400s, timer); + MyUpdateHook postInitInfinite(output, output_mutex, "AII", 0s, timer); + mm.addMetricUpdateHook(postInitShort); + mm.addMetricUpdateHook(postInitLong); + mm.addMetricUpdateHook(postInitInfinite); // After 5 seconds the short ones should get another. diff --git a/metrics/src/vespa/metrics/metricmanager.cpp b/metrics/src/vespa/metrics/metricmanager.cpp index f6601e710fc..0102d98e9c7 100644 --- a/metrics/src/vespa/metrics/metricmanager.cpp +++ b/metrics/src/vespa/metrics/metricmanager.cpp @@ -115,29 +115,26 @@ MetricManager::stop() } void -MetricManager::addMetricUpdateHook(UpdateHook& hook, uint32_t period) +MetricManager::addMetricUpdateHook(UpdateHook& hook) { - hook.setPeriod(vespalib::from_s(period)); hook.updateNextCall(_timer->getTime()); std::lock_guard sync(_waiter); - // If we've already initialized manager, log period has been set. - // In this case. Call first time after period - if (period == 0) { - for (UpdateHook * sHook : _snapshotUpdateHooks) { - if (sHook == &hook) { + if ( hook.is_periodic() ) { + for (UpdateHook * pHook : _periodicUpdateHooks) { + if (pHook == &hook) { LOG(warning, "Update hook already registered"); return; } } - _snapshotUpdateHooks.push_back(&hook); + _periodicUpdateHooks.push_back(&hook); } else { - for (UpdateHook * pHook : _periodicUpdateHooks) { - if (pHook == &hook) { + for (UpdateHook * sHook : _snapshotUpdateHooks) { + if (sHook == &hook) { LOG(warning, "Update hook already registered"); return; } } - _periodicUpdateHooks.push_back(&hook); + _snapshotUpdateHooks.push_back(&hook); } } @@ -146,16 +143,16 @@ MetricManager::removeMetricUpdateHook(UpdateHook& hook) { std::lock_guard sync(_waiter); if (hook.is_periodic()) { - for (auto it = _snapshotUpdateHooks.begin(); it != _snapshotUpdateHooks.end(); it++) { + for (auto it = _periodicUpdateHooks.begin(); it != _periodicUpdateHooks.end(); it++) { if (*it == &hook) { - _snapshotUpdateHooks.erase(it); + _periodicUpdateHooks.erase(it); return; } } } else { - for (auto it = _periodicUpdateHooks.begin(); it != _periodicUpdateHooks.end(); it++) { + for (auto it = _snapshotUpdateHooks.begin(); it != _snapshotUpdateHooks.end(); it++) { if (*it == &hook) { - _periodicUpdateHooks.erase(it); + _snapshotUpdateHooks.erase(it); return; } } diff --git a/metrics/src/vespa/metrics/metricmanager.h b/metrics/src/vespa/metrics/metricmanager.h index db5bb5090f7..a53bad72641 100644 --- a/metrics/src/vespa/metrics/metricmanager.h +++ b/metrics/src/vespa/metrics/metricmanager.h @@ -136,7 +136,7 @@ public: * snapshotting and metric logging, to make the metrics the best as they can * be at those occasions. * - * @param period Period in seconds for how often callback should be called. + * @param period Period for how often callback should be called. * The default value of 0, means only before snapshotting or * logging, while another value will give callbacks each * period seconds. Expensive metrics to calculate will @@ -145,7 +145,7 @@ public: * seconds or so. Any value of period >= the smallest snapshot * time will behave identically as if period is set to 0. */ - void addMetricUpdateHook(UpdateHook&, uint32_t period); + void addMetricUpdateHook(UpdateHook&); /** Remove a metric update hook so it won't get any more updates. */ void removeMetricUpdateHook(UpdateHook&); diff --git a/metrics/src/vespa/metrics/updatehook.h b/metrics/src/vespa/metrics/updatehook.h index a8f158f713e..997bdf0b5a4 100644 --- a/metrics/src/vespa/metrics/updatehook.h +++ b/metrics/src/vespa/metrics/updatehook.h @@ -28,24 +28,26 @@ class MetricManager; class UpdateHook { public: using MetricLockGuard = metrics::MetricLockGuard; - UpdateHook(const char* name) : _name(name), _nextCall(), _period(vespalib::duration::zero()) {} + UpdateHook(const char* name, vespalib::duration period) + : _name(name), + _period(period), + _nextCall() + {} virtual ~UpdateHook() = default; virtual void updateMetrics(const MetricLockGuard & guard) = 0; const char* getName() const { return _name; } void updateNextCall() { updateNextCall(_nextCall); } - void updateNextCall(time_point now) { _nextCall = now + _period; } - bool is_periodic() const noexcept { return _period == vespalib::duration::zero(); } + void updateNextCall(time_point now) { setNextCall(now + _period); } + bool is_periodic() const noexcept { return _period != vespalib::duration::zero(); } bool expired(time_point now) { return _nextCall <= now; } bool has_valid_expiry() const noexcept { return _nextCall != time_point(); } vespalib::duration getPeriod() const noexcept { return _period; } time_point getNextCall() const noexcept { return _nextCall; } - // This should be moved to constructor - void setPeriod(vespalib::duration period) { _period = period; } void setNextCall(time_point now) { _nextCall = now; } private: - const char* _name; - time_point _nextCall; - vespalib::duration _period; + const char* _name; + const vespalib::duration _period; + time_point _nextCall; }; } |