Skip to content

Commit

Permalink
router: relax checks in auto disable
Browse files Browse the repository at this point in the history
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=<documentation is correct>
  • Loading branch information
Serpentian committed Dec 14, 2023
1 parent c035ffb commit 6bbe76b
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 52 deletions.
49 changes: 11 additions & 38 deletions test/router-luatest/router_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 1 addition & 14 deletions vshard/router/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down

0 comments on commit 6bbe76b

Please sign in to comment.