Skip to content

Commit

Permalink
router: disable box.cfg
Browse files Browse the repository at this point in the history
There is no reason for router to have box configured. It needs neither
storage nor listening on a port. This is breaking change, so user's
code, which relies on vshard.router.cfg to call box.cfg must be changed.

Closes #353
Closes #428

@TarantoolBot document
Title: vshard: disable box.cfg on routers

Since vshard 0.1.25 vshard.router.cfg doesn't call box.cfg and this must
be done manually outside of vshard.
  • Loading branch information
Serpentian committed Nov 15, 2023
1 parent 6cff2d1 commit ae87d5f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 67 deletions.
2 changes: 1 addition & 1 deletion example/router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ cfg = dofile('localcfg.lua')
if arg[1] == 'discovery_disable' then
cfg.discovery_mode = 'off'
end
cfg.listen = 3300

box.cfg{listen = 3300}
-- Start the database with sharding
vshard = require('vshard')
vshard.router.cfg(cfg)
41 changes: 8 additions & 33 deletions test/router-luatest/router_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -462,33 +462,16 @@ g.test_enable_disable = function(g)
_G.ivshard.router.internal.errinj.ERRINJ_CFG_DELAY = true
end)
router:exec(function(cfg)
rawset(_G, 'fiber_static', ifiber.new(ivshard.router.cfg, cfg))
rawset(_G, 'fiber_new', ifiber.new(ivshard.router.new,
'new_router', cfg))
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)
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 routers = ivshard.router.internal.routers
rawset(_G, 'static_router', routers._static_router)
rawset(_G, 'new_router', routers.new_router)
end, {global_cfg})

local echo_func = function()
return router:exec(function(timeout)
Expand All @@ -502,15 +485,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
44 changes: 11 additions & 33 deletions vshard/router/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ if not M then
-- This counter is used to restart background fibers with
-- new reloaded code.
module_version = 0,
-- Flag whether box.info.status is acceptable.
is_loaded = false,

----------------------- Map-Reduce -----------------------
-- Storage Ref ID. It must be unique for each ref request
Expand Down Expand Up @@ -1258,31 +1256,23 @@ end
-- Configuration
--------------------------------------------------------------------------------

local function router_cfg(router, cfg, is_reload)
local function router_cfg(router, cfg)
cfg = lcfg.check(cfg, router.current_cfg)
local vshard_cfg, box_cfg = lcfg.split(cfg)
local vshard_cfg = lcfg.split(cfg)
if not M.replicasets then
log.info('Starting router configuration')
else
log.info('Starting router reconfiguration')
end
local new_replicasets = lreplicaset.buildall(vshard_cfg)
log.info("Calling box.cfg()...")
for k, v in pairs(box_cfg) do
log.info({[k] = v})
end
-- It is considered that all possible errors during cfg
-- process occur only before this place.
-- This check should be placed as late as possible.
if M.errinj.ERRINJ_CFG then
error('Error injection: cfg')
end
if not is_reload then
box.cfg(box_cfg)
log.info("Box has been configured")
while M.errinj.ERRINJ_CFG_DELAY do
lfiber.sleep(0.01)
end
while M.errinj.ERRINJ_CFG_DELAY do
lfiber.sleep(0.01)
end
-- Move connections from an old configuration to a new one.
-- It must be done with no yields to prevent usage both of not
Expand Down Expand Up @@ -1323,13 +1313,13 @@ local function router_cfg(router, cfg, is_reload)
router.is_configured = true
end

local function router_cfg_fiber_safe(router, cfg, is_reload)
local function router_cfg_fiber_safe(router, cfg)
if router.is_cfg_in_progress then
error(lerror.vshard(lerror.code.ROUTER_CFG_IS_IN_PROGRESS, router.name))
end

router.is_cfg_in_progress = true
local ok, err = pcall(router_cfg, router, cfg, is_reload)
local ok, err = pcall(router_cfg, router, cfg)
router.is_cfg_in_progress = false
if not ok then
error(err)
Expand Down Expand Up @@ -1683,20 +1673,6 @@ 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
if not router.is_configured then
local msg = 'router is not configured'
return error(lerror.vshard(lerror.code.ROUTER_IS_DISABLED, msg))
Expand Down Expand Up @@ -1734,7 +1710,9 @@ end

local router_mt = {
__index = {
cfg = function(router, cfg) return router_cfg_fiber_safe(router, cfg, false) end,
cfg = function(router, cfg)
return router_cfg_fiber_safe(router, cfg)
end,
info = router_make_api(router_info),
buckets_info = router_make_api(router_buckets_info),
call = router_make_api(router_call),
Expand Down Expand Up @@ -1832,7 +1810,7 @@ local function legacy_cfg(cfg)
end
else
-- Reconfigure
router_cfg_fiber_safe(router, cfg, false)
router_cfg_fiber_safe(router, cfg)
end
end

Expand Down Expand Up @@ -1861,7 +1839,7 @@ else
if router.api_call_cache == nil then
router.api_call_cache = router_api_call_unsafe
end
router_cfg_fiber_safe(router, router.current_cfg, true)
router_cfg_fiber_safe(router, router.current_cfg)
setmetatable(router, router_mt)
end
if M.static_router then
Expand Down

0 comments on commit ae87d5f

Please sign in to comment.