From c18f70db87ec058ede37c8e3fdf03ae73e4a0715 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 24 Dec 2021 20:10:34 +0300 Subject: [PATCH] Add schema reload counter to stats Add counter of schema reloads in calls to module statistics. Schema reloads are counted over all spaces and stored in top level, like `space_not_found` counter. ``` crud.stats().schema_reloads --- - 2 ... ``` In metrics, they are stored in `tnt_crud_schema_reloads` counter. Follows up #224 --- README.md | 8 +++- crud/common/fiber_context.lua | 22 +++++++++++ crud/common/schema.lua | 4 ++ crud/common/utils.lua | 18 --------- crud/select.lua | 5 ++- crud/select/compat/select.lua | 3 +- crud/select/compat/select_old.lua | 3 +- crud/select/executor.lua | 3 +- crud/stats/local_registry.lua | 18 +++++++++ crud/stats/metrics_registry.lua | 36 ++++++++++++++++-- crud/stats/module.lua | 11 +++++- test/entrypoint/srv_stats.lua | 39 +++++++++++++++++++ test/integration/stats_test.lua | 62 +++++++++++++++++++++++++++++-- test/unit/stats_test.lua | 47 +++++++++++++++++++++++ 14 files changed, 245 insertions(+), 34 deletions(-) create mode 100644 crud/common/fiber_context.lua diff --git a/README.md b/README.md index 8ec7c9b60..11fd7c162 100644 --- a/README.md +++ b/README.md @@ -638,6 +638,7 @@ crud.stats() count: 4 time: 0.000004 space_not_found: 0 + schema_reloads: 0 ... crud.stats('my_space') --- @@ -658,7 +659,9 @@ field will be empty. If no operations has been called for a space, it will not be represented. Operations called for spaces not found both on storages and in statistics registry (including cases when crud can't verify space existence) -will increment `space_not_found` counter. +will increment `space_not_found` counter. Operation calls +that reloaded a schema will increment `schema_reloads` +counter. Possible statistics operation labels are `insert` (for `insert` and `insert_object` calls), @@ -678,7 +681,8 @@ otherwise `latency` is total average. In [`metrics`](https://www.tarantool.io/en/doc/latest/book/monitoring/) registry statistics are stored as `tnt_crud_stats` metrics with `operation`, `status` and `name` labels. Collector -`tnt_crud_space_not_found` stores count of calls to unknown spaces. +`tnt_crud_space_not_found` stores count of calls to unknown spaces, +`tnt_crud_schema_reloads` stores count of schema reloads in calls. ``` metrics:collect() --- diff --git a/crud/common/fiber_context.lua b/crud/common/fiber_context.lua new file mode 100644 index 000000000..9ba974b2e --- /dev/null +++ b/crud/common/fiber_context.lua @@ -0,0 +1,22 @@ +local fiber = require('fiber') + +local fiber_context = {} + +function fiber_context.init_section(section) + local storage = fiber.self().storage + if storage[section] == nil then + storage[section] = {} + end + + return storage[section] +end + +function fiber_context.get_section(section) + return fiber.self().storage[section] +end + +function fiber_context.drop_section(section) + fiber.self().storage[section] = nil +end + +return fiber_context diff --git a/crud/common/schema.lua b/crud/common/schema.lua index 25375ff4f..fc1d702fa 100644 --- a/crud/common/schema.lua +++ b/crud/common/schema.lua @@ -9,6 +9,7 @@ local ReloadSchemaError = errors.new_class('ReloadSchemaError', {capture_stack = local const = require('crud.common.const') local dev_checks = require('crud.common.dev_checks') +local fiber_context = require('crud.common.fiber_context') local schema = {} @@ -103,6 +104,9 @@ function schema.wrap_func_reload(func, ...) end end + local context_stats = fiber_context.init_section('router_stats') + context_stats.schema_reloads = i + return res, err end diff --git a/crud/common/utils.lua b/crud/common/utils.lua index b5df46423..23c11fbe3 100644 --- a/crud/common/utils.lua +++ b/crud/common/utils.lua @@ -1,6 +1,5 @@ local errors = require('errors') local ffi = require('ffi') -local fiber = require('fiber') local vshard = require('vshard') local fun = require('fun') @@ -614,21 +613,4 @@ end -- Do nothing. function utils.pass() end -function utils.init_context_section(section) - local storage = fiber.self().storage - if storage[section] == nil then - storage[section] = {} - end - - return storage[section] -end - -function utils.get_context_section(section) - return fiber.self().storage[section] -end - -function utils.drop_context_section(section) - fiber.self().storage[section] = nil -end - return utils diff --git a/crud/select.lua b/crud/select.lua index dcea2456e..f2650cf91 100644 --- a/crud/select.lua +++ b/crud/select.lua @@ -1,5 +1,6 @@ local errors = require('errors') +local fiber_context = require('crud.common.fiber_context') local utils = require('crud.common.utils') local select_executor = require('crud.select.executor') local select_filters = require('crud.select.filters') @@ -76,8 +77,8 @@ local function select_on_storage(space_name, index_id, conditions, opts) cursor = make_cursor(tuples) end - cursor.stats = utils.get_context_section('storage_stats') - utils.drop_context_section('storage_stats') + cursor.stats = fiber_context.get_section('storage_stats') + fiber_context.drop_section('storage_stats') -- getting tuples with user defined fields (if `fields` option is specified) -- and fields that are needed for comparison on router (primary key + scan key) diff --git a/crud/select/compat/select.lua b/crud/select/compat/select.lua index 210b483d8..88cbbf6cc 100644 --- a/crud/select/compat/select.lua +++ b/crud/select/compat/select.lua @@ -2,6 +2,7 @@ local checks = require('checks') local errors = require('errors') local vshard = require('vshard') +local fiber_context = require('crud.common.fiber_context') local utils = require('crud.common.utils') local sharding = require('crud.common.sharding') local dev_checks = require('crud.common.dev_checks') @@ -113,7 +114,7 @@ local function build_select_iterator(space_name, user_conditions, opts) return nil, err, true end else - local context_stats = utils.init_context_section('router_stats') + local context_stats = fiber_context.init_section('router_stats') context_stats.map_reduces = 1 end diff --git a/crud/select/compat/select_old.lua b/crud/select/compat/select_old.lua index 497e5e413..2b407bdff 100644 --- a/crud/select/compat/select_old.lua +++ b/crud/select/compat/select_old.lua @@ -4,6 +4,7 @@ local vshard = require('vshard') local fun = require('fun') local call = require('crud.common.call') +local fiber_context = require('crud.common.fiber_context') local utils = require('crud.common.utils') local sharding = require('crud.common.sharding') local dev_checks = require('crud.common.dev_checks') @@ -148,7 +149,7 @@ local function build_select_iterator(space_name, user_conditions, opts) return nil, err, true end else - local context_stats = utils.init_context_section('router_stats') + local context_stats = fiber_context.init_section('router_stats') context_stats.map_reduces = 1 end diff --git a/crud/select/executor.lua b/crud/select/executor.lua index 3aed25cb5..285bd359d 100644 --- a/crud/select/executor.lua +++ b/crud/select/executor.lua @@ -1,6 +1,7 @@ local errors = require('errors') local dev_checks = require('crud.common.dev_checks') +local fiber_context = require('crud.common.fiber_context') local select_comparators = require('crud.compare.comparators') local compat = require('crud.common.compat') local has_keydef = compat.exists('tuple.keydef', 'key_def') @@ -68,7 +69,7 @@ function executor.execute(space, index, filter_func, opts) opts = opts or {} - local stats = utils.init_context_section('storage_stats') + local stats = fiber_context.init_section('storage_stats') stats.tuples_lookup = 0 stats.tuples_fetched = 0 diff --git a/crud/stats/local_registry.lua b/crud/stats/local_registry.lua index a322385db..1a5de6288 100644 --- a/crud/stats/local_registry.lua +++ b/crud/stats/local_registry.lua @@ -31,6 +31,7 @@ function registry.init(opts) internal.registry = {} internal.registry.spaces = {} internal.registry.space_not_found = 0 + internal.registry.schema_reloads = 0 return true end @@ -188,4 +189,21 @@ function registry.observe_map_reduces(count, space_name) return true end +--- Increase statistics of schema reloads +-- +-- @function observe_schema_reloads +-- +-- @tparam number count +-- Schema reloads performed. +-- +-- @treturn boolean Returns true. +-- +function registry.observe_schema_reloads(count) + dev_checks('number') + + internal.registry.schema_reloads = internal.registry.schema_reloads + count + + return true +end + return registry diff --git a/crud/stats/metrics_registry.lua b/crud/stats/metrics_registry.lua index 800160042..16ba168df 100644 --- a/crud/stats/metrics_registry.lua +++ b/crud/stats/metrics_registry.lua @@ -20,6 +20,9 @@ local metric_name = { -- Counter collector for spaces not found. space_not_found = 'tnt_crud_space_not_found', + -- Counter collector for schema reloads. + schema_reloads = 'tnt_crud_schema_reloads', + -- Counter collectors for select/pairs details. details = { tuples_fetched = 'tnt_crud_tuples_fetched', @@ -103,6 +106,10 @@ function registry.init(opts) metric_name.space_not_found, 'Spaces not found during CRUD calls') + internal.registry[metric_name.schema_reloads] = metrics.counter( + metric_name.schema_reloads, + 'Schema reloads performed in operation calls') + internal.registry[metric_name.details.tuples_fetched] = metrics.counter( metric_name.details.tuples_fetched, 'Tuples fetched from CRUD storages during select/pairs') @@ -203,6 +210,7 @@ function registry.get(space_name) local stats = { spaces = {}, space_not_found = 0, + schema_reloads = 0, } -- Fill operation basic statistics values. @@ -257,9 +265,14 @@ function registry.get(space_name) return stats.spaces[space_name] or {} end - local _, obs = next(internal.registry[metric_name.space_not_found]:collect()) - if obs ~= nil then - stats.space_not_found = obs.value + local _, not_found_obs = next(internal.registry[metric_name.space_not_found]:collect()) + if not_found_obs ~= nil then + stats.space_not_found = not_found_obs.value + end + + local _, reload_obs = next(internal.registry[metric_name.schema_reloads]:collect()) + if reload_obs ~= nil then + stats.schema_reloads = reload_obs.value end return stats @@ -389,6 +402,23 @@ function registry.observe_map_reduces(count, space_name) return true end +--- Increase statistics of schema reloads +-- +-- @function observe_schema_reloads +-- +-- @tparam number count +-- Schema reloads performed. +-- +-- @treturn boolean Returns true. +-- +function registry.observe_schema_reloads(count) + dev_checks('number') + + internal.registry[metric_name.schema_reloads]:inc(count) + + return true +end + -- Workaround for https://github.com/tarantool/metrics/issues/334 . -- This workaround does not prevent observations reset between role reloads, -- but it fixes collector unlink from registry. Without this workaround, diff --git a/crud/stats/module.lua b/crud/stats/module.lua index 52dd20ff9..e4465d604 100644 --- a/crud/stats/module.lua +++ b/crud/stats/module.lua @@ -4,6 +4,7 @@ local errors = require('errors') local vshard = require('vshard') local dev_checks = require('crud.common.dev_checks') +local fiber_context = require('crud.common.fiber_context') local utils = require('crud.common.utils') local op_module = require('crud.stats.operation') local stash = require('crud.stats.stash') @@ -181,7 +182,7 @@ local function wrap_tail(space_name, op, opts, start_time, call_status, ...) err = second_return_val end - local context_stats = utils.get_context_section('router_stats') + local context_stats = fiber_context.get_section('router_stats') -- Describe local variables to use `goto`. local space_not_found_msg, space @@ -226,7 +227,13 @@ local function wrap_tail(space_name, op, opts, start_time, call_status, ...) internal.registry.observe_map_reduces( context_stats.map_reduces, space_name) end - utils.drop_context_section('router_stats') + + if context_stats.schema_reloads ~= nil then + internal.registry.observe_schema_reloads( + context_stats.schema_reloads) + end + + fiber_context.drop_section('router_stats') end :: return_values :: diff --git a/test/entrypoint/srv_stats.lua b/test/entrypoint/srv_stats.lua index b8bd813fb..ffa69d8c1 100755 --- a/test/entrypoint/srv_stats.lua +++ b/test/entrypoint/srv_stats.lua @@ -40,6 +40,45 @@ package.preload['customers-storage'] = function() unique = false, if_not_exists = true, }) + + rawset(_G, 'create_space', function() + local change_customers_space = box.schema.space.create('change_customers', { + format = { + {name = 'id', type = 'unsigned'}, + {name = 'bucket_id', type = 'unsigned'}, + {name = 'value', type = 'string'}, + }, + if_not_exists = true, + engine = engine, + }) + + -- primary index + change_customers_space:create_index('id_index', { + parts = { {field = 'id'} }, + if_not_exists = true, + }) + end) + + rawset(_G, 'create_bucket_id_index', function() + box.space.change_customers:create_index('bucket_id', { + parts = { {field = 'bucket_id'} }, + if_not_exists = true, + unique = false, + }) + end) + + rawset(_G, 'set_value_type_to_unsigned', function() + local new_format = {} + + for _, field_format in ipairs(box.space.change_customers:format()) do + if field_format.name == 'value' then + field_format.type = 'unsigned' + end + table.insert(new_format, field_format) + end + + box.space.change_customers:format(new_format) + end) end, } end diff --git a/test/integration/stats_test.lua b/test/integration/stats_test.lua index 6c206fcd9..b297b4cdc 100644 --- a/test/integration/stats_test.lua +++ b/test/integration/stats_test.lua @@ -19,6 +19,7 @@ local space_id = 542 local space_name = 'customers' local unknown_space_name = 'non_existing_space' local new_space_name = 'newspace' +local schema_change_space_name = 'change_customers' local function before_all(g) g.cluster = helpers.Cluster:new({ @@ -64,6 +65,7 @@ local function before_each(g) enable_stats(g) helpers.truncate_space_on_cluster(g.cluster, space_name) helpers.drop_space_on_cluster(g.cluster, new_space_name) + helpers.drop_space_on_cluster(g.cluster, schema_change_space_name) end local function get_metrics(g) @@ -609,6 +611,50 @@ for name, case in pairs(select_cases) do end end +-- luacheck: max comment line length 150 +-- Based on https://github.com/tarantool/crud/blob/76e33749226d5fd1195e2628502a9e01d6a616fa/test/integration/updated_shema_test.lua#L622 +local function perform_insert_call_with_schema_reload(g) + -- create space w/ bucket_id index + helpers.call_on_servers(g.cluster, {'s1-master', 's2-master'}, function(server) + server.net_box:call('create_space') + server.net_box:call('create_bucket_id_index') + end) + + -- value should be string error + local obj, err = g.router:call( + 'crud.insert_object', { schema_change_space_name, { id = 11, value = 123 } } + ) + + t.assert_equals(obj, nil) + t.assert_is_not(err, nil) + t.assert_str_contains(err.err, "type does not match one required by operation: expected string") + + -- set value type to unsigned + helpers.call_on_servers(g.cluster, {'s1-master', 's2-master'}, function(server) + server.net_box:call('set_value_type_to_unsigned') + end) + + -- check that schema changes were applied + -- insert value unsigned - OK + local obj, err = g.router:call( + 'crud.insert_object', { schema_change_space_name, { id = 11, value = 123 } } + ) + + t.assert_is_not(obj, nil) + t.assert_equals(err, nil) +end + +pgroup.test_call_with_schema_reload_increments_counter = function(g) + local stats_before = get_stats(g) + + perform_insert_call_with_schema_reload(g) + + local stats_after = get_stats(g) + + t.assert_gt(stats_after.schema_reloads, stats_before.schema_reloads, + "Schema reloads counter incremented") +end + pgroup.test_resolve_name_from_id = function(g) local op = 'len' g.router:call('crud.len', { space_id }) @@ -660,6 +706,9 @@ local function generate_stats(g) local case = unknown_space_cases.insert local _, err = g.router:call(case.func, case.args) t.assert_not_equals(err, nil) + + -- Generate non-null schema reloads. + perform_insert_call_with_schema_reload(g) end -- https://github.com/tarantool/metrics/blob/fc5a67072340b12f983f09b7d383aca9e2f10cf1/test/utils.lua#L22-L31 @@ -747,7 +796,8 @@ local function validate_stats(g, metrics) 'Metrics are labelled with status') - local expected_names = { space_name } + local expected_names = { space_name, schema_change_space_name } + local expected_names_select = { space_name } if g.params.quantiles == true then t.assert_items_equals( @@ -778,7 +828,7 @@ local function validate_stats(g, metrics) t.assert_items_equals(get_unique_label_values(tuples_fetched, 'operation'), { 'select' }, 'Metrics are labelled with operation') - t.assert_items_equals(get_unique_label_values(tuples_fetched, 'name'), expected_names, + t.assert_items_equals(get_unique_label_values(tuples_fetched, 'name'), expected_names_select, 'Metrics are labelled with space name (only existing spaces)') @@ -788,7 +838,7 @@ local function validate_stats(g, metrics) t.assert_items_equals( get_unique_label_values(tuples_lookup, 'operation'), { 'select' }, 'Metrics are labelled with operation') - t.assert_items_equals(get_unique_label_values(tuples_lookup, 'name'), expected_names, + t.assert_items_equals(get_unique_label_values(tuples_lookup, 'name'), expected_names_select, 'Metrics are labelled with space name (only existing spaces)') @@ -798,12 +848,16 @@ local function validate_stats(g, metrics) t.assert_items_equals(get_unique_label_values(map_reduces, 'operation'), { 'select' }, 'Metrics are labelled with operation') - t.assert_items_equals(get_unique_label_values(map_reduces, 'name'), expected_names, + t.assert_items_equals(get_unique_label_values(map_reduces, 'name'), expected_names_select, 'Metrics are labelled with space name (only existing spaces)') local space_not_found = find_metric('tnt_crud_space_not_found', metrics) t.assert_type(space_not_found, 'table', '`tnt_crud_space_not_found` metrics found') + + + local schema_reloads = find_metric('tnt_crud_schema_reloads', metrics) + t.assert_type(schema_reloads, 'table', '`tnt_crud_schema_reloads` metrics found') end local function check_updated_per_call(g) diff --git a/test/unit/stats_test.lua b/test/unit/stats_test.lua index 278c2ae06..d574b211e 100644 --- a/test/unit/stats_test.lua +++ b/test/unit/stats_test.lua @@ -88,6 +88,7 @@ pgroup.test_get_format_after_enable = function(g) t.assert_type(stats, 'table') t.assert_equals(stats.spaces, {}) t.assert_equals(stats.space_not_found, 0) + t.assert_equals(stats.schema_reloads, 0) end pgroup.test_get_by_space_name_format_after_enable = function(g) @@ -421,6 +422,52 @@ for name, case in pairs(error_cases) do end end +local eval_with_fiber_context_setup = [[ + local fiber_context = require('crud.common.fiber_context') + + local op = select(1, ...) + local space_name = select(2, ...) + local context_field = select(3, ...) + local context_value = select(4, ...) + + local func = function(space_name) + local ctx = fiber_context.init_section('router_stats') + ctx[context_field] = context_value + return true + end + + + return stats_module.wrap(func, op)(space_name) +]] + +pgroup.test_map_reduce_increment = function(g) + local op = stats_module.op.SELECT + local inc = 1 + + local _, err = g.router:eval(eval_with_fiber_context_setup, + { op, space_name, 'map_reduces', inc }) + t.assert_equals(err, nil) + + local stats = get_stats(g) + + t.assert_equals(stats.spaces[space_name][op].details.map_reduces, inc, + "Counter of map reduces incremented") +end + +pgroup.test_schema_reload = function(g) + local op = stats_module.op.INSERT + local inc = 1 + + local _, err = g.router:eval(eval_with_fiber_context_setup, + { op, space_name, 'schema_reloads', inc }) + t.assert_equals(err, nil) + + local stats = get_stats(g) + + t.assert_equals(stats.schema_reloads, inc, + "Counter of map reduces incremented") +end + pgroup.test_stats_is_empty_after_disable = function(g) disable_stats(g)