From 1784b07c75c8888c809ca18dad4975d81a9215a8 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev Date: Thu, 27 May 2021 18:19:29 +0300 Subject: [PATCH 1/5] Registry refactored to search with O(1) --- metrics/registry.lua | 18 +++++------------- test/collectors_test.lua | 18 +++++++++++++----- test/lj_metrics_test.lua | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/metrics/registry.lua b/metrics/registry.lua index 1e995163..8dc4bda2 100644 --- a/metrics/registry.lua +++ b/metrics/registry.lua @@ -15,11 +15,7 @@ function Registry:clear() end function Registry:find(kind, name) - for _, c in ipairs(self.collectors) do - if c.name == name and c.kind == kind then - return c - end - end + return self.collectors[name .. kind] end function Registry:find_or_create(class, name, ...) @@ -32,22 +28,18 @@ end function Registry:register(collector) assert(collector ~= nil, 'Collector is empty') - assert(not is_empty(collector.name), "Collector''s name is empty") - assert(not is_empty(collector.kind), "Collector''s kind is empty") + assert(not is_empty(collector.name), "Collector's name is empty") + assert(not is_empty(collector.kind), "Collector's kind is empty") if self:find(collector.kind, collector.name) then error('Already registered') end collector:set_registry(self) - table.insert(self.collectors, collector) + self.collectors[collector.name .. collector.kind] = collector return collector end function Registry:unregister(collector) - for i, c in ipairs(self.collectors) do - if c.name == collector.name and c.kind == collector.kind then - table.remove(self.collectors, i) - end - end + self.collectors[collector.name .. collector.kind] = nil end function Registry:invoke_callbacks() diff --git a/test/collectors_test.lua b/test/collectors_test.lua index 16eadf62..62c683ba 100755 --- a/test/collectors_test.lua +++ b/test/collectors_test.lua @@ -21,6 +21,14 @@ g.after_each(function() metrics.clear() end) +local function len(tbl) + local l = 0 + for _ in pairs(tbl) do + l = l + 1 + end + return l +end + g.test_counter = function() t.assert_error_msg_contains("bad argument #1 to counter (string expected, got nil)", function() metrics.counter() @@ -38,7 +46,7 @@ g.test_counter = function() local collectors = metrics.collectors() local observations = metrics.collect() local obs = utils.find_obs('cnt', {}, observations) - t.assert_equals(#collectors, 1, 'counter seen as only collector') + t.assert_equals(len(collectors), 1, 'counter seen as only collector') t.assert_equals(obs.value, 8, '3 + 5 = 8 (via metrics.collectors())') t.assert_equals(c:collect()[1].value, 8, '3 + 5 = 8') @@ -70,7 +78,7 @@ g.test_gauge = function() local collectors = metrics.collectors() local observations = metrics.collect() local obs = utils.find_obs('gauge', {}, observations) - t.assert_equals(#collectors, 1, 'gauge seen as only collector') + t.assert_equals(len(collectors), 1, 'gauge seen as only collector') t.assert_equals(obs.value, -2, '3 - 5 = -2 (via metrics.collectors())') t.assert_equals(gauge:collect()[1].value, -2, '3 - 5 = -2') @@ -100,7 +108,7 @@ g.test_histogram = function() h:observe(5) local collectors = metrics.collectors() - t.assert_equals(#collectors, 1, 'histogram seen as only 1 collector') + t.assert_equals(len(collectors), 1, 'histogram seen as only 1 collector') local observations = metrics.collect() local obs_sum = utils.find_obs('hist_sum', {}, observations) local obs_count = utils.find_obs('hist_count', {}, observations) @@ -117,7 +125,7 @@ g.test_histogram = function() h:observe(3, { foo = 'bar' }) collectors = metrics.collectors() - t.assert_equals(#collectors, 1, 'still histogram seen as only 1 collector') + t.assert_equals(len(collectors), 1, 'still histogram seen as only 1 collector') observations = metrics.collect() obs_sum = utils.find_obs('hist_sum', { foo = 'bar' }, observations) obs_count = utils.find_obs('hist_count', { foo = 'bar' }, observations) @@ -145,7 +153,7 @@ g.test_counter_cache = function() local collectors = metrics.collectors() local observations = metrics.collect() local obs = utils.find_obs('cnt', {}, observations) - t.assert_equals(#collectors, 2, 'counter_1 and counter_2 refer to the same object') + t.assert_equals(len(collectors), 2, 'counter_1 and counter_2 refer to the same object') t.assert_equals(obs.value, 8, '3 + 5 = 8') obs = utils.find_obs('cnt2', {}, observations) t.assert_equals(obs.value, 7, 'counter_3 is the only reference to cnt2') diff --git a/test/lj_metrics_test.lua b/test/lj_metrics_test.lua index 9582bbbd..c9f1d727 100755 --- a/test/lj_metrics_test.lua +++ b/test/lj_metrics_test.lua @@ -50,5 +50,5 @@ g.test_lj_metrics = function() "lj_gc_steps_pause", } - t.assert_covers(lj_metrics, expected_lj_metrics) + t.assert_items_equals(lj_metrics, expected_lj_metrics) end From 8524a762d25da43c5cac361d4c4854e8cf0e0469 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev Date: Thu, 27 May 2021 18:47:47 +0300 Subject: [PATCH 2/5] Fix graphite test --- test/plugins/graphite_test.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/test/plugins/graphite_test.lua b/test/plugins/graphite_test.lua index 21242f17..9fac1381 100755 --- a/test/plugins/graphite_test.lua +++ b/test/plugins/graphite_test.lua @@ -83,6 +83,7 @@ g.test_graphite_format_observation_time_in_seconds = function() end g.test_graphite_sends_data_to_socket = function() + metrics.clear() local cnt = metrics.counter('test_cnt', 'test-cnt') cnt:inc(1) local port = 22003 From 129ad12250f03a6f7b9412f2aa88aee402036518 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev Date: Wed, 30 Jun 2021 17:20:58 +0300 Subject: [PATCH 3/5] Add metrics.clear() call before graphite tests --- test/plugins/graphite_test.lua | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/plugins/graphite_test.lua b/test/plugins/graphite_test.lua index 9fac1381..0b152517 100755 --- a/test/plugins/graphite_test.lua +++ b/test/plugins/graphite_test.lua @@ -33,6 +33,10 @@ g.before_all(function() metrics.enable_default_metrics(); end) +g.before_each(function() + metrics.clear() +end) + g.after_each(function() -- Delete all collectors and global labels metrics.clear() @@ -83,7 +87,6 @@ g.test_graphite_format_observation_time_in_seconds = function() end g.test_graphite_sends_data_to_socket = function() - metrics.clear() local cnt = metrics.counter('test_cnt', 'test-cnt') cnt:inc(1) local port = 22003 @@ -97,6 +100,7 @@ g.test_graphite_sends_data_to_socket = function() local obs_table = graphite_obs:split(' ') t.assert_equals(obs_table[1], 'tarantool.test_cnt') t.assert_equals(obs_table[2], '1') + sock:close() end local function mock_graphite_worker() From 5f4c5ec53cfeab129b1322dddd5aedb887af38a4 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev Date: Tue, 6 Jul 2021 14:20:45 +0300 Subject: [PATCH 4/5] Changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01f754f2..8ab3cad0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- metrics registry refactoring to search with `O(1)` + ### Fixed - be gentle to http routes, don't leave gaps in the array [#246](https://github.com/tarantool/metrics/issues/246) From f7535da8c1526ab70ad3fe08bd27d598c0a383dc Mon Sep 17 00:00:00 2001 From: Igor Zolotarev Date: Tue, 6 Jul 2021 14:36:26 +0300 Subject: [PATCH 5/5] Add link to issues --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ab3cad0..c3a28d34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed -- metrics registry refactoring to search with `O(1)` +- metrics registry refactoring to search with `O(1)` [#188](https://github.com/tarantool/metrics/issues/188) ### Fixed - be gentle to http routes, don't leave gaps in the array