From 6bbe76bdf47d26501872a8ce77ab74fcf4a094a0 Mon Sep 17 00:00:00 2001 From: Nikita Zheleztsov Date: Thu, 14 Dec 2023 17:49:11 +0300 Subject: [PATCH] router: relax checks in auto disable Router's API is protected until the configuration is done, as accessing router's functions at that time is not safe and can cause low level errors. Let's relax checks in this protection. The thrown error messages are incorrect from now on: router can be started on unconfigured instance, when box_cfg_mode is 'manual'. box.info.status in such case will be 'unconfigured', not 'running', we shouldn't rely on it either. NO_DOC= --- test/router-luatest/router_test.lua | 49 +++++++---------------------- vshard/router/init.lua | 15 +-------- 2 files changed, 12 insertions(+), 52 deletions(-) diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua index 0a01dba2..a5258ad1 100644 --- a/test/router-luatest/router_test.lua +++ b/test/router-luatest/router_test.lua @@ -458,40 +458,20 @@ g.test_enable_disable = function(g) -- gh-291: router enable/disable -- local router = vtest.router_new(g, 'router_1') - -- do not allow router's configuration to complete - router:exec(function() - _G.ivshard.router.internal.errinj.ERRINJ_CFG_DELAY = true - end) router:exec(function(cfg) ivtest.clear_test_cfg_options(cfg) - rawset(_G, 'fiber_static', ifiber.new(ivshard.router.cfg, cfg)) - rawset(_G, 'fiber_new', ifiber.new(ivshard.router.new, - 'new_router', cfg)) + -- Do not allow router's configuration to complete. + _G.ivshard.router.internal.errinj.ERRINJ_CFG_DELAY = true + rawset(_G, 'fiber_static', ifiber.create(ivshard.router.cfg, cfg)) + rawset(_G, 'fiber_new', ifiber.create(ivshard.router.new, + 'new_router', cfg)) _G.fiber_static:set_joinable(true) _G.fiber_new:set_joinable(true) + local routers = ivshard.router.internal.routers + rawset(_G, 'static_router', routers._static_router) + rawset(_G, 'new_router', routers.new_router) end, {global_cfg}) - local err1, err2 = router:exec(function() - -- emulate unconfigured box - local old_box_cfg = box.cfg - box.cfg = function(...) return old_box_cfg(...) end - - rawset(_G, 'static_router', ivshard.router.internal.routers._static_router) - rawset(_G, 'new_router', ivshard.router.internal.routers.new_router) - local _, err_1 = pcall(_G.static_router.info, _G.static_router) - local _, err_2 = pcall(_G.new_router.info, _G.new_router) - - box.cfg = old_box_cfg - return err_1, err_2 - end) - assert_errors_equals(err1, err2, 'box seems not to be configured') - - -- set box status to loading - router:exec(function() - rawset(_G, 'old_box_info', box.info) - box.info = {status = 'loading'} - end) - local echo_func = function() return router:exec(function(timeout) local echo = function(router) @@ -504,15 +484,7 @@ g.test_enable_disable = function(g) end, {vtest.wait_timeout}) end - err1, err2 = echo_func() - assert_errors_equals(err1, err2, 'instance status is "loading"') - - -- restore proper box configuration - router:exec(function() - box.info = _G.old_box_info - end) - - err1, err2 = echo_func() + local err1, err2 = echo_func() assert_errors_equals(err1, err2, 'router is not configured') -- unblock router's configuration and wait until it's finished @@ -661,8 +633,9 @@ g.test_router_box_cfg_mode = function(g) local box_cfg = box.cfg box.cfg = function() end ivshard.router.cfg(cfg) + local router = ivshard.router.new('router', cfg) local opts = {timeout = iwait_timeout} - local res, err = ivshard.router.callrw(1, 'echo', {1}, opts) + local res, err = router:callrw(1, 'echo', {1}, opts) ilt.assert_equals(err, nil) ilt.assert_equals(res, 1) box.cfg = box_cfg diff --git a/vshard/router/init.lua b/vshard/router/init.lua index d6804c43..4c93f5c7 100644 --- a/vshard/router/init.lua +++ b/vshard/router/init.lua @@ -1698,20 +1698,7 @@ end -- the beginning of instance's lifetime. -- local function router_api_call_unsafe(func, router, ...) - -- box.info is quite expensive. Avoid calling it again when the instance - -- is finally loaded. - if not M.is_loaded then - if type(box.cfg) == 'function' then - local msg = 'box seems not to be configured' - return error(lerror.vshard(lerror.code.ROUTER_IS_DISABLED, msg)) - end - local status = box.info.status - if status ~= 'running' then - local msg = ('instance status is "%s"'):format(status) - return error(lerror.vshard(lerror.code.ROUTER_IS_DISABLED, msg)) - end - M.is_loaded = true - end + -- 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))