From 594884c36d0e92d3c89b3cf2f50c818d90a67488 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 3 Dec 2021 23:50:30 +0300 Subject: [PATCH] tests: use in-built stats instead of custom helper Use in-built `crud.stats()` info instead on `storage_stat` helper in tests to track map reduce calls. Part of #224 --- test/helper.lua | 10 ++ test/helpers/storage_stat.lua | 118 --------------------- test/integration/count_test.lua | 37 ++----- test/integration/ddl_sharding_key_test.lua | 40 +++---- test/integration/pairs_test.lua | 39 +++---- test/integration/select_test.lua | 37 ++----- 6 files changed, 59 insertions(+), 222 deletions(-) delete mode 100644 test/helpers/storage_stat.lua diff --git a/test/helper.lua b/test/helper.lua index 669dec07a..f355efed2 100644 --- a/test/helper.lua +++ b/test/helper.lua @@ -466,4 +466,14 @@ function helpers.reload_roles(srv) t.assert_equals({ok, err}, {true, nil}) end +function helpers.get_map_reduces_stat(router, space_name) + return router:eval([[ + local stats = require('crud').stats(...) + if stats.select == nil then + return 0 + end + return stats.select.details.map_reduces + ]], { space_name }) +end + return helpers diff --git a/test/helpers/storage_stat.lua b/test/helpers/storage_stat.lua deleted file mode 100644 index 2bb4dcd48..000000000 --- a/test/helpers/storage_stat.lua +++ /dev/null @@ -1,118 +0,0 @@ -local checks = require('checks') -local helpers = require('test.helper') - -local storage_stat = {} - --- Wrap crud's select_on_storage()/count_on_storage() --- function to count selects/counts and add storage_stat() --- function that returns resulting statistics. --- --- Call it after crud's initialization. -function storage_stat.init_on_storage(method_on_storage_name) - assert(_G._crud[method_on_storage_name] ~= nil) - - -- Here we count requests. - local storage_stat_table = { - requests = 0, - } - - -- Wrap method on storage function. - local requests_on_storage_saved = _G._crud[method_on_storage_name] - _G._crud[method_on_storage_name] = function(...) - local requests = storage_stat_table.requests - storage_stat_table.requests = requests + 1 - return requests_on_storage_saved(...) - end - - -- Accessor for the statistics. - rawset(_G, 'storage_stat', function() - return storage_stat_table - end) -end - -function storage_stat.init_on_storage_for_select() - storage_stat.init_on_storage('select_on_storage') -end - -function storage_stat.init_on_storage_for_count() - storage_stat.init_on_storage('count_on_storage') -end - --- Accumulate statistics from storages. --- --- The statistics is grouped by replicasets. --- --- Example of a return value: --- --- | { --- | ['s-1'] = { --- | select_requests = 1, --- | }, --- | ['s-2'] = { --- | select_requests = 0, --- | }, --- | } -function storage_stat.collect(cluster) - checks('table') - - local res = {} - - helpers.call_on_storages(cluster, function(server, replicaset) - checks('table', 'table') - - -- Collect the statistics. - local storage_stat = server.net_box:call('storage_stat') - - -- Initialize if needed. - if res[replicaset.alias] == nil then - res[replicaset.alias] = {} - end - - -- Accumulate the collected statistics. - for key, val in pairs(storage_stat) do - local old = res[replicaset.alias][key] or 0 - res[replicaset.alias][key] = old + val - end - end) - - return res -end - --- Difference between 'a' and 'b' storage statistics. --- --- The return value structure is the same as for --- storage_stat.collect(). -function storage_stat.diff(a, b) - checks('table', 'table') - - local diff = table.deepcopy(a) - - for replicaset_alias, stat_b in pairs(b) do - -- Initialize if needed. - if diff[replicaset_alias] == nil then - diff[replicaset_alias] = {} - end - - -- Substract 'b' statistics from 'a'. - for key, val in pairs(stat_b) do - local old = diff[replicaset_alias][key] or 0 - diff[replicaset_alias][key] = old - val - end - end - - return diff -end - --- Accepts collect (or diff) return value and returns --- total number of select/count requests across all storages. -function storage_stat.total(stats) - local total = 0 - - for _, stat in pairs(stats) do - total = total + (stat.requests or 0) - end - - return total -end - -return storage_stat diff --git a/test/integration/count_test.lua b/test/integration/count_test.lua index 9fb62a10a..008ea7888 100644 --- a/test/integration/count_test.lua +++ b/test/integration/count_test.lua @@ -4,7 +4,6 @@ local clock = require('clock') local t = require('luatest') local helpers = require('test.helper') -local storage_stat = require('test.helpers.storage_stat') local pgroup = t.group('count', { {engine = 'memtx'}, @@ -24,12 +23,9 @@ pgroup.before_all(function(g) g.cluster:start() - helpers.call_on_storages(g.cluster, function(server) - server.net_box:eval([[ - local storage_stat = require('test.helpers.storage_stat') - storage_stat.init_on_storage_for_count() - ]]) - end) + g.cluster:server('router').net_box:eval([[ + require('crud').cfg{ stats = true } + ]]) end) pgroup.after_all(function(g) helpers.stop_cluster(g.cluster) end) @@ -583,7 +579,8 @@ pgroup.test_count_no_map_reduce = function(g) }, }) - local stat_a = storage_stat.collect(g.cluster) + local router = g.cluster:server('router').net_box + local map_reduces_before = helpers.get_map_reduces_stat(router, 'customers') -- Case: no conditions, just bucket id. local result, err = g.cluster.main_server.net_box:call('crud.count', { @@ -594,15 +591,9 @@ pgroup.test_count_no_map_reduce = function(g) t.assert_equals(err, nil) t.assert_equals(result, 1) - local stat_b = storage_stat.collect(g.cluster) - t.assert_equals(storage_stat.diff(stat_b, stat_a), { - ['s-1'] = { - requests = 1, - }, - ['s-2'] = { - requests = 0, - }, - }) + local map_reduces_after_1 = helpers.get_map_reduces_stat(router, 'customers') + local diff_1 = map_reduces_after_1 - map_reduces_before + t.assert_equals(diff_1, 0, 'Count request was not a map reduce') -- Case: EQ on secondary index, which is not in the sharding -- index (primary index in the case). @@ -614,15 +605,9 @@ pgroup.test_count_no_map_reduce = function(g) t.assert_equals(err, nil) t.assert_equals(result, 1) - local stat_c = storage_stat.collect(g.cluster) - t.assert_equals(storage_stat.diff(stat_c, stat_b), { - ['s-1'] = { - requests = 0, - }, - ['s-2'] = { - requests = 1, - }, - }) + local map_reduces_after_2 = helpers.get_map_reduces_stat(router, 'customers') + local diff_2 = map_reduces_after_2 - map_reduces_after_1 + t.assert_equals(diff_2, 0, 'Count request was not a map reduce') end pgroup.test_count_timeout = function(g) diff --git a/test/integration/ddl_sharding_key_test.lua b/test/integration/ddl_sharding_key_test.lua index 8a874ad06..da02b54da 100644 --- a/test/integration/ddl_sharding_key_test.lua +++ b/test/integration/ddl_sharding_key_test.lua @@ -3,7 +3,6 @@ local crud = require('crud') local t = require('luatest') local helpers = require('test.helper') -local storage_stat = require('test.helpers.storage_stat') local ok = pcall(require, 'ddl') if not ok then @@ -35,12 +34,9 @@ pgroup.before_all(function(g) t.assert_equals(type(result), 'table') t.assert_equals(err, nil) - helpers.call_on_storages(g.cluster, function(server) - server.net_box:eval([[ - local storage_stat = require('test.helpers.storage_stat') - storage_stat.init_on_storage_for_select() - ]]) - end) + g.cluster.main_server.net_box:eval([[ + require('crud').cfg{ stats = true } + ]]) end) pgroup.after_all(function(g) helpers.stop_cluster(g.cluster) end) @@ -367,22 +363,19 @@ for name, case in pairs(cases) do pgroup[('test_%s_wont_lead_to_map_reduce'):format(name)] = function(g) case.prepare_data(g, case.space_name) - local stat_a = storage_stat.collect(g.cluster) + local router = g.cluster:server('router').net_box + local map_reduces_before = helpers.get_map_reduces_stat(router, case.space_name) - local result, err = g.cluster.main_server.net_box:call('crud.select', { + local result, err = router:call('crud.select', { case.space_name, case.conditions }) t.assert_equals(err, nil) t.assert_not_equals(result, nil) t.assert_equals(#result.rows, 1) - local stat_b = storage_stat.collect(g.cluster) - - -- Check a number of select() requests made by CRUD on cluster's storages - -- after calling select() on a router. Make sure only a single storage has - -- a single select() request. Otherwise we lead to map-reduce. - local stats = storage_stat.diff(stat_b, stat_a) - t.assert_equals(storage_stat.total(stats), 1, 'Select request was not a map reduce') + local map_reduces_after = helpers.get_map_reduces_stat(router, case.space_name) + local diff = map_reduces_after - map_reduces_before + t.assert_equals(diff, 0, 'Select request was not a map reduce') end end @@ -390,22 +383,19 @@ pgroup.test_select_for_part_of_sharding_key_will_lead_to_map_reduce = function(g local space_name = 'customers_name_age_key_different_indexes' prepare_data_name_age_sharding_key(g, space_name) - local stat_a = storage_stat.collect(g.cluster) + local router = g.cluster:server('router').net_box + local map_reduces_before = helpers.get_map_reduces_stat(router, space_name) - local result, err = g.cluster.main_server.net_box:call('crud.select', { + local result, err = router:call('crud.select', { space_name, {{'==', 'age', 58}}, }) t.assert_equals(err, nil) t.assert_not_equals(result, nil) t.assert_equals(#result.rows, 1) - local stat_b = storage_stat.collect(g.cluster) - - -- Check a number of select() requests made by CRUD on cluster's storages - -- after calling select() on a router. Make sure it was a map-reduce - -- since we do not have sharding key values in conditions. - local stats = storage_stat.diff(stat_b, stat_a) - t.assert_equals(storage_stat.total(stats), 2, 'Select request was a map reduce') + local map_reduces_after = helpers.get_map_reduces_stat(router, space_name) + local diff = map_reduces_after - map_reduces_before + t.assert_equals(diff, 1, 'Select request was a map reduce') end pgroup.test_select_secondary_idx = function(g) diff --git a/test/integration/pairs_test.lua b/test/integration/pairs_test.lua index aacb052fb..a0c906183 100644 --- a/test/integration/pairs_test.lua +++ b/test/integration/pairs_test.lua @@ -5,7 +5,6 @@ local t = require('luatest') local crud_utils = require('crud.common.utils') local helpers = require('test.helper') -local storage_stat = require('test.helpers.storage_stat') local pgroup = t.group('pairs', { {engine = 'memtx'}, @@ -27,12 +26,9 @@ pgroup.before_all(function(g) g.space_format = g.cluster.servers[2].net_box.space.customers:format() - helpers.call_on_storages(g.cluster, function(server) - server.net_box:eval([[ - local storage_stat = require('test.helpers.storage_stat') - storage_stat.init_on_storage_for_select() - ]]) - end) + g.cluster.main_server.net_box:eval([[ + require('crud').cfg{ stats = true } + ]]) end) pgroup.after_all(function(g) helpers.stop_cluster(g.cluster) end) @@ -842,10 +838,11 @@ pgroup.test_pairs_no_map_reduce = function(g) table.sort(customers, function(obj1, obj2) return obj1.id < obj2.id end) - local stat_a = storage_stat.collect(g.cluster) + local router = g.cluster:server('router').net_box + local map_reduces_before = helpers.get_map_reduces_stat(router, 'customers') -- Case: no conditions, just bucket id. - local rows = g.cluster.main_server.net_box:eval([[ + local rows = router:eval([[ local crud = require('crud') return crud.pairs(...):totable() @@ -858,15 +855,9 @@ pgroup.test_pairs_no_map_reduce = function(g) {3, 2804, 'David', 'Smith', 33, 'Los Angeles'}, }) - local stat_b = storage_stat.collect(g.cluster) - t.assert_equals(storage_stat.diff(stat_b, stat_a), { - ['s-1'] = { - requests = 1, - }, - ['s-2'] = { - requests = 0, - }, - }) + local map_reduces_after_1 = helpers.get_map_reduces_stat(router, 'customers') + local diff_1 = map_reduces_after_1 - map_reduces_before + t.assert_equals(diff_1, 0, 'Select request was not a map reduce') -- Case: EQ on secondary index, which is not in the sharding -- index (primary index in the case). @@ -883,13 +874,7 @@ pgroup.test_pairs_no_map_reduce = function(g) {4, 1161, 'William', 'White', 81, 'Chicago'}, }) - local stat_c = storage_stat.collect(g.cluster) - t.assert_equals(storage_stat.diff(stat_c, stat_b), { - ['s-1'] = { - requests = 0, - }, - ['s-2'] = { - requests = 1, - }, - }) + local map_reduces_after_2 = helpers.get_map_reduces_stat(router, 'customers') + local diff_2 = map_reduces_after_2 - map_reduces_after_1 + t.assert_equals(diff_2, 0, 'Select request was not a map reduce') end diff --git a/test/integration/select_test.lua b/test/integration/select_test.lua index f42896f3b..97eadf35d 100644 --- a/test/integration/select_test.lua +++ b/test/integration/select_test.lua @@ -6,7 +6,6 @@ local crud = require('crud') local crud_utils = require('crud.common.utils') local helpers = require('test.helper') -local storage_stat = require('test.helpers.storage_stat') local pgroup = t.group('select', { {engine = 'memtx'}, @@ -28,12 +27,9 @@ pgroup.before_all(function(g) g.space_format = g.cluster.servers[2].net_box.space.customers:format() - helpers.call_on_storages(g.cluster, function(server) - server.net_box:eval([[ - local storage_stat = require('test.helpers.storage_stat') - storage_stat.init_on_storage_for_select() - ]]) - end) + g.cluster:server('router').net_box:eval([[ + require('crud').cfg{ stats = true } + ]]) end) pgroup.after_all(function(g) helpers.stop_cluster(g.cluster) end) @@ -1624,7 +1620,8 @@ pgroup.test_select_no_map_reduce = function(g) table.sort(customers, function(obj1, obj2) return obj1.id < obj2.id end) - local stat_a = storage_stat.collect(g.cluster) + local router = g.cluster:server('router').net_box + local map_reduces_before = helpers.get_map_reduces_stat(router, 'customers') -- Case: no conditions, just bucket id. local result, err = g.cluster.main_server.net_box:call('crud.select', { @@ -1637,15 +1634,9 @@ pgroup.test_select_no_map_reduce = function(g) {3, 2804, 'David', 'Smith', 33, 'Los Angeles'}, }) - local stat_b = storage_stat.collect(g.cluster) - t.assert_equals(storage_stat.diff(stat_b, stat_a), { - ['s-1'] = { - requests = 1, - }, - ['s-2'] = { - requests = 0, - }, - }) + local map_reduces_after_1 = helpers.get_map_reduces_stat(router, 'customers') + local diff_1 = map_reduces_after_1 - map_reduces_before + t.assert_equals(diff_1, 0, 'Select request was not a map reduce') -- Case: EQ on secondary index, which is not in the sharding -- index (primary index in the case). @@ -1659,13 +1650,7 @@ pgroup.test_select_no_map_reduce = function(g) {4, 1161, 'William', 'White', 81, 'Chicago'}, }) - local stat_c = storage_stat.collect(g.cluster) - t.assert_equals(storage_stat.diff(stat_c, stat_b), { - ['s-1'] = { - requests = 0, - }, - ['s-2'] = { - requests = 1, - }, - }) + local map_reduces_after_2 = helpers.get_map_reduces_stat(router, 'customers') + local diff_2 = map_reduces_after_2 - map_reduces_after_1 + t.assert_equals(diff_2, 0, 'Select request was not a map reduce') end