From 52e438f84f72bce3b0caad1743d8279f99764188 Mon Sep 17 00:00:00 2001 From: kamenkremen Date: Wed, 3 Dec 2025 12:45:02 +0300 Subject: [PATCH 1/3] storage: use varargs in API calls Currently, internal storage API functions use explicit arguments. This differs from the router API implementation, where varargs are used. This patch replaces explicit arguments with varargs in the storage_api_call_safe, storage_api_call_unsafe and storage_make_api functions. Motivation: - Consistency between router and storage internal APIs - Improved performance: using varargs is more efficient than passing explicit arguments Needed for #565 NO_DOC=refactoring NO_TEST=refactoring --- vshard/storage/init.lua | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index 97642250..ecb8e464 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -4016,19 +4016,15 @@ end -- Public API protection -------------------------------------------------------------------------------- --- --- Arguments are listed explicitly instead of '...' because the latter does not --- jit. --- -local function storage_api_call_safe(func, arg1, arg2, arg3, arg4) - return func(arg1, arg2, arg3, arg4) +local function storage_api_call_safe(func, ...) + return func(...) end -- -- Unsafe proxy is loaded with protections. But it is used rarely and only in -- the beginning of instance's lifetime. -- -local function storage_api_call_unsafe(func, arg1, arg2, arg3, arg4) +local function storage_api_call_unsafe(func, ...) -- box.info is quite expensive. Avoid calling it again when the instance -- is finally loaded. if not M.is_loaded then @@ -4054,12 +4050,12 @@ local function storage_api_call_unsafe(func, arg1, arg2, arg3, arg4) return error(lerror.vshard(lerror.code.STORAGE_IS_DISABLED, msg)) end M.api_call_cache = storage_api_call_safe - return func(arg1, arg2, arg3, arg4) + return func(...) end local function storage_make_api(func) - return function(arg1, arg2, arg3, arg4) - return M.api_call_cache(func, arg1, arg2, arg3, arg4) + return function(...) + return M.api_call_cache(func, ...) end end From ea81036bb3b0d8cce0f865ded521771dc15986e4 Mon Sep 17 00:00:00 2001 From: kamenkremen Date: Wed, 3 Dec 2025 12:58:46 +0300 Subject: [PATCH 2/3] storage: allow calling info when disabled A storage may be marked as disabled, which previously made it difficult to obtain status information from the component. This patch allows calling vshard.storage.info even when the component is disabled. The function now also returns a new boolean field is_enabled indicating whether the component is currently enabled. Closes #565 @TarantoolBot document Title: vshard: storage.info behavior change vshard.storage.info() This function can now be called even when the storage is disabled. It now also returns a new boolean field is_enabled indicating whether the component is currently enabled. --- test/storage-luatest/storage_1_test.lua | 41 +++++++++++++++++++++++++ test/storage/storage.result | 6 ++++ vshard/storage/init.lua | 14 +++++---- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/test/storage-luatest/storage_1_test.lua b/test/storage-luatest/storage_1_test.lua index ba2015aa..d84f25f7 100644 --- a/test/storage-luatest/storage_1_test.lua +++ b/test/storage-luatest/storage_1_test.lua @@ -633,3 +633,44 @@ test_group.test_alerts_for_named_replica = function(g) test_alerts_for_non_vshard_config_template(g, non_config_replica) end + +test_group.test_info_disable_consistency = function(g) + -- Make storage unconfigured. + g.replica_1_a:restart() + g.replica_1_a:exec(function(cfg) + ivtest.clear_test_cfg_options(cfg) + -- Imitate unconfigured box. + local old_box_cfg = box.cfg + box.cfg = function(...) return old_box_cfg(...) end + local _, err = pcall(ivshard.storage.info) + ilt.assert_not_equals(err, nil) + ilt.assert_str_contains(err.message, 'box seems not to be configured') + box.cfg = old_box_cfg + + local old_box_info = box.info + box.info = {status = 'loading'} + _, err = pcall(ivshard.storage.info) + ilt.assert_not_equals(err, nil) + ilt.assert_str_contains(err.message, 'instance status is "loading"') + box.info = old_box_info + + _, err = pcall(ivshard.storage.info) + ilt.assert_not_equals(err, nil) + ilt.assert_str_contains(err.message, 'storage is not configured') + + ivshard.storage.cfg(cfg, _G.get_uuid()) + local res = ivshard.storage.info() + ilt.assert_not_equals(res, nil) + ilt.assert(res.is_enabled) + + ivshard.storage.disable() + res = ivshard.storage.info() + ilt.assert_not_equals(res, nil) + ilt.assert_not(res.is_enabled) + + ivshard.storage.enable() + res = ivshard.storage.info() + ilt.assert_not_equals(res, nil) + ilt.assert(res.is_enabled) + end, {global_cfg}) +end diff --git a/test/storage/storage.result b/test/storage/storage.result index e4d8f038..c8dbbb21 100644 --- a/test/storage/storage.result +++ b/test/storage/storage.result @@ -153,6 +153,7 @@ vshard.storage.info() uri: storage@127.0.0.1:3301 identification_mode: uuid_as_key status: 2 + is_enabled: true replication: status: slave alerts: @@ -276,6 +277,7 @@ vshard.storage.info() uri: storage@127.0.0.1:3304 identification_mode: uuid_as_key status: 0 + is_enabled: true replication: status: follow lag: @@ -338,6 +340,7 @@ info uri: storage@127.0.0.1:3303 identification_mode: uuid_as_key status: 0 + is_enabled: true replication: status: master alerts: [] @@ -370,6 +373,7 @@ vshard.storage.info() uri: storage@127.0.0.1:3303 identification_mode: uuid_as_key status: 3 + is_enabled: true replication: status: master alerts: @@ -403,6 +407,7 @@ vshard.storage.info() uri: storage@127.0.0.1:3304 identification_mode: uuid_as_key status: 0 + is_enabled: true replication: status: follow lag: @@ -436,6 +441,7 @@ vshard.storage.info() uri: storage@127.0.0.1:3303 identification_mode: uuid_as_key status: 0 + is_enabled: true replication: status: master alerts: [] diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index ecb8e464..43193978 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -4009,6 +4009,7 @@ local function storage_info(opts) M.instance_watch_service:info(), } end + state.is_enabled = M.is_enabled return state end @@ -4016,7 +4017,7 @@ end -- Public API protection -------------------------------------------------------------------------------- -local function storage_api_call_safe(func, ...) +local function storage_api_call_safe(func, _opts, ...) return func(...) end @@ -4024,7 +4025,7 @@ end -- Unsafe proxy is loaded with protections. But it is used rarely and only in -- the beginning of instance's lifetime. -- -local function storage_api_call_unsafe(func, ...) +local function storage_api_call_unsafe(func, opts, ...) -- box.info is quite expensive. Avoid calling it again when the instance -- is finally loaded. if not M.is_loaded then @@ -4045,7 +4046,8 @@ local function storage_api_call_unsafe(func, ...) local msg = 'storage is not configured' return error(lerror.vshard(lerror.code.STORAGE_IS_DISABLED, msg)) end - if not M.is_enabled then + local is_disabled_skip = opts and opts.is_disabled_skip + if not M.is_enabled and not is_disabled_skip then local msg = 'storage is disabled explicitly' return error(lerror.vshard(lerror.code.STORAGE_IS_DISABLED, msg)) end @@ -4053,9 +4055,9 @@ local function storage_api_call_unsafe(func, ...) return func(...) end -local function storage_make_api(func) +local function storage_make_api(func, opts) return function(...) - return M.api_call_cache(func, ...) + return M.api_call_cache(func, opts, ...) end end @@ -4237,7 +4239,7 @@ return { -- Instance info. -- is_locked = storage_make_api(is_this_replicaset_locked), - info = storage_make_api(storage_info), + info = storage_make_api(storage_info, {is_disabled_skip = true}), sharded_spaces = storage_make_api(storage_sharded_spaces), _sharded_spaces = storage_sharded_spaces, module_version = function() return M.module_version end, From 4bda356ac10788071301b116b4ee7c60dff80286 Mon Sep 17 00:00:00 2001 From: kamenkremen Date: Fri, 21 Nov 2025 03:55:36 +0300 Subject: [PATCH 3/3] router: allow calling info when disabled A router may be marked as disabled, which previously made it difficult to obtain status information from the component. This patch allows calling vshard.router.info even when the component is disabled. The function now also returns a new boolean field is_enabled indicating whether the component is currently enabled. Follow-up #565 @TarantoolBot document Title: vshard: router.info behavior change vshard.router.info() This function can now be called even when the router is disabled. It now also returns a new boolean field is_enabled indicating whether the component is currently enabled. --- test/failover/cluster_changes.result | 3 ++ .../restart_during_rebalancing.result | 1 + test/router-luatest/router_2_2_test.lua | 34 +++++++++++++++++++ test/router/reconnect_to_master.result | 1 + test/router/router.result | 2 ++ vshard/router/init.lua | 14 ++++---- 6 files changed, 49 insertions(+), 6 deletions(-) diff --git a/test/failover/cluster_changes.result b/test/failover/cluster_changes.result index 26a8025b..498da39a 100644 --- a/test/failover/cluster_changes.result +++ b/test/failover/cluster_changes.result @@ -127,6 +127,7 @@ info available_rw: 0 identification_mode: uuid_as_key status: 1 + is_enabled: true alerts: - ['UNKNOWN_BUCKETS', '3000 buckets are not discovered'] ... @@ -278,6 +279,7 @@ info available_rw: 0 identification_mode: uuid_as_key status: 3 + is_enabled: true alerts: - ['MISSING_MASTER', 'Master is not configured for replicaset 739fe4fb-2850-4cde-9637-10150724c5eb'] - ['MISSING_MASTER', 'Master is not configured for replicaset 832bbba0-9699-4aa1-907d-c7c7af61f5c9'] @@ -384,6 +386,7 @@ info available_rw: 0 identification_mode: uuid_as_key status: 1 + is_enabled: true alerts: - ['UNKNOWN_BUCKETS', '3000 buckets are not discovered'] ... diff --git a/test/rebalancer/restart_during_rebalancing.result b/test/rebalancer/restart_during_rebalancing.result index e325b607..33911194 100644 --- a/test/rebalancer/restart_during_rebalancing.result +++ b/test/rebalancer/restart_during_rebalancing.result @@ -285,6 +285,7 @@ info available_rw: 200 identification_mode: uuid_as_key status: 0 + is_enabled: true alerts: [] ... util.stop_loading() diff --git a/test/router-luatest/router_2_2_test.lua b/test/router-luatest/router_2_2_test.lua index 71316449..be611b53 100644 --- a/test/router-luatest/router_2_2_test.lua +++ b/test/router-luatest/router_2_2_test.lua @@ -1265,3 +1265,37 @@ g.test_log_ratelimiter_is_dropped_on_disable_of_the_service = function(g) _G.ratelimit = nil end, {name}) end + +g.test_info_disable_consistency = function(g) + local router = vtest.router_new(g, 'router_1') + router:exec(function(cfg) + ivtest.clear_test_cfg_options(cfg) + -- Do not allow router's configuration to complete. + _G.ivshard.router.internal.errinj.ERRINJ_CFG_DELAY = true + local fiber_cfg = ifiber.create(ivshard.router.cfg, cfg) + fiber_cfg:set_joinable(true) + local routers = ivshard.router.internal.routers + local _, err = pcall(routers._static_router.info, + routers._static_router) + ilt.assert_not_equals(err, nil) + ilt.assert_str_contains(err.message, 'router is not configured') + + -- Unblock router's configuration and wait until it's finished. + _G.ivshard.router.internal.errinj.ERRINJ_CFG_DELAY = false + fiber_cfg:join() + local res = ivshard.router:info() + ilt.assert_not_equals(res, nil) + ilt.assert(res.is_enabled) + + ivshard.router:disable() + res = ivshard.router:info() + ilt.assert_not_equals(res, nil) + ilt.assert_not(res.is_enabled) + + ivshard.router:enable() + res = ivshard.router:info() + t.assert_not_equals(res, nil) + t.assert(res.is_enabled) + end, {global_cfg}) + vtest.drop_instance(g, router) +end diff --git a/test/router/reconnect_to_master.result b/test/router/reconnect_to_master.result index ce321261..c5e2c8e2 100644 --- a/test/router/reconnect_to_master.result +++ b/test/router/reconnect_to_master.result @@ -149,6 +149,7 @@ vshard.router.info() available_rw: 0 identification_mode: uuid_as_key status: 2 + is_enabled: true alerts: - ['UNREACHABLE_MASTER', 'Master of replicaset cbf06940-0790-498b-948d-042b62cf3d29 is unreachable: disconnected'] diff --git a/test/router/router.result b/test/router/router.result index f380efce..a01ed757 100644 --- a/test/router/router.result +++ b/test/router/router.result @@ -220,6 +220,7 @@ vshard.router.info() available_rw: 1 identification_mode: uuid_as_key status: 1 + is_enabled: true alerts: - ['UNKNOWN_BUCKETS', '2999 buckets are not discovered'] ... @@ -1076,6 +1077,7 @@ info available_rw: 3000 identification_mode: uuid_as_key status: 0 + is_enabled: true alerts: [] ... -- Remove replica and master connections to trigger alert diff --git a/vshard/router/init.lua b/vshard/router/init.lua index 50e6b85a..ca152475 100644 --- a/vshard/router/init.lua +++ b/vshard/router/init.lua @@ -1546,6 +1546,7 @@ local function router_info(router, opts) router.discovery_service:info(), } end + state.is_enabled = router.is_enabled return state end @@ -1661,7 +1662,7 @@ end -- Public API protection -------------------------------------------------------------------------------- -local function router_api_call_safe(func, router, ...) +local function router_api_call_safe(func, router, _opts, ...) return func(router, ...) end @@ -1669,13 +1670,14 @@ end -- Unsafe proxy is loaded with protections. But it is used rarely and only in -- the beginning of instance's lifetime. -- -local function router_api_call_unsafe(func, router, ...) +local function router_api_call_unsafe(func, router, opts, ...) -- Router can be started on instance with unconfigured box.cfg. if not router.is_configured then local msg = 'router is not configured' return error(lerror.vshard(lerror.code.ROUTER_IS_DISABLED, msg)) end - if not router.is_enabled then + local is_disabled_skip = opts and opts.is_disabled_skip + if not router.is_enabled and not is_disabled_skip then local msg = 'router is disabled explicitly' return error(lerror.vshard(lerror.code.ROUTER_IS_DISABLED, msg)) end @@ -1683,9 +1685,9 @@ local function router_api_call_unsafe(func, router, ...) return func(router, ...) end -local function router_make_api(func) +local function router_make_api(func, opts) return function(router, ...) - return router.api_call_cache(func, router, ...) + return router.api_call_cache(func, router, opts, ...) end end @@ -1709,7 +1711,7 @@ end local router_mt = { __index = { cfg = function(router, cfg) return router_cfg_fiber_safe(router, cfg, false) end, - info = router_make_api(router_info), + info = router_make_api(router_info, {is_disabled_skip = true}), buckets_info = router_make_api(router_buckets_info), call = router_make_api(router_call), callro = router_make_api(router_callro),