[Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Fri Jan 28 03:23:17 MSK 2022


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)



More information about the Tarantool-patches mailing list