* [Tarantool-patches] [PATCH vshard 0/2] Deprecations @ 2022-01-28 0:23 Vladislav Shpilevoy via Tarantool-patches 2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support Vladislav Shpilevoy via Tarantool-patches 2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call Vladislav Shpilevoy via Tarantool-patches 0 siblings, 2 replies; 6+ messages in thread From: Vladislav Shpilevoy via Tarantool-patches @ 2022-01-28 0:23 UTC (permalink / raw) To: tarantool-patches, olegrok Branch: http://github.com/tarantool/vshard/tree/gerold103/gh-267-drop-1.9 Issue: https://github.com/tarantool/vshard/issues/267 Vladislav Shpilevoy (2): Drop Tarantool < 1.10.1 support Drop periodic Lua GC explicit call test/multiple_routers/multiple_routers.result | 64 ---------- .../multiple_routers.test.lua | 20 --- test/router/garbage_collector.result | 116 ------------------ test/router/garbage_collector.test.lua | 39 ------ test/storage/garbage_collector.result | 62 ---------- test/storage/garbage_collector.test.lua | 23 ---- test/unit/config.result | 27 +--- test/unit/config.test.lua | 13 +- vshard/cfg.lua | 9 +- vshard/consts.lua | 1 - vshard/error.lua | 9 +- vshard/lua_gc.lua | 54 -------- vshard/router/init.lua | 101 +-------------- vshard/storage/init.lua | 29 +---- vshard/util.lua | 4 + 15 files changed, 23 insertions(+), 548 deletions(-) delete mode 100644 test/router/garbage_collector.result delete mode 100644 test/router/garbage_collector.test.lua delete mode 100644 vshard/lua_gc.lua -- 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support 2022-01-28 0:23 [Tarantool-patches] [PATCH vshard 0/2] Deprecations Vladislav Shpilevoy via Tarantool-patches @ 2022-01-28 0:23 ` Vladislav Shpilevoy via Tarantool-patches 2022-01-28 5:47 ` Oleg Babin via Tarantool-patches 2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call Vladislav Shpilevoy via Tarantool-patches 1 sibling, 1 reply; 6+ messages in thread From: Vladislav Shpilevoy via Tarantool-patches @ 2022-01-28 0:23 UTC (permalink / raw) To: tarantool-patches, olegrok The reasons are: - Rebalancer won't work. It uses fiber.join(). - Bucket discovery on router is extra expensive - it downloads the entire _bucket from each storage. That can make the latter and the router unresponsive for notable time if bucket count is big enough. - There are other minor hacks which complicate development. For instance, box.error.new() couldn't be used. - Tests don't pass. Thus can't tell what else is broken. Makes no sense to support it if it can't be even tested properly The reason why it is done now is because the code will get more complicated soon and will use more new features which even 1.10 won't support. This patch should reduce the level of version dependency complications before they get even worse. Closes #256 Closes #267 @TarantoolBot document Title: VShard now supports only Tarantool >= 1.10.1 It used to support 1.9 too, but now it is dropped. To use VShard > 0.1.19 a user needs Tarantool >= 1.10.1. --- vshard/cfg.lua | 5 ---- vshard/error.lua | 9 ++---- vshard/router/init.lua | 65 ++--------------------------------------- vshard/storage/init.lua | 21 ++----------- vshard/util.lua | 4 +++ 5 files changed, 13 insertions(+), 91 deletions(-) diff --git a/vshard/cfg.lua b/vshard/cfg.lua index a059d44..1903967 100644 --- a/vshard/cfg.lua +++ b/vshard/cfg.lua @@ -2,7 +2,6 @@ local log = require('log') local luri = require('uri') -local lutil = require('vshard.util') local consts = require('vshard.consts') local function check_uri(uri) @@ -22,10 +21,6 @@ local function check_replica_master(master, ctx) end local function check_replicaset_master(master, ctx) - -- Limit the version due to extensive usage of netbox is_async feature. - if not lutil.version_is_at_least(1, 10, 1) then - error('Only versions >= 1.10.1 support master discovery') - end if master ~= 'auto' then error('Only "auto" master is supported') end diff --git a/vshard/error.lua b/vshard/error.lua index c673827..d36803a 100644 --- a/vshard/error.lua +++ b/vshard/error.lua @@ -235,8 +235,7 @@ local function make_error(e) -- box.error, return unpacked return box_error(e) elseif type(e) == 'string' then - local ok, err = pcall(box.error, box.error.PROC_LUA, e) - return box_error(err) + return box_error(box.error.new(box.error.PROC_LUA, e)) elseif type(e) == 'table' then return setmetatable(e, {__tostring = json.encode}) else @@ -271,12 +270,10 @@ local function make_alert(code, ...) end -- --- Create a timeout error object. Box.error.new() can't be used because is --- present only since 1.10. +-- Create a timeout error object. -- local function make_timeout() - local _, err = pcall(box.error, box.error.TIMEOUT) - return make_error(err) + return make_error(box.error.new(box.error.TIMEOUT)) end return { diff --git a/vshard/router/init.lua b/vshard/router/init.lua index 5a94a2f..ebeaac9 100644 --- a/vshard/router/init.lua +++ b/vshard/router/init.lua @@ -249,15 +249,10 @@ local function discovery_handle_buckets(router, replicaset, buckets) end end -local discovery_f - -if util.version_is_at_least(1, 10, 0) then -- --- >= 1.10 version of discovery fiber does parallel discovery of --- all replicasets at once. It uses is_async feature of netbox --- for that. +-- Bucket discovery main loop. -- -discovery_f = function(router) +local function discovery_f(router) local module_version = M.module_version assert(router.discovery_mode == 'on' or router.discovery_mode == 'once') local iterators = {} @@ -394,47 +389,6 @@ discovery_f = function(router) end end --- Version >= 1.10. -else --- Version < 1.10. - --- --- Background fiber to perform discovery. It periodically scans --- replicasets one by one and updates route_map. --- -discovery_f = function(router) - local module_version = M.module_version - while module_version == M.module_version do - while not next(router.replicasets) do - lfiber.sleep(consts.DISCOVERY_IDLE_INTERVAL) - end - local old_replicasets = router.replicasets - for rs_uuid, replicaset in pairs(router.replicasets) do - local active_buckets, err = - replicaset:callro('vshard.storage.buckets_discovery', {}, - {timeout = 2}) - while M.errinj.ERRINJ_LONG_DISCOVERY do - M.errinj.ERRINJ_LONG_DISCOVERY = 'waiting' - lfiber.sleep(0.01) - end - -- Renew replicasets object captured by the for loop - -- in case of reconfigure and reload events. - if router.replicasets ~= old_replicasets then - break - end - if not active_buckets then - log.error('Error during discovery %s: %s', replicaset, err) - else - discovery_handle_buckets(router, replicaset, active_buckets) - end - lfiber.sleep(consts.DISCOVERY_IDLE_INTERVAL) - end - end -end - --- Version < 1.10. -end - -- -- Immediately wakeup discovery fiber if exists. -- @@ -758,9 +712,6 @@ local function router_call(router, bucket_id, opts, ...) ...) end -local router_map_callrw - -if util.version_is_at_least(1, 10, 0) then -- -- Consistent Map-Reduce. The given function is called on all masters in the -- cluster with a guarantee that in case of success it was executed with all @@ -800,7 +751,7 @@ if util.version_is_at_least(1, 10, 0) then -- about concrete replicaset. For example, not all buckets were found even -- though all replicasets were scanned. -- -router_map_callrw = function(router, func, args, opts) +local function router_map_callrw(router, func, args, opts) local replicasets = router.replicasets local timeout = opts and opts.timeout or consts.CALL_TIMEOUT_MIN local deadline = fiber_clock() + timeout @@ -919,16 +870,6 @@ router_map_callrw = function(router, func, args, opts) return nil, err, err_uuid end --- Version >= 1.10. -else --- Version < 1.10. - -router_map_callrw = function() - error('Supported for Tarantool >= 1.10') -end - -end - -- -- Get replicaset object by bucket identifier. -- @param bucket_id Bucket identifier. diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index 3eaad9c..78103cf 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -1288,11 +1288,8 @@ local function bucket_recv_xc(bucket_id, from, data, opts) local space_name, space_data = row[1], row[2] local space = box.space[space_name] if space == nil then - -- Tarantool doesn't provide API to create box.error - -- objects before 1.10. - local _, boxerror = pcall(box.error, box.error.NO_SUCH_SPACE, - space_name) - return nil, lerror.box(boxerror) + local err = box.error.new(box.error.NO_SUCH_SPACE, space_name) + return nil, lerror.box(err) end box.begin() for _, tuple in ipairs(space_data) do @@ -2209,13 +2206,6 @@ end -- local function rebalancer_worker_f(worker_id, dispenser, quit_cond) lfiber.name(string.format('vshard.rebalancer_worker_%d', worker_id)) - if not util.version_is_at_least(1, 10, 0) then - -- Return control to the caller immediately to allow it - -- to finish preparations. In 1.9 a caller couldn't create - -- a fiber without switching to it. - lfiber.yield() - end - local _status = box.space._bucket.index.status local opts = {timeout = consts.REBALANCER_CHUNK_TIMEOUT} local active_key = {consts.BUCKET.ACTIVE} @@ -2299,12 +2289,7 @@ local function rebalancer_apply_routes_f(routes) local quit_cond = lfiber.cond() local workers = table.new(worker_count, 0) for i = 1, worker_count do - local f - if util.version_is_at_least(1, 10, 0) then - f = lfiber.new(rebalancer_worker_f, i, dispenser, quit_cond) - else - f = lfiber.create(rebalancer_worker_f, i, dispenser, quit_cond) - end + local f = lfiber.new(rebalancer_worker_f, i, dispenser, quit_cond) f:set_joinable(true) workers[i] = f end diff --git a/vshard/util.lua b/vshard/util.lua index afa658b..366fdea 100644 --- a/vshard/util.lua +++ b/vshard/util.lua @@ -154,6 +154,10 @@ local function version_is_at_least(major_need, middle_need, minor_need) return minor >= minor_need end +if not version_is_at_least(1, 10, 1) then + error("VShard supports only Tarantool >= 1.10.1") +end + -- -- Copy @a src table. Fiber yields every @a interval keys copied. Does not give -- any guarantees on what is the result when the source table is changed during -- 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support 2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support Vladislav Shpilevoy via Tarantool-patches @ 2022-01-28 5:47 ` Oleg Babin via Tarantool-patches 0 siblings, 0 replies; 6+ messages in thread From: Oleg Babin via Tarantool-patches @ 2022-01-28 5:47 UTC (permalink / raw) To: Vladislav Shpilevoy, tarantool-patches Hi! Thanks for your patch. LGTM. On 28.01.2022 03:23, Vladislav Shpilevoy wrote: > The reasons are: > > - Rebalancer won't work. It uses fiber.join(). > > - Bucket discovery on router is extra expensive - it downloads the > entire _bucket from each storage. That can make the latter and > the router unresponsive for notable time if bucket count is big > enough. > > - There are other minor hacks which complicate development. For > instance, box.error.new() couldn't be used. > > - Tests don't pass. Thus can't tell what else is broken. Makes no > sense to support it if it can't be even tested properly > > The reason why it is done now is because the code will get more > complicated soon and will use more new features which even 1.10 > won't support. This patch should reduce the level of version > dependency complications before they get even worse. > > Closes #256 > Closes #267 > > @TarantoolBot document > Title: VShard now supports only Tarantool >= 1.10.1 > > It used to support 1.9 too, but now it is dropped. To use VShard >> 0.1.19 a user needs Tarantool >= 1.10.1. > --- > vshard/cfg.lua | 5 ---- > vshard/error.lua | 9 ++---- > vshard/router/init.lua | 65 ++--------------------------------------- > vshard/storage/init.lua | 21 ++----------- > vshard/util.lua | 4 +++ > 5 files changed, 13 insertions(+), 91 deletions(-) > > diff --git a/vshard/cfg.lua b/vshard/cfg.lua > index a059d44..1903967 100644 > --- a/vshard/cfg.lua > +++ b/vshard/cfg.lua > @@ -2,7 +2,6 @@ > > local log = require('log') > local luri = require('uri') > -local lutil = require('vshard.util') > local consts = require('vshard.consts') > > local function check_uri(uri) > @@ -22,10 +21,6 @@ local function check_replica_master(master, ctx) > end > > local function check_replicaset_master(master, ctx) > - -- Limit the version due to extensive usage of netbox is_async feature. > - if not lutil.version_is_at_least(1, 10, 1) then > - error('Only versions >= 1.10.1 support master discovery') > - end > if master ~= 'auto' then > error('Only "auto" master is supported') > end > diff --git a/vshard/error.lua b/vshard/error.lua > index c673827..d36803a 100644 > --- a/vshard/error.lua > +++ b/vshard/error.lua > @@ -235,8 +235,7 @@ local function make_error(e) > -- box.error, return unpacked > return box_error(e) > elseif type(e) == 'string' then > - local ok, err = pcall(box.error, box.error.PROC_LUA, e) > - return box_error(err) > + return box_error(box.error.new(box.error.PROC_LUA, e)) > elseif type(e) == 'table' then > return setmetatable(e, {__tostring = json.encode}) > else > @@ -271,12 +270,10 @@ local function make_alert(code, ...) > end > > -- > --- Create a timeout error object. Box.error.new() can't be used because is > --- present only since 1.10. > +-- Create a timeout error object. > -- > local function make_timeout() > - local _, err = pcall(box.error, box.error.TIMEOUT) > - return make_error(err) > + return make_error(box.error.new(box.error.TIMEOUT)) > end > > return { > diff --git a/vshard/router/init.lua b/vshard/router/init.lua > index 5a94a2f..ebeaac9 100644 > --- a/vshard/router/init.lua > +++ b/vshard/router/init.lua > @@ -249,15 +249,10 @@ local function discovery_handle_buckets(router, replicaset, buckets) > end > end > > -local discovery_f > - > -if util.version_is_at_least(1, 10, 0) then > -- > --- >= 1.10 version of discovery fiber does parallel discovery of > --- all replicasets at once. It uses is_async feature of netbox > --- for that. > +-- Bucket discovery main loop. > -- > -discovery_f = function(router) > +local function discovery_f(router) > local module_version = M.module_version > assert(router.discovery_mode == 'on' or router.discovery_mode == 'once') > local iterators = {} > @@ -394,47 +389,6 @@ discovery_f = function(router) > end > end > > --- Version >= 1.10. > -else > --- Version < 1.10. > - > --- > --- Background fiber to perform discovery. It periodically scans > --- replicasets one by one and updates route_map. > --- > -discovery_f = function(router) > - local module_version = M.module_version > - while module_version == M.module_version do > - while not next(router.replicasets) do > - lfiber.sleep(consts.DISCOVERY_IDLE_INTERVAL) > - end > - local old_replicasets = router.replicasets > - for rs_uuid, replicaset in pairs(router.replicasets) do > - local active_buckets, err = > - replicaset:callro('vshard.storage.buckets_discovery', {}, > - {timeout = 2}) > - while M.errinj.ERRINJ_LONG_DISCOVERY do > - M.errinj.ERRINJ_LONG_DISCOVERY = 'waiting' > - lfiber.sleep(0.01) > - end > - -- Renew replicasets object captured by the for loop > - -- in case of reconfigure and reload events. > - if router.replicasets ~= old_replicasets then > - break > - end > - if not active_buckets then > - log.error('Error during discovery %s: %s', replicaset, err) > - else > - discovery_handle_buckets(router, replicaset, active_buckets) > - end > - lfiber.sleep(consts.DISCOVERY_IDLE_INTERVAL) > - end > - end > -end > - > --- Version < 1.10. > -end > - > -- > -- Immediately wakeup discovery fiber if exists. > -- > @@ -758,9 +712,6 @@ local function router_call(router, bucket_id, opts, ...) > ...) > end > > -local router_map_callrw > - > -if util.version_is_at_least(1, 10, 0) then > -- > -- Consistent Map-Reduce. The given function is called on all masters in the > -- cluster with a guarantee that in case of success it was executed with all > @@ -800,7 +751,7 @@ if util.version_is_at_least(1, 10, 0) then > -- about concrete replicaset. For example, not all buckets were found even > -- though all replicasets were scanned. > -- > -router_map_callrw = function(router, func, args, opts) > +local function router_map_callrw(router, func, args, opts) > local replicasets = router.replicasets > local timeout = opts and opts.timeout or consts.CALL_TIMEOUT_MIN > local deadline = fiber_clock() + timeout > @@ -919,16 +870,6 @@ router_map_callrw = function(router, func, args, opts) > return nil, err, err_uuid > end > > --- Version >= 1.10. > -else > --- Version < 1.10. > - > -router_map_callrw = function() > - error('Supported for Tarantool >= 1.10') > -end > - > -end > - > -- > -- Get replicaset object by bucket identifier. > -- @param bucket_id Bucket identifier. > diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua > index 3eaad9c..78103cf 100644 > --- a/vshard/storage/init.lua > +++ b/vshard/storage/init.lua > @@ -1288,11 +1288,8 @@ local function bucket_recv_xc(bucket_id, from, data, opts) > local space_name, space_data = row[1], row[2] > local space = box.space[space_name] > if space == nil then > - -- Tarantool doesn't provide API to create box.error > - -- objects before 1.10. > - local _, boxerror = pcall(box.error, box.error.NO_SUCH_SPACE, > - space_name) > - return nil, lerror.box(boxerror) > + local err = box.error.new(box.error.NO_SUCH_SPACE, space_name) > + return nil, lerror.box(err) > end > box.begin() > for _, tuple in ipairs(space_data) do > @@ -2209,13 +2206,6 @@ end > -- > local function rebalancer_worker_f(worker_id, dispenser, quit_cond) > lfiber.name(string.format('vshard.rebalancer_worker_%d', worker_id)) > - if not util.version_is_at_least(1, 10, 0) then > - -- Return control to the caller immediately to allow it > - -- to finish preparations. In 1.9 a caller couldn't create > - -- a fiber without switching to it. > - lfiber.yield() > - end > - > local _status = box.space._bucket.index.status > local opts = {timeout = consts.REBALANCER_CHUNK_TIMEOUT} > local active_key = {consts.BUCKET.ACTIVE} > @@ -2299,12 +2289,7 @@ local function rebalancer_apply_routes_f(routes) > local quit_cond = lfiber.cond() > local workers = table.new(worker_count, 0) > for i = 1, worker_count do > - local f > - if util.version_is_at_least(1, 10, 0) then > - f = lfiber.new(rebalancer_worker_f, i, dispenser, quit_cond) > - else > - f = lfiber.create(rebalancer_worker_f, i, dispenser, quit_cond) > - end > + local f = lfiber.new(rebalancer_worker_f, i, dispenser, quit_cond) > f:set_joinable(true) > workers[i] = f > end > diff --git a/vshard/util.lua b/vshard/util.lua > index afa658b..366fdea 100644 > --- a/vshard/util.lua > +++ b/vshard/util.lua > @@ -154,6 +154,10 @@ local function version_is_at_least(major_need, middle_need, minor_need) > return minor >= minor_need > end > > +if not version_is_at_least(1, 10, 1) then > + error("VShard supports only Tarantool >= 1.10.1") > +end > + > -- > -- Copy @a src table. Fiber yields every @a interval keys copied. Does not give > -- any guarantees on what is the result when the source table is changed during ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call 2022-01-28 0:23 [Tarantool-patches] [PATCH vshard 0/2] Deprecations Vladislav Shpilevoy via Tarantool-patches 2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support Vladislav Shpilevoy via Tarantool-patches @ 2022-01-28 0:23 ` Vladislav Shpilevoy via Tarantool-patches 2022-01-28 5:47 ` Oleg Babin via Tarantool-patches 1 sibling, 1 reply; 6+ messages in thread From: Vladislav Shpilevoy via Tarantool-patches @ 2022-01-28 0:23 UTC (permalink / raw) To: tarantool-patches, olegrok It was introduced because somebody very long time ago complained that otherwise router goes OOM and the person was sure it was due to Lua GC working too rare/late. This of course makes little sense. Either Lua GC was never the issue, or if it is - then it should be fixed in Lua implementation in the core. In case anybody still needs that due to any reason, he can start his own fiber making periodic collectgarbage(). This has nothing to do with vshard and never had, really. The patch is done now in scope of general code cleanup from old unnecessary stuff, like Tarantool < 1.10.0 support. @TarantoolBot document Title: VShard cfg option collect_lua_garbage is deprecated It won't do anything now, only print a warning that it is deprecated. --- test/multiple_routers/multiple_routers.result | 64 ---------- .../multiple_routers.test.lua | 20 --- test/router/garbage_collector.result | 116 ------------------ test/router/garbage_collector.test.lua | 39 ------ test/storage/garbage_collector.result | 62 ---------- test/storage/garbage_collector.test.lua | 23 ---- test/unit/config.result | 27 +--- test/unit/config.test.lua | 13 +- vshard/cfg.lua | 4 +- vshard/consts.lua | 1 - vshard/lua_gc.lua | 54 -------- vshard/router/init.lua | 36 +----- vshard/storage/init.lua | 8 +- 13 files changed, 10 insertions(+), 457 deletions(-) delete mode 100644 test/router/garbage_collector.result delete mode 100644 test/router/garbage_collector.test.lua delete mode 100644 vshard/lua_gc.lua diff --git a/test/multiple_routers/multiple_routers.result b/test/multiple_routers/multiple_routers.result index b18c194..91b5d29 100644 --- a/test/multiple_routers/multiple_routers.result +++ b/test/multiple_routers/multiple_routers.result @@ -212,70 +212,6 @@ routers[5]:call(1, 'read', 'do_select', {2}) --- - [[2, 2]] ... --- Check lua_gc counter. -lua_gc = require('vshard.lua_gc') ---- -... -vshard.router.internal.collect_lua_garbage_cnt == 0 ---- -- true -... -lua_gc.internal.bg_fiber == nil ---- -- true -... -configs.cfg_2.collect_lua_garbage = true ---- -... -routers[5]:cfg(configs.cfg_2) ---- -... -lua_gc.internal.bg_fiber ~= nil ---- -- true -... -routers[7]:cfg(configs.cfg_2) ---- -... -lua_gc.internal.bg_fiber ~= nil ---- -- true -... -vshard.router.internal.collect_lua_garbage_cnt == 2 ---- -- true -... -package.loaded['vshard.router'] = nil ---- -... -vshard.router = require('vshard.router') ---- -... -vshard.router.internal.collect_lua_garbage_cnt == 2 ---- -- true -... -configs.cfg_2.collect_lua_garbage = nil ---- -... -routers[5]:cfg(configs.cfg_2) ---- -... -lua_gc.internal.bg_fiber ~= nil ---- -- true -... -routers[7]:cfg(configs.cfg_2) ---- -... -vshard.router.internal.collect_lua_garbage_cnt == 0 ---- -- true -... -lua_gc.internal.bg_fiber == nil ---- -- true -... -- Self checker. util.check_error(router_2.info) --- diff --git a/test/multiple_routers/multiple_routers.test.lua b/test/multiple_routers/multiple_routers.test.lua index b5da5cf..c61f71d 100644 --- a/test/multiple_routers/multiple_routers.test.lua +++ b/test/multiple_routers/multiple_routers.test.lua @@ -80,26 +80,6 @@ vshard.router.call(1, 'read', 'do_select', {1}) router_2:call(1, 'read', 'do_select', {2}) routers[5]:call(1, 'read', 'do_select', {2}) --- Check lua_gc counter. -lua_gc = require('vshard.lua_gc') -vshard.router.internal.collect_lua_garbage_cnt == 0 -lua_gc.internal.bg_fiber == nil -configs.cfg_2.collect_lua_garbage = true -routers[5]:cfg(configs.cfg_2) -lua_gc.internal.bg_fiber ~= nil -routers[7]:cfg(configs.cfg_2) -lua_gc.internal.bg_fiber ~= nil -vshard.router.internal.collect_lua_garbage_cnt == 2 -package.loaded['vshard.router'] = nil -vshard.router = require('vshard.router') -vshard.router.internal.collect_lua_garbage_cnt == 2 -configs.cfg_2.collect_lua_garbage = nil -routers[5]:cfg(configs.cfg_2) -lua_gc.internal.bg_fiber ~= nil -routers[7]:cfg(configs.cfg_2) -vshard.router.internal.collect_lua_garbage_cnt == 0 -lua_gc.internal.bg_fiber == nil - -- Self checker. util.check_error(router_2.info) diff --git a/test/router/garbage_collector.result b/test/router/garbage_collector.result deleted file mode 100644 index 7780046..0000000 --- a/test/router/garbage_collector.result +++ /dev/null @@ -1,116 +0,0 @@ -test_run = require('test_run').new() ---- -... -REPLICASET_1 = { 'storage_1_a', 'storage_1_b' } ---- -... -REPLICASET_2 = { 'storage_2_a', 'storage_2_b' } ---- -... -test_run:create_cluster(REPLICASET_1, 'router') ---- -... -test_run:create_cluster(REPLICASET_2, 'router') ---- -... -util = require('util') ---- -... -util.wait_master(test_run, REPLICASET_1, 'storage_1_a') ---- -... -util.wait_master(test_run, REPLICASET_2, 'storage_2_a') ---- -... -test_run:cmd("create server router_1 with script='router/router_1.lua'") ---- -- true -... -test_run:cmd("start server router_1") ---- -- true -... --- --- gh-77: garbage collection options and Lua garbage collection. --- -test_run:switch('router_1') ---- -- true -... -fiber = require('fiber') ---- -... -lua_gc = require('vshard.lua_gc') ---- -... -cfg.collect_lua_garbage = true ---- -... -vshard.router.cfg(cfg) ---- -... -lua_gc.internal.bg_fiber ~= nil ---- -- true -... --- Check that `collectgarbage()` was really called. -a = setmetatable({}, {__mode = 'v'}) ---- -... -a.k = {b = 100} ---- -... -iterations = lua_gc.internal.iterations ---- -... -lua_gc.internal.bg_fiber:wakeup() ---- -... -while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end ---- -... -a.k ---- -- null -... -lua_gc.internal.interval = 0.001 ---- -... -cfg.collect_lua_garbage = false ---- -... -vshard.router.cfg(cfg) ---- -... -lua_gc.internal.bg_fiber == nil ---- -- true -... -iterations = lua_gc.internal.iterations ---- -... -fiber.sleep(0.01) ---- -... -iterations == lua_gc.internal.iterations ---- -- true -... -test_run:switch("default") ---- -- true -... -test_run:cmd("stop server router_1") ---- -- true -... -test_run:cmd("cleanup server router_1") ---- -- true -... -test_run:drop_cluster(REPLICASET_1) ---- -... -test_run:drop_cluster(REPLICASET_2) ---- -... diff --git a/test/router/garbage_collector.test.lua b/test/router/garbage_collector.test.lua deleted file mode 100644 index e8d0876..0000000 --- a/test/router/garbage_collector.test.lua +++ /dev/null @@ -1,39 +0,0 @@ -test_run = require('test_run').new() -REPLICASET_1 = { 'storage_1_a', 'storage_1_b' } -REPLICASET_2 = { 'storage_2_a', 'storage_2_b' } -test_run:create_cluster(REPLICASET_1, 'router') -test_run:create_cluster(REPLICASET_2, 'router') -util = require('util') -util.wait_master(test_run, REPLICASET_1, 'storage_1_a') -util.wait_master(test_run, REPLICASET_2, 'storage_2_a') -test_run:cmd("create server router_1 with script='router/router_1.lua'") -test_run:cmd("start server router_1") --- --- gh-77: garbage collection options and Lua garbage collection. --- -test_run:switch('router_1') -fiber = require('fiber') -lua_gc = require('vshard.lua_gc') -cfg.collect_lua_garbage = true -vshard.router.cfg(cfg) -lua_gc.internal.bg_fiber ~= nil --- Check that `collectgarbage()` was really called. -a = setmetatable({}, {__mode = 'v'}) -a.k = {b = 100} -iterations = lua_gc.internal.iterations -lua_gc.internal.bg_fiber:wakeup() -while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end -a.k -lua_gc.internal.interval = 0.001 -cfg.collect_lua_garbage = false -vshard.router.cfg(cfg) -lua_gc.internal.bg_fiber == nil -iterations = lua_gc.internal.iterations -fiber.sleep(0.01) -iterations == lua_gc.internal.iterations - -test_run:switch("default") -test_run:cmd("stop server router_1") -test_run:cmd("cleanup server router_1") -test_run:drop_cluster(REPLICASET_1) -test_run:drop_cluster(REPLICASET_2) diff --git a/test/storage/garbage_collector.result b/test/storage/garbage_collector.result index 08ce085..c0212ff 100644 --- a/test/storage/garbage_collector.result +++ b/test/storage/garbage_collector.result @@ -111,68 +111,6 @@ test:select{} - - [10, 1] - [11, 2] ... --- --- gh-77: garbage collection options and Lua garbage collection. --- -_ = test_run:switch('storage_1_a') ---- -... -lua_gc = require('vshard.lua_gc') ---- -... -cfg.collect_lua_garbage = true ---- -... -vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a) ---- -... -lua_gc.internal.bg_fiber ~= nil ---- -- true -... --- Check that `collectgarbage()` was really called. -a = setmetatable({}, {__mode = 'v'}) ---- -... -a.k = {b = 100} ---- -... -iterations = lua_gc.internal.iterations ---- -... -lua_gc.internal.bg_fiber:wakeup() ---- -... -while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end ---- -... -a.k ---- -- null -... -lua_gc.internal.interval = 0.001 ---- -... -cfg.collect_lua_garbage = false ---- -... -vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a) ---- -... -lua_gc.internal.bg_fiber == nil ---- -- true -... -iterations = lua_gc.internal.iterations ---- -... -fiber.sleep(0.01) ---- -... -iterations == lua_gc.internal.iterations ---- -- true -... _ = test_run:switch('default') --- ... diff --git a/test/storage/garbage_collector.test.lua b/test/storage/garbage_collector.test.lua index 3704f11..a5aac53 100644 --- a/test/storage/garbage_collector.test.lua +++ b/test/storage/garbage_collector.test.lua @@ -40,29 +40,6 @@ _ = test_run:switch('storage_1_b') while box.space._bucket:get{3} ~= nil do fiber.sleep(0.1) end test:select{} --- --- gh-77: garbage collection options and Lua garbage collection. --- -_ = test_run:switch('storage_1_a') -lua_gc = require('vshard.lua_gc') -cfg.collect_lua_garbage = true -vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a) -lua_gc.internal.bg_fiber ~= nil --- Check that `collectgarbage()` was really called. -a = setmetatable({}, {__mode = 'v'}) -a.k = {b = 100} -iterations = lua_gc.internal.iterations -lua_gc.internal.bg_fiber:wakeup() -while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end -a.k -lua_gc.internal.interval = 0.001 -cfg.collect_lua_garbage = false -vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a) -lua_gc.internal.bg_fiber == nil -iterations = lua_gc.internal.iterations -fiber.sleep(0.01) -iterations == lua_gc.internal.iterations - _ = test_run:switch('default') test_run:drop_cluster(REPLICASET_2) test_run:drop_cluster(REPLICASET_1) diff --git a/test/unit/config.result b/test/unit/config.result index 676ee4d..3ddac10 100644 --- a/test/unit/config.result +++ b/test/unit/config.result @@ -426,28 +426,6 @@ _ = lcfg.check(cfg) --- ... -- --- gh-77: garbage collection options. --- -cfg.collect_lua_garbage = 100 ---- -... -check(cfg) ---- -- Garbage Lua collect necessity must be boolean -... -cfg.collect_lua_garbage = true ---- -... -_ = lcfg.check(cfg) ---- -... -cfg.collect_lua_garbage = false ---- -... -_ = lcfg.check(cfg) ---- -... --- -- gh-84: sync before master demotion, and allow to configure -- sync timeout. -- @@ -589,11 +567,14 @@ cfg.rebalancer_max_sending = nil --- ... -- --- Deprecated option does not break anything. +-- Deprecated options do not break anything. -- cfg.collect_bucket_garbage_interval = 100 --- ... +cfg.collect_lua_garbage = 100 +--- +... _ = lcfg.check(cfg) --- ... diff --git a/test/unit/config.test.lua b/test/unit/config.test.lua index 4dfbd77..11d126b 100644 --- a/test/unit/config.test.lua +++ b/test/unit/config.test.lua @@ -172,16 +172,6 @@ check(cfg) cfg.shard_index = 'vbucket' _ = lcfg.check(cfg) --- --- gh-77: garbage collection options. --- -cfg.collect_lua_garbage = 100 -check(cfg) -cfg.collect_lua_garbage = true -_ = lcfg.check(cfg) -cfg.collect_lua_garbage = false -_ = lcfg.check(cfg) - -- -- gh-84: sync before master demotion, and allow to configure -- sync timeout. @@ -237,9 +227,10 @@ lcfg.check(cfg).rebalancer_max_sending cfg.rebalancer_max_sending = nil -- --- Deprecated option does not break anything. +-- Deprecated options do not break anything. -- cfg.collect_bucket_garbage_interval = 100 +cfg.collect_lua_garbage = 100 _ = lcfg.check(cfg) -- diff --git a/vshard/cfg.lua b/vshard/cfg.lua index 1903967..fdb839f 100644 --- a/vshard/cfg.lua +++ b/vshard/cfg.lua @@ -272,8 +272,8 @@ local cfg_template = { reason = 'Has no effect anymore' }, collect_lua_garbage = { - type = 'boolean', name = 'Garbage Lua collect necessity', - is_optional = true, default = false + name = 'Garbage Lua collect necessity', is_deprecated = true, + reason = 'Has no effect anymore and never had much sense' }, sync_timeout = { type = 'non-negative number', name = 'Sync timeout', is_optional = true, diff --git a/vshard/consts.lua b/vshard/consts.lua index 55c769c..de113eb 100644 --- a/vshard/consts.lua +++ b/vshard/consts.lua @@ -41,7 +41,6 @@ return { GC_BACKOFF_INTERVAL = 5, RECOVERY_BACKOFF_INTERVAL = 5, REPLICA_BACKOFF_INTERVAL = 5, - COLLECT_LUA_GARBAGE_INTERVAL = 100; DEFAULT_BUCKET_SEND_TIMEOUT = 10, DEFAULT_BUCKET_RECV_TIMEOUT = 10, diff --git a/vshard/lua_gc.lua b/vshard/lua_gc.lua deleted file mode 100644 index c6c5cd3..0000000 --- a/vshard/lua_gc.lua +++ /dev/null @@ -1,54 +0,0 @@ --- --- This module implements background lua GC fiber. --- It's purpose is to make GC more aggressive. --- - -local lfiber = require('fiber') -local MODULE_INTERNALS = '__module_vshard_lua_gc' - -local M = rawget(_G, MODULE_INTERNALS) -if not M then - M = { - -- Background fiber. - bg_fiber = nil, - -- GC interval in seconds. - interval = nil, - -- Main loop. - -- Stored here to make the fiber reloadable. - main_loop = nil, - -- Number of `collectgarbage()` calls. - iterations = 0, - } -end - -M.main_loop = function() - lfiber.sleep(M.interval) - collectgarbage() - M.iterations = M.iterations + 1 - return M.main_loop() -end - -local function set_state(active, interval) - assert(type(interval) == 'number') - M.interval = interval - if active and not M.bg_fiber then - M.bg_fiber = lfiber.create(M.main_loop) - M.bg_fiber:name('vshard.lua_gc') - end - if not active and M.bg_fiber then - M.bg_fiber:cancel() - M.bg_fiber = nil - end - if active then - M.bg_fiber:wakeup() - end -end - -if not rawget(_G, MODULE_INTERNALS) then - rawset(_G, MODULE_INTERNALS, M) -end - -return { - set_state = set_state, - internal = M, -} diff --git a/vshard/router/init.lua b/vshard/router/init.lua index ebeaac9..064bd5a 100644 --- a/vshard/router/init.lua +++ b/vshard/router/init.lua @@ -8,8 +8,7 @@ local MODULE_INTERNALS = '__module_vshard_router' if rawget(_G, MODULE_INTERNALS) then local vshard_modules = { 'vshard.consts', 'vshard.error', 'vshard.cfg', - 'vshard.hash', 'vshard.replicaset', 'vshard.util', - 'vshard.lua_gc', + 'vshard.hash', 'vshard.replicaset', 'vshard.util' } for _, module in pairs(vshard_modules) do package.loaded[module] = nil @@ -21,7 +20,6 @@ local lcfg = require('vshard.cfg') local lhash = require('vshard.hash') local lreplicaset = require('vshard.replicaset') local util = require('vshard.util') -local lua_gc = require('vshard.lua_gc') local seq_serializer = { __serialize = 'seq' } local M = rawget(_G, MODULE_INTERNALS) @@ -43,8 +41,6 @@ if not M then -- This counter is used to restart background fibers with -- new reloaded code. module_version = 0, - -- Number of router which require collecting lua garbage. - collect_lua_garbage_cnt = 0, ----------------------- Map-Reduce ----------------------- -- Storage Ref ID. It must be unique for each ref request @@ -79,8 +75,6 @@ local ROUTER_TEMPLATE = { -- Bucket count stored on all replicasets. total_bucket_count = 0, known_bucket_count = 0, - -- Boolean lua_gc state (create periodic gc task). - collect_lua_garbage = nil, -- Timeout after which a ping is considered to be -- unacknowledged. Used by failover fiber to detect if a -- node is down. @@ -133,26 +127,6 @@ local function route_map_clear(router) end end --- --- Increase/decrease number of routers which require to collect --- a lua garbage and change state of the `lua_gc` fiber. --- - -local function lua_gc_cnt_inc() - M.collect_lua_garbage_cnt = M.collect_lua_garbage_cnt + 1 - if M.collect_lua_garbage_cnt == 1 then - lua_gc.set_state(true, consts.COLLECT_LUA_GARBAGE_INTERVAL) - end -end - -local function lua_gc_cnt_dec() - M.collect_lua_garbage_cnt = M.collect_lua_garbage_cnt - 1 - assert(M.collect_lua_garbage_cnt >= 0) - if M.collect_lua_garbage_cnt == 0 then - lua_gc.set_state(false, consts.COLLECT_LUA_GARBAGE_INTERVAL) - end -end - -------------------------------------------------------------------------------- -- Discovery -------------------------------------------------------------------------------- @@ -1187,19 +1161,11 @@ local function router_cfg(router, cfg, is_reload) for _, replicaset in pairs(new_replicasets) do replicaset:connect_all() end - -- Change state of lua GC. - if vshard_cfg.collect_lua_garbage and not router.collect_lua_garbage then - lua_gc_cnt_inc() - elseif not vshard_cfg.collect_lua_garbage and - router.collect_lua_garbage then - lua_gc_cnt_dec() - end lreplicaset.wait_masters_connect(new_replicasets) lreplicaset.outdate_replicasets(router.replicasets, vshard_cfg.connection_outdate_delay) router.connection_outdate_delay = vshard_cfg.connection_outdate_delay router.total_bucket_count = vshard_cfg.bucket_count - router.collect_lua_garbage = vshard_cfg.collect_lua_garbage router.current_cfg = cfg router.replicasets = new_replicasets router.failover_ping_timeout = vshard_cfg.failover_ping_timeout diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index 78103cf..1642609 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -15,8 +15,7 @@ if rawget(_G, MODULE_INTERNALS) then local vshard_modules = { 'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.replicaset', 'vshard.util', - 'vshard.storage.reload_evolution', - 'vshard.lua_gc', 'vshard.rlist', 'vshard.registry', + 'vshard.storage.reload_evolution', 'vshard.rlist', 'vshard.registry', 'vshard.heap', 'vshard.storage.ref', 'vshard.storage.sched', } for _, module in pairs(vshard_modules) do @@ -29,7 +28,6 @@ local lerror = require('vshard.error') local lcfg = require('vshard.cfg') local lreplicaset = require('vshard.replicaset') local util = require('vshard.util') -local lua_gc = require('vshard.lua_gc') local lregistry = require('vshard.registry') local lref = require('vshard.storage.ref') local lsched = require('vshard.storage.sched') @@ -151,8 +149,6 @@ if not M then ------------------- Garbage collection ------------------- -- Fiber to remove garbage buckets data. collect_bucket_garbage_fiber = nil, - -- Boolean lua_gc state (create periodic gc task). - collect_lua_garbage = nil, -------------------- Bucket recovery --------------------- recovery_fiber = nil, @@ -2746,7 +2742,6 @@ local function storage_cfg(cfg, this_replica_uuid, is_reload) vshard_cfg.rebalancer_disbalance_threshold M.rebalancer_receiving_quota = vshard_cfg.rebalancer_max_receiving M.shard_index = vshard_cfg.shard_index - M.collect_lua_garbage = vshard_cfg.collect_lua_garbage M.rebalancer_worker_count = vshard_cfg.rebalancer_max_sending M.current_cfg = cfg @@ -2773,7 +2768,6 @@ local function storage_cfg(cfg, this_replica_uuid, is_reload) M.rebalancer_fiber:cancel() M.rebalancer_fiber = nil end - lua_gc.set_state(M.collect_lua_garbage, consts.COLLECT_LUA_GARBAGE_INTERVAL) M.is_configured = true -- Destroy connections, not used in a new configuration. collectgarbage() -- 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call 2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call Vladislav Shpilevoy via Tarantool-patches @ 2022-01-28 5:47 ` Oleg Babin via Tarantool-patches 2022-01-28 22:19 ` Vladislav Shpilevoy via Tarantool-patches 0 siblings, 1 reply; 6+ messages in thread From: Oleg Babin via Tarantool-patches @ 2022-01-28 5:47 UTC (permalink / raw) To: Vladislav Shpilevoy, tarantool-patches Thanks for your patch. Consider also following diff diff --git a/vshard/CMakeLists.txt b/vshard/CMakeLists.txt index 2a15df5..2d9be25 100644 --- a/vshard/CMakeLists.txt +++ b/vshard/CMakeLists.txt @@ -7,5 +7,5 @@ add_subdirectory(router) # Install module install(FILES cfg.lua error.lua consts.lua hash.lua init.lua replicaset.lua - util.lua lua_gc.lua rlist.lua heap.lua registry.lua + util.lua rlist.lua heap.lua registry.lua DESTINATION ${TARANTOOL_INSTALL_LUADIR}/vshard) After it LGTM. On 28.01.2022 03:23, Vladislav Shpilevoy wrote: > It was introduced because somebody very long time ago complained > that otherwise router goes OOM and the person was sure it was due > to Lua GC working too rare/late. > > This of course makes little sense. Either Lua GC was never the > issue, or if it is - then it should be fixed in Lua implementation > in the core. In case anybody still needs that due to any reason, > he can start his own fiber making periodic collectgarbage(). This > has nothing to do with vshard and never had, really. > > The patch is done now in scope of general code cleanup from old > unnecessary stuff, like Tarantool < 1.10.0 support. > > @TarantoolBot document > Title: VShard cfg option collect_lua_garbage is deprecated > > It won't do anything now, only print a warning that it is > deprecated. > --- > test/multiple_routers/multiple_routers.result | 64 ---------- > .../multiple_routers.test.lua | 20 --- > test/router/garbage_collector.result | 116 ------------------ > test/router/garbage_collector.test.lua | 39 ------ > test/storage/garbage_collector.result | 62 ---------- > test/storage/garbage_collector.test.lua | 23 ---- > test/unit/config.result | 27 +--- > test/unit/config.test.lua | 13 +- > vshard/cfg.lua | 4 +- > vshard/consts.lua | 1 - > vshard/lua_gc.lua | 54 -------- > vshard/router/init.lua | 36 +----- > vshard/storage/init.lua | 8 +- > 13 files changed, 10 insertions(+), 457 deletions(-) > delete mode 100644 test/router/garbage_collector.result > delete mode 100644 test/router/garbage_collector.test.lua > delete mode 100644 vshard/lua_gc.lua > > diff --git a/test/multiple_routers/multiple_routers.result b/test/multiple_routers/multiple_routers.result > index b18c194..91b5d29 100644 > --- a/test/multiple_routers/multiple_routers.result > +++ b/test/multiple_routers/multiple_routers.result > @@ -212,70 +212,6 @@ routers[5]:call(1, 'read', 'do_select', {2}) > --- > - [[2, 2]] > ... > --- Check lua_gc counter. > -lua_gc = require('vshard.lua_gc') > ---- > -... > -vshard.router.internal.collect_lua_garbage_cnt == 0 > ---- > -- true > -... > -lua_gc.internal.bg_fiber == nil > ---- > -- true > -... > -configs.cfg_2.collect_lua_garbage = true > ---- > -... > -routers[5]:cfg(configs.cfg_2) > ---- > -... > -lua_gc.internal.bg_fiber ~= nil > ---- > -- true > -... > -routers[7]:cfg(configs.cfg_2) > ---- > -... > -lua_gc.internal.bg_fiber ~= nil > ---- > -- true > -... > -vshard.router.internal.collect_lua_garbage_cnt == 2 > ---- > -- true > -... > -package.loaded['vshard.router'] = nil > ---- > -... > -vshard.router = require('vshard.router') > ---- > -... > -vshard.router.internal.collect_lua_garbage_cnt == 2 > ---- > -- true > -... > -configs.cfg_2.collect_lua_garbage = nil > ---- > -... > -routers[5]:cfg(configs.cfg_2) > ---- > -... > -lua_gc.internal.bg_fiber ~= nil > ---- > -- true > -... > -routers[7]:cfg(configs.cfg_2) > ---- > -... > -vshard.router.internal.collect_lua_garbage_cnt == 0 > ---- > -- true > -... > -lua_gc.internal.bg_fiber == nil > ---- > -- true > -... > -- Self checker. > util.check_error(router_2.info) > --- > diff --git a/test/multiple_routers/multiple_routers.test.lua b/test/multiple_routers/multiple_routers.test.lua > index b5da5cf..c61f71d 100644 > --- a/test/multiple_routers/multiple_routers.test.lua > +++ b/test/multiple_routers/multiple_routers.test.lua > @@ -80,26 +80,6 @@ vshard.router.call(1, 'read', 'do_select', {1}) > router_2:call(1, 'read', 'do_select', {2}) > routers[5]:call(1, 'read', 'do_select', {2}) > > --- Check lua_gc counter. > -lua_gc = require('vshard.lua_gc') > -vshard.router.internal.collect_lua_garbage_cnt == 0 > -lua_gc.internal.bg_fiber == nil > -configs.cfg_2.collect_lua_garbage = true > -routers[5]:cfg(configs.cfg_2) > -lua_gc.internal.bg_fiber ~= nil > -routers[7]:cfg(configs.cfg_2) > -lua_gc.internal.bg_fiber ~= nil > -vshard.router.internal.collect_lua_garbage_cnt == 2 > -package.loaded['vshard.router'] = nil > -vshard.router = require('vshard.router') > -vshard.router.internal.collect_lua_garbage_cnt == 2 > -configs.cfg_2.collect_lua_garbage = nil > -routers[5]:cfg(configs.cfg_2) > -lua_gc.internal.bg_fiber ~= nil > -routers[7]:cfg(configs.cfg_2) > -vshard.router.internal.collect_lua_garbage_cnt == 0 > -lua_gc.internal.bg_fiber == nil > - > -- Self checker. > util.check_error(router_2.info) > > diff --git a/test/router/garbage_collector.result b/test/router/garbage_collector.result > deleted file mode 100644 > index 7780046..0000000 > --- a/test/router/garbage_collector.result > +++ /dev/null > @@ -1,116 +0,0 @@ > -test_run = require('test_run').new() > ---- > -... > -REPLICASET_1 = { 'storage_1_a', 'storage_1_b' } > ---- > -... > -REPLICASET_2 = { 'storage_2_a', 'storage_2_b' } > ---- > -... > -test_run:create_cluster(REPLICASET_1, 'router') > ---- > -... > -test_run:create_cluster(REPLICASET_2, 'router') > ---- > -... > -util = require('util') > ---- > -... > -util.wait_master(test_run, REPLICASET_1, 'storage_1_a') > ---- > -... > -util.wait_master(test_run, REPLICASET_2, 'storage_2_a') > ---- > -... > -test_run:cmd("create server router_1 with script='router/router_1.lua'") > ---- > -- true > -... > -test_run:cmd("start server router_1") > ---- > -- true > -... > --- > --- gh-77: garbage collection options and Lua garbage collection. > --- > -test_run:switch('router_1') > ---- > -- true > -... > -fiber = require('fiber') > ---- > -... > -lua_gc = require('vshard.lua_gc') > ---- > -... > -cfg.collect_lua_garbage = true > ---- > -... > -vshard.router.cfg(cfg) > ---- > -... > -lua_gc.internal.bg_fiber ~= nil > ---- > -- true > -... > --- Check that `collectgarbage()` was really called. > -a = setmetatable({}, {__mode = 'v'}) > ---- > -... > -a.k = {b = 100} > ---- > -... > -iterations = lua_gc.internal.iterations > ---- > -... > -lua_gc.internal.bg_fiber:wakeup() > ---- > -... > -while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end > ---- > -... > -a.k > ---- > -- null > -... > -lua_gc.internal.interval = 0.001 > ---- > -... > -cfg.collect_lua_garbage = false > ---- > -... > -vshard.router.cfg(cfg) > ---- > -... > -lua_gc.internal.bg_fiber == nil > ---- > -- true > -... > -iterations = lua_gc.internal.iterations > ---- > -... > -fiber.sleep(0.01) > ---- > -... > -iterations == lua_gc.internal.iterations > ---- > -- true > -... > -test_run:switch("default") > ---- > -- true > -... > -test_run:cmd("stop server router_1") > ---- > -- true > -... > -test_run:cmd("cleanup server router_1") > ---- > -- true > -... > -test_run:drop_cluster(REPLICASET_1) > ---- > -... > -test_run:drop_cluster(REPLICASET_2) > ---- > -... > diff --git a/test/router/garbage_collector.test.lua b/test/router/garbage_collector.test.lua > deleted file mode 100644 > index e8d0876..0000000 > --- a/test/router/garbage_collector.test.lua > +++ /dev/null > @@ -1,39 +0,0 @@ > -test_run = require('test_run').new() > -REPLICASET_1 = { 'storage_1_a', 'storage_1_b' } > -REPLICASET_2 = { 'storage_2_a', 'storage_2_b' } > -test_run:create_cluster(REPLICASET_1, 'router') > -test_run:create_cluster(REPLICASET_2, 'router') > -util = require('util') > -util.wait_master(test_run, REPLICASET_1, 'storage_1_a') > -util.wait_master(test_run, REPLICASET_2, 'storage_2_a') > -test_run:cmd("create server router_1 with script='router/router_1.lua'") > -test_run:cmd("start server router_1") > --- > --- gh-77: garbage collection options and Lua garbage collection. > --- > -test_run:switch('router_1') > -fiber = require('fiber') > -lua_gc = require('vshard.lua_gc') > -cfg.collect_lua_garbage = true > -vshard.router.cfg(cfg) > -lua_gc.internal.bg_fiber ~= nil > --- Check that `collectgarbage()` was really called. > -a = setmetatable({}, {__mode = 'v'}) > -a.k = {b = 100} > -iterations = lua_gc.internal.iterations > -lua_gc.internal.bg_fiber:wakeup() > -while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end > -a.k > -lua_gc.internal.interval = 0.001 > -cfg.collect_lua_garbage = false > -vshard.router.cfg(cfg) > -lua_gc.internal.bg_fiber == nil > -iterations = lua_gc.internal.iterations > -fiber.sleep(0.01) > -iterations == lua_gc.internal.iterations > - > -test_run:switch("default") > -test_run:cmd("stop server router_1") > -test_run:cmd("cleanup server router_1") > -test_run:drop_cluster(REPLICASET_1) > -test_run:drop_cluster(REPLICASET_2) > diff --git a/test/storage/garbage_collector.result b/test/storage/garbage_collector.result > index 08ce085..c0212ff 100644 > --- a/test/storage/garbage_collector.result > +++ b/test/storage/garbage_collector.result > @@ -111,68 +111,6 @@ test:select{} > - - [10, 1] > - [11, 2] > ... > --- > --- gh-77: garbage collection options and Lua garbage collection. > --- > -_ = test_run:switch('storage_1_a') > ---- > -... > -lua_gc = require('vshard.lua_gc') > ---- > -... > -cfg.collect_lua_garbage = true > ---- > -... > -vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a) > ---- > -... > -lua_gc.internal.bg_fiber ~= nil > ---- > -- true > -... > --- Check that `collectgarbage()` was really called. > -a = setmetatable({}, {__mode = 'v'}) > ---- > -... > -a.k = {b = 100} > ---- > -... > -iterations = lua_gc.internal.iterations > ---- > -... > -lua_gc.internal.bg_fiber:wakeup() > ---- > -... > -while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end > ---- > -... > -a.k > ---- > -- null > -... > -lua_gc.internal.interval = 0.001 > ---- > -... > -cfg.collect_lua_garbage = false > ---- > -... > -vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a) > ---- > -... > -lua_gc.internal.bg_fiber == nil > ---- > -- true > -... > -iterations = lua_gc.internal.iterations > ---- > -... > -fiber.sleep(0.01) > ---- > -... > -iterations == lua_gc.internal.iterations > ---- > -- true > -... > _ = test_run:switch('default') > --- > ... > diff --git a/test/storage/garbage_collector.test.lua b/test/storage/garbage_collector.test.lua > index 3704f11..a5aac53 100644 > --- a/test/storage/garbage_collector.test.lua > +++ b/test/storage/garbage_collector.test.lua > @@ -40,29 +40,6 @@ _ = test_run:switch('storage_1_b') > while box.space._bucket:get{3} ~= nil do fiber.sleep(0.1) end > test:select{} > > --- > --- gh-77: garbage collection options and Lua garbage collection. > --- > -_ = test_run:switch('storage_1_a') > -lua_gc = require('vshard.lua_gc') > -cfg.collect_lua_garbage = true > -vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a) > -lua_gc.internal.bg_fiber ~= nil > --- Check that `collectgarbage()` was really called. > -a = setmetatable({}, {__mode = 'v'}) > -a.k = {b = 100} > -iterations = lua_gc.internal.iterations > -lua_gc.internal.bg_fiber:wakeup() > -while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end > -a.k > -lua_gc.internal.interval = 0.001 > -cfg.collect_lua_garbage = false > -vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a) > -lua_gc.internal.bg_fiber == nil > -iterations = lua_gc.internal.iterations > -fiber.sleep(0.01) > -iterations == lua_gc.internal.iterations > - > _ = test_run:switch('default') > test_run:drop_cluster(REPLICASET_2) > test_run:drop_cluster(REPLICASET_1) > diff --git a/test/unit/config.result b/test/unit/config.result > index 676ee4d..3ddac10 100644 > --- a/test/unit/config.result > +++ b/test/unit/config.result > @@ -426,28 +426,6 @@ _ = lcfg.check(cfg) > --- > ... > -- > --- gh-77: garbage collection options. > --- > -cfg.collect_lua_garbage = 100 > ---- > -... > -check(cfg) > ---- > -- Garbage Lua collect necessity must be boolean > -... > -cfg.collect_lua_garbage = true > ---- > -... > -_ = lcfg.check(cfg) > ---- > -... > -cfg.collect_lua_garbage = false > ---- > -... > -_ = lcfg.check(cfg) > ---- > -... > --- > -- gh-84: sync before master demotion, and allow to configure > -- sync timeout. > -- > @@ -589,11 +567,14 @@ cfg.rebalancer_max_sending = nil > --- > ... > -- > --- Deprecated option does not break anything. > +-- Deprecated options do not break anything. > -- > cfg.collect_bucket_garbage_interval = 100 > --- > ... > +cfg.collect_lua_garbage = 100 > +--- > +... > _ = lcfg.check(cfg) > --- > ... > diff --git a/test/unit/config.test.lua b/test/unit/config.test.lua > index 4dfbd77..11d126b 100644 > --- a/test/unit/config.test.lua > +++ b/test/unit/config.test.lua > @@ -172,16 +172,6 @@ check(cfg) > cfg.shard_index = 'vbucket' > _ = lcfg.check(cfg) > > --- > --- gh-77: garbage collection options. > --- > -cfg.collect_lua_garbage = 100 > -check(cfg) > -cfg.collect_lua_garbage = true > -_ = lcfg.check(cfg) > -cfg.collect_lua_garbage = false > -_ = lcfg.check(cfg) > - > -- > -- gh-84: sync before master demotion, and allow to configure > -- sync timeout. > @@ -237,9 +227,10 @@ lcfg.check(cfg).rebalancer_max_sending > cfg.rebalancer_max_sending = nil > > -- > --- Deprecated option does not break anything. > +-- Deprecated options do not break anything. > -- > cfg.collect_bucket_garbage_interval = 100 > +cfg.collect_lua_garbage = 100 > _ = lcfg.check(cfg) > > -- > diff --git a/vshard/cfg.lua b/vshard/cfg.lua > index 1903967..fdb839f 100644 > --- a/vshard/cfg.lua > +++ b/vshard/cfg.lua > @@ -272,8 +272,8 @@ local cfg_template = { > reason = 'Has no effect anymore' > }, > collect_lua_garbage = { > - type = 'boolean', name = 'Garbage Lua collect necessity', > - is_optional = true, default = false > + name = 'Garbage Lua collect necessity', is_deprecated = true, > + reason = 'Has no effect anymore and never had much sense' > }, > sync_timeout = { > type = 'non-negative number', name = 'Sync timeout', is_optional = true, > diff --git a/vshard/consts.lua b/vshard/consts.lua > index 55c769c..de113eb 100644 > --- a/vshard/consts.lua > +++ b/vshard/consts.lua > @@ -41,7 +41,6 @@ return { > GC_BACKOFF_INTERVAL = 5, > RECOVERY_BACKOFF_INTERVAL = 5, > REPLICA_BACKOFF_INTERVAL = 5, > - COLLECT_LUA_GARBAGE_INTERVAL = 100; > DEFAULT_BUCKET_SEND_TIMEOUT = 10, > DEFAULT_BUCKET_RECV_TIMEOUT = 10, > > diff --git a/vshard/lua_gc.lua b/vshard/lua_gc.lua > deleted file mode 100644 > index c6c5cd3..0000000 > --- a/vshard/lua_gc.lua > +++ /dev/null > @@ -1,54 +0,0 @@ > --- > --- This module implements background lua GC fiber. > --- It's purpose is to make GC more aggressive. > --- > - > -local lfiber = require('fiber') > -local MODULE_INTERNALS = '__module_vshard_lua_gc' > - > -local M = rawget(_G, MODULE_INTERNALS) > -if not M then > - M = { > - -- Background fiber. > - bg_fiber = nil, > - -- GC interval in seconds. > - interval = nil, > - -- Main loop. > - -- Stored here to make the fiber reloadable. > - main_loop = nil, > - -- Number of `collectgarbage()` calls. > - iterations = 0, > - } > -end > - > -M.main_loop = function() > - lfiber.sleep(M.interval) > - collectgarbage() > - M.iterations = M.iterations + 1 > - return M.main_loop() > -end > - > -local function set_state(active, interval) > - assert(type(interval) == 'number') > - M.interval = interval > - if active and not M.bg_fiber then > - M.bg_fiber = lfiber.create(M.main_loop) > - M.bg_fiber:name('vshard.lua_gc') > - end > - if not active and M.bg_fiber then > - M.bg_fiber:cancel() > - M.bg_fiber = nil > - end > - if active then > - M.bg_fiber:wakeup() > - end > -end > - > -if not rawget(_G, MODULE_INTERNALS) then > - rawset(_G, MODULE_INTERNALS, M) > -end > - > -return { > - set_state = set_state, > - internal = M, > -} > diff --git a/vshard/router/init.lua b/vshard/router/init.lua > index ebeaac9..064bd5a 100644 > --- a/vshard/router/init.lua > +++ b/vshard/router/init.lua > @@ -8,8 +8,7 @@ local MODULE_INTERNALS = '__module_vshard_router' > if rawget(_G, MODULE_INTERNALS) then > local vshard_modules = { > 'vshard.consts', 'vshard.error', 'vshard.cfg', > - 'vshard.hash', 'vshard.replicaset', 'vshard.util', > - 'vshard.lua_gc', > + 'vshard.hash', 'vshard.replicaset', 'vshard.util' > } > for _, module in pairs(vshard_modules) do > package.loaded[module] = nil > @@ -21,7 +20,6 @@ local lcfg = require('vshard.cfg') > local lhash = require('vshard.hash') > local lreplicaset = require('vshard.replicaset') > local util = require('vshard.util') > -local lua_gc = require('vshard.lua_gc') > local seq_serializer = { __serialize = 'seq' } > > local M = rawget(_G, MODULE_INTERNALS) > @@ -43,8 +41,6 @@ if not M then > -- This counter is used to restart background fibers with > -- new reloaded code. > module_version = 0, > - -- Number of router which require collecting lua garbage. > - collect_lua_garbage_cnt = 0, > > ----------------------- Map-Reduce ----------------------- > -- Storage Ref ID. It must be unique for each ref request > @@ -79,8 +75,6 @@ local ROUTER_TEMPLATE = { > -- Bucket count stored on all replicasets. > total_bucket_count = 0, > known_bucket_count = 0, > - -- Boolean lua_gc state (create periodic gc task). > - collect_lua_garbage = nil, > -- Timeout after which a ping is considered to be > -- unacknowledged. Used by failover fiber to detect if a > -- node is down. > @@ -133,26 +127,6 @@ local function route_map_clear(router) > end > end > > --- > --- Increase/decrease number of routers which require to collect > --- a lua garbage and change state of the `lua_gc` fiber. > --- > - > -local function lua_gc_cnt_inc() > - M.collect_lua_garbage_cnt = M.collect_lua_garbage_cnt + 1 > - if M.collect_lua_garbage_cnt == 1 then > - lua_gc.set_state(true, consts.COLLECT_LUA_GARBAGE_INTERVAL) > - end > -end > - > -local function lua_gc_cnt_dec() > - M.collect_lua_garbage_cnt = M.collect_lua_garbage_cnt - 1 > - assert(M.collect_lua_garbage_cnt >= 0) > - if M.collect_lua_garbage_cnt == 0 then > - lua_gc.set_state(false, consts.COLLECT_LUA_GARBAGE_INTERVAL) > - end > -end > - > -------------------------------------------------------------------------------- > -- Discovery > -------------------------------------------------------------------------------- > @@ -1187,19 +1161,11 @@ local function router_cfg(router, cfg, is_reload) > for _, replicaset in pairs(new_replicasets) do > replicaset:connect_all() > end > - -- Change state of lua GC. > - if vshard_cfg.collect_lua_garbage and not router.collect_lua_garbage then > - lua_gc_cnt_inc() > - elseif not vshard_cfg.collect_lua_garbage and > - router.collect_lua_garbage then > - lua_gc_cnt_dec() > - end > lreplicaset.wait_masters_connect(new_replicasets) > lreplicaset.outdate_replicasets(router.replicasets, > vshard_cfg.connection_outdate_delay) > router.connection_outdate_delay = vshard_cfg.connection_outdate_delay > router.total_bucket_count = vshard_cfg.bucket_count > - router.collect_lua_garbage = vshard_cfg.collect_lua_garbage > router.current_cfg = cfg > router.replicasets = new_replicasets > router.failover_ping_timeout = vshard_cfg.failover_ping_timeout > diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua > index 78103cf..1642609 100644 > --- a/vshard/storage/init.lua > +++ b/vshard/storage/init.lua > @@ -15,8 +15,7 @@ if rawget(_G, MODULE_INTERNALS) then > local vshard_modules = { > 'vshard.consts', 'vshard.error', 'vshard.cfg', > 'vshard.replicaset', 'vshard.util', > - 'vshard.storage.reload_evolution', > - 'vshard.lua_gc', 'vshard.rlist', 'vshard.registry', > + 'vshard.storage.reload_evolution', 'vshard.rlist', 'vshard.registry', > 'vshard.heap', 'vshard.storage.ref', 'vshard.storage.sched', > } > for _, module in pairs(vshard_modules) do > @@ -29,7 +28,6 @@ local lerror = require('vshard.error') > local lcfg = require('vshard.cfg') > local lreplicaset = require('vshard.replicaset') > local util = require('vshard.util') > -local lua_gc = require('vshard.lua_gc') > local lregistry = require('vshard.registry') > local lref = require('vshard.storage.ref') > local lsched = require('vshard.storage.sched') > @@ -151,8 +149,6 @@ if not M then > ------------------- Garbage collection ------------------- > -- Fiber to remove garbage buckets data. > collect_bucket_garbage_fiber = nil, > - -- Boolean lua_gc state (create periodic gc task). > - collect_lua_garbage = nil, > > -------------------- Bucket recovery --------------------- > recovery_fiber = nil, > @@ -2746,7 +2742,6 @@ local function storage_cfg(cfg, this_replica_uuid, is_reload) > vshard_cfg.rebalancer_disbalance_threshold > M.rebalancer_receiving_quota = vshard_cfg.rebalancer_max_receiving > M.shard_index = vshard_cfg.shard_index > - M.collect_lua_garbage = vshard_cfg.collect_lua_garbage > M.rebalancer_worker_count = vshard_cfg.rebalancer_max_sending > M.current_cfg = cfg > > @@ -2773,7 +2768,6 @@ local function storage_cfg(cfg, this_replica_uuid, is_reload) > M.rebalancer_fiber:cancel() > M.rebalancer_fiber = nil > end > - lua_gc.set_state(M.collect_lua_garbage, consts.COLLECT_LUA_GARBAGE_INTERVAL) > M.is_configured = true > -- Destroy connections, not used in a new configuration. > collectgarbage() ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call 2022-01-28 5:47 ` Oleg Babin via Tarantool-patches @ 2022-01-28 22:19 ` Vladislav Shpilevoy via Tarantool-patches 0 siblings, 0 replies; 6+ messages in thread From: Vladislav Shpilevoy via Tarantool-patches @ 2022-01-28 22:19 UTC (permalink / raw) To: Oleg Babin, tarantool-patches Hi! Thanks for the review and for noticing the issue! On 28.01.2022 06:47, Oleg Babin wrote: > Thanks for your patch. > > Consider also following diff > > diff --git a/vshard/CMakeLists.txt b/vshard/CMakeLists.txt > index 2a15df5..2d9be25 100644 > --- a/vshard/CMakeLists.txt > +++ b/vshard/CMakeLists.txt > @@ -7,5 +7,5 @@ add_subdirectory(router) > > # Install module > install(FILES cfg.lua error.lua consts.lua hash.lua init.lua replicaset.lua > - util.lua lua_gc.lua rlist.lua heap.lua registry.lua > + util.lua rlist.lua heap.lua registry.lua > DESTINATION ${TARANTOOL_INSTALL_LUADIR}/vshard) > > > After it LGTM. Done and pushed to master. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-28 22:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-28 0:23 [Tarantool-patches] [PATCH vshard 0/2] Deprecations Vladislav Shpilevoy via Tarantool-patches 2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support Vladislav Shpilevoy via Tarantool-patches 2022-01-28 5:47 ` Oleg Babin via Tarantool-patches 2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call Vladislav Shpilevoy via Tarantool-patches 2022-01-28 5:47 ` Oleg Babin via Tarantool-patches 2022-01-28 22:19 ` Vladislav Shpilevoy via Tarantool-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox