Tarantool development patches archive
 help / color / mirror / Atom feed
From: AKhatskevich <avkhatskevich@tarantool.org>
To: v.shpilevoy@tarantool.org, tarantool-patches@freelists.org
Subject: [tarantool-patches] [PATCH 2/3] Move lua gc to a dedicated module
Date: Tue, 31 Jul 2018 19:25:27 +0300	[thread overview]
Message-ID: <ae988884e373544e9ba3b39401b60033b08eb295.1533054045.git.avkhatskevich@tarantool.org> (raw)
In-Reply-To: <cover.1533054045.git.avkhatskevich@tarantool.org>
In-Reply-To: <cover.1533054045.git.avkhatskevich@tarantool.org>

`vshard.lua_gc.lua` is a new module which helps make gc work more
intense.
Before the commit that was a duty of router and storage.

Reasons to move lua gc to a separate module:
1. It is not a duty of vshard to collect garbage, so let gc fiber
   be as far from vshard as possible.
2. Next commits will introduce multiple routers feature, which require
   gc fiber to be a singleton.

Closes #138
---
 test/router/garbage_collector.result    | 27 +++++++++++------
 test/router/garbage_collector.test.lua  | 18 ++++++-----
 test/storage/garbage_collector.result   | 27 +++++++++--------
 test/storage/garbage_collector.test.lua | 22 ++++++--------
 vshard/lua_gc.lua                       | 54 +++++++++++++++++++++++++++++++++
 vshard/router/init.lua                  | 19 +++---------
 vshard/storage/init.lua                 | 20 ++++--------
 7 files changed, 116 insertions(+), 71 deletions(-)
 create mode 100644 vshard/lua_gc.lua

diff --git a/test/router/garbage_collector.result b/test/router/garbage_collector.result
index 3c2a4f1..a7474fc 100644
--- a/test/router/garbage_collector.result
+++ b/test/router/garbage_collector.result
@@ -40,27 +40,30 @@ test_run:switch('router_1')
 fiber = require('fiber')
 ---
 ...
-cfg.collect_lua_garbage = true
+lua_gc = require('vshard.lua_gc')
 ---
 ...
-iters = vshard.consts.COLLECT_LUA_GARBAGE_INTERVAL / vshard.consts.DISCOVERY_INTERVAL
+cfg.collect_lua_garbage = true
 ---
 ...
 vshard.router.cfg(cfg)
 ---
 ...
-a = setmetatable({}, {__mode = 'v'})
+lua_gc.internal.bg_fiber ~= nil
+---
+- true
+...
+iterations = lua_gc.internal.iterations
 ---
 ...
-a.k = {b = 100}
+lua_gc.internal.bg_fiber:wakeup()
 ---
 ...
-for i = 1, iters + 1 do vshard.router.discovery_wakeup() fiber.sleep(0.01) end
+while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end
 ---
 ...
-a.k
+lua_gc.internal.interval = 0.001
 ---
-- null
 ...
 cfg.collect_lua_garbage = false
 ---
@@ -68,13 +71,17 @@ cfg.collect_lua_garbage = false
 vshard.router.cfg(cfg)
 ---
 ...
-a.k = {b = 100}
+lua_gc.internal.bg_fiber == nil
+---
+- true
+...
+iterations = lua_gc.internal.iterations
 ---
 ...
-for i = 1, iters + 1 do vshard.router.discovery_wakeup() fiber.sleep(0.01) end
+fiber.sleep(0.01)
 ---
 ...
-a.k ~= nil
+iterations == lua_gc.internal.iterations
 ---
 - true
 ...
diff --git a/test/router/garbage_collector.test.lua b/test/router/garbage_collector.test.lua
index b3411cd..d1da8e9 100644
--- a/test/router/garbage_collector.test.lua
+++ b/test/router/garbage_collector.test.lua
@@ -13,18 +13,20 @@ test_run:cmd("start server router_1")
 --
 test_run:switch('router_1')
 fiber = require('fiber')
+lua_gc = require('vshard.lua_gc')
 cfg.collect_lua_garbage = true
-iters = vshard.consts.COLLECT_LUA_GARBAGE_INTERVAL / vshard.consts.DISCOVERY_INTERVAL
 vshard.router.cfg(cfg)
-a = setmetatable({}, {__mode = 'v'})
-a.k = {b = 100}
-for i = 1, iters + 1 do vshard.router.discovery_wakeup() fiber.sleep(0.01) end
-a.k
+lua_gc.internal.bg_fiber ~= nil
+iterations = lua_gc.internal.iterations
+lua_gc.internal.bg_fiber:wakeup()
+while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end
+lua_gc.internal.interval = 0.001
 cfg.collect_lua_garbage = false
 vshard.router.cfg(cfg)
-a.k = {b = 100}
-for i = 1, iters + 1 do vshard.router.discovery_wakeup() fiber.sleep(0.01) end
-a.k ~= nil
+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")
diff --git a/test/storage/garbage_collector.result b/test/storage/garbage_collector.result
index 3588fb4..d94ba24 100644
--- a/test/storage/garbage_collector.result
+++ b/test/storage/garbage_collector.result
@@ -120,7 +120,7 @@ test_run:switch('storage_1_a')
 fiber = require('fiber')
 ---
 ...
-log = require('log')
+lua_gc = require('vshard.lua_gc')
 ---
 ...
 cfg.collect_lua_garbage = true
@@ -129,24 +129,21 @@ cfg.collect_lua_garbage = true
 vshard.storage.cfg(cfg, names.storage_1_a)
 ---
 ...
--- Create a weak reference to a able {b = 100} - it must be
--- deleted on the next GC.
-a = setmetatable({}, {__mode = 'v'})
+lua_gc.internal.bg_fiber ~= nil
 ---
+- true
 ...
-a.k = {b = 100}
+iterations = lua_gc.internal.iterations
 ---
 ...
-iters = vshard.consts.COLLECT_LUA_GARBAGE_INTERVAL / vshard.consts.DEFAULT_COLLECT_BUCKET_GARBAGE_INTERVAL
+lua_gc.internal.bg_fiber:wakeup()
 ---
 ...
--- Wait until Lua GC deletes a.k.
-for i = 1, iters + 1 do vshard.storage.garbage_collector_wakeup() fiber.sleep(0.01) end
+while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end
 ---
 ...
-a.k
+lua_gc.internal.interval = 0.001
 ---
-- null
 ...
 cfg.collect_lua_garbage = false
 ---
@@ -154,13 +151,17 @@ cfg.collect_lua_garbage = false
 vshard.storage.cfg(cfg, names.storage_1_a)
 ---
 ...
-a.k = {b = 100}
+lua_gc.internal.bg_fiber == nil
+---
+- true
+...
+iterations = lua_gc.internal.iterations
 ---
 ...
-for i = 1, iters + 1 do vshard.storage.garbage_collector_wakeup() fiber.sleep(0.01) end
+fiber.sleep(0.01)
 ---
 ...
-a.k ~= nil
+iterations == lua_gc.internal.iterations
 ---
 - true
 ...
diff --git a/test/storage/garbage_collector.test.lua b/test/storage/garbage_collector.test.lua
index 79e76d8..ee3ecf4 100644
--- a/test/storage/garbage_collector.test.lua
+++ b/test/storage/garbage_collector.test.lua
@@ -46,22 +46,20 @@ customer:select{}
 --
 test_run:switch('storage_1_a')
 fiber = require('fiber')
-log = require('log')
+lua_gc = require('vshard.lua_gc')
 cfg.collect_lua_garbage = true
 vshard.storage.cfg(cfg, names.storage_1_a)
--- Create a weak reference to a able {b = 100} - it must be
--- deleted on the next GC.
-a = setmetatable({}, {__mode = 'v'})
-a.k = {b = 100}
-iters = vshard.consts.COLLECT_LUA_GARBAGE_INTERVAL / vshard.consts.DEFAULT_COLLECT_BUCKET_GARBAGE_INTERVAL
--- Wait until Lua GC deletes a.k.
-for i = 1, iters + 1 do vshard.storage.garbage_collector_wakeup() fiber.sleep(0.01) end
-a.k
+lua_gc.internal.bg_fiber ~= nil
+iterations = lua_gc.internal.iterations
+lua_gc.internal.bg_fiber:wakeup()
+while lua_gc.internal.iterations < iterations + 1 do fiber.sleep(0.01) end
+lua_gc.internal.interval = 0.001
 cfg.collect_lua_garbage = false
 vshard.storage.cfg(cfg, names.storage_1_a)
-a.k = {b = 100}
-for i = 1, iters + 1 do vshard.storage.garbage_collector_wakeup() fiber.sleep(0.01) end
-a.k ~= nil
+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)
diff --git a/vshard/lua_gc.lua b/vshard/lua_gc.lua
new file mode 100644
index 0000000..8d6af3e
--- /dev/null
+++ b/vshard/lua_gc.lua
@@ -0,0 +1,54 @@
+--
+-- 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
+local DEFALUT_INTERVAL = 100
+
+M.main_loop = function()
+    lfiber.sleep(M.interval or DEFALUT_INTERVAL)
+    collectgarbage()
+    M.iterations = M.iterations + 1
+    return M.main_loop()
+end
+
+local function set_state(active, interval)
+    M.inverval = 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 e2b2b22..3e127cb 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -7,6 +7,7 @@ if rawget(_G, MODULE_INTERNALS) then
     local vshard_modules = {
         'vshard.consts', 'vshard.error', 'vshard.cfg',
         'vshard.hash', 'vshard.replicaset', 'vshard.util',
+        'vshard.lua_gc',
     }
     for _, module in pairs(vshard_modules) do
         package.loaded[module] = nil
@@ -18,6 +19,7 @@ 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 M = rawget(_G, MODULE_INTERNALS)
 if not M then
@@ -43,8 +45,7 @@ if not M then
         discovery_fiber = nil,
         -- Bucket count stored on all replicasets.
         total_bucket_count = 0,
-        -- If true, then discovery fiber starts to call
-        -- collectgarbage() periodically.
+        -- Boolean lua_gc state (create periodic gc task).
         collect_lua_garbage = nil,
         -- This counter is used to restart background fibers with
         -- new reloaded code.
@@ -151,8 +152,6 @@ end
 --
 local function discovery_f()
     local module_version = M.module_version
-    local iterations_until_lua_gc =
-        consts.COLLECT_LUA_GARBAGE_INTERVAL / consts.DISCOVERY_INTERVAL
     while module_version == M.module_version do
         while not next(M.replicasets) do
             lfiber.sleep(consts.DISCOVERY_INTERVAL)
@@ -188,12 +187,6 @@ local function discovery_f()
                     M.route_map[bucket_id] = replicaset
                 end
             end
-            iterations_until_lua_gc = iterations_until_lua_gc - 1
-            if M.collect_lua_garbage and iterations_until_lua_gc == 0 then
-                iterations_until_lua_gc =
-                    consts.COLLECT_LUA_GARBAGE_INTERVAL / consts.DISCOVERY_INTERVAL
-                collectgarbage()
-            end
             lfiber.sleep(consts.DISCOVERY_INTERVAL)
         end
     end
@@ -504,7 +497,6 @@ local function router_cfg(cfg)
     end
     local new_replicasets = lreplicaset.buildall(vshard_cfg)
     local total_bucket_count = vshard_cfg.bucket_count
-    local collect_lua_garbage = vshard_cfg.collect_lua_garbage
     log.info("Calling box.cfg()...")
     for k, v in pairs(box_cfg) do
         log.info({[k] = v})
@@ -531,7 +523,7 @@ local function router_cfg(cfg)
                                     vshard_cfg.connection_outdate_delay)
     M.connection_outdate_delay = vshard_cfg.connection_outdate_delay
     M.total_bucket_count = total_bucket_count
-    M.collect_lua_garbage = collect_lua_garbage
+    M.collect_lua_garbage = vshard_cfg.collect_lua_garbage
     M.current_cfg = vshard_cfg
     M.replicasets = new_replicasets
     -- Update existing route map in-place.
@@ -548,8 +540,7 @@ local function router_cfg(cfg)
         M.discovery_fiber = util.reloadable_fiber_create(
             'vshard.discovery', M, 'discovery_f')
     end
-    -- Destroy connections, not used in a new configuration.
-    collectgarbage()
+    lua_gc.set_state(M.collect_lua_garbage, consts.COLLECT_LUA_GARBAGE_INTERVAL)
 end
 
 --------------------------------------------------------------------------------
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 75f5df9..1e11960 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -10,7 +10,8 @@ if rawget(_G, MODULE_INTERNALS) then
     local vshard_modules = {
         'vshard.consts', 'vshard.error', 'vshard.cfg',
         'vshard.replicaset', 'vshard.util',
-        'vshard.storage.reload_evolution'
+        'vshard.storage.reload_evolution',
+        'vshard.lua_gc',
     }
     for _, module in pairs(vshard_modules) do
         package.loaded[module] = nil
@@ -21,6 +22,7 @@ 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 reload_evolution = require('vshard.storage.reload_evolution')
 
 local M = rawget(_G, MODULE_INTERNALS)
@@ -75,8 +77,7 @@ if not M then
         collect_bucket_garbage_fiber = nil,
         -- Do buckets garbage collection once per this time.
         collect_bucket_garbage_interval = nil,
-        -- If true, then bucket garbage collection fiber starts to
-        -- call collectgarbage() periodically.
+        -- Boolean lua_gc state (create periodic gc task).
         collect_lua_garbage = nil,
 
         -------------------- Bucket recovery ---------------------
@@ -1063,9 +1064,6 @@ function collect_garbage_f()
     -- buckets_for_redirect is deleted, it gets empty_sent_buckets
     -- for next deletion.
     local empty_sent_buckets = {}
-    local iterations_until_lua_gc =
-        consts.COLLECT_LUA_GARBAGE_INTERVAL / M.collect_bucket_garbage_interval
-
     while M.module_version == module_version do
         -- Check if no changes in buckets configuration.
         if control.bucket_generation_collected ~= control.bucket_generation then
@@ -1106,12 +1104,6 @@ function collect_garbage_f()
             end
         end
 ::continue::
-        iterations_until_lua_gc = iterations_until_lua_gc - 1
-        if iterations_until_lua_gc == 0 and M.collect_lua_garbage then
-            iterations_until_lua_gc = consts.COLLECT_LUA_GARBAGE_INTERVAL /
-                                      M.collect_bucket_garbage_interval
-            collectgarbage()
-        end
         lfiber.sleep(M.collect_bucket_garbage_interval)
     end
 end
@@ -1590,7 +1582,6 @@ local function storage_cfg(cfg, this_replica_uuid)
     local shard_index = vshard_cfg.shard_index
     local collect_bucket_garbage_interval =
         vshard_cfg.collect_bucket_garbage_interval
-    local collect_lua_garbage = vshard_cfg.collect_lua_garbage
 
     -- It is considered that all possible errors during cfg
     -- process occur only before this place.
@@ -1646,7 +1637,7 @@ local function storage_cfg(cfg, this_replica_uuid)
     M.rebalancer_max_receiving = rebalancer_max_receiving
     M.shard_index = shard_index
     M.collect_bucket_garbage_interval = collect_bucket_garbage_interval
-    M.collect_lua_garbage = collect_lua_garbage
+    M.collect_lua_garbage = vshard_cfg.collect_lua_garbage
     M.current_cfg = vshard_cfg
 
     if was_master and not is_master then
@@ -1671,6 +1662,7 @@ local function storage_cfg(cfg, this_replica_uuid)
         M.rebalancer_fiber:cancel()
         M.rebalancer_fiber = nil
     end
+    lua_gc.set_state(M.collect_lua_garbage, consts.COLLECT_LUA_GARBAGE_INTERVAL)
     -- Destroy connections, not used in a new configuration.
     collectgarbage()
 end
-- 
2.14.1

  parent reply	other threads:[~2018-07-31 16:25 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-31 16:25 [tarantool-patches] [PATCH 0/3] multiple routers AKhatskevich
2018-07-31 16:25 ` [tarantool-patches] [PATCH 1/3] Update only vshard part of a cfg on reload AKhatskevich
2018-08-01 18:43   ` [tarantool-patches] " Vladislav Shpilevoy
2018-08-03 20:03     ` Alex Khatskevich
2018-08-06 17:03       ` Vladislav Shpilevoy
2018-08-07 13:19         ` Alex Khatskevich
2018-08-08 11:17           ` Vladislav Shpilevoy
2018-07-31 16:25 ` AKhatskevich [this message]
2018-08-01 18:43   ` [tarantool-patches] Re: [PATCH 2/3] Move lua gc to a dedicated module Vladislav Shpilevoy
2018-08-03 20:04     ` Alex Khatskevich
2018-08-06 17:03       ` Vladislav Shpilevoy
2018-08-08 11:17       ` Vladislav Shpilevoy
2018-07-31 16:25 ` [tarantool-patches] [PATCH 3/3] Introduce multiple routers feature AKhatskevich
2018-08-01 18:43   ` [tarantool-patches] " Vladislav Shpilevoy
2018-08-03 20:05     ` Alex Khatskevich
2018-08-06 17:03       ` Vladislav Shpilevoy
2018-08-07 13:18         ` Alex Khatskevich
2018-08-08 12:28           ` Vladislav Shpilevoy
2018-08-08 14:04             ` Alex Khatskevich
2018-08-08 15:37               ` Vladislav Shpilevoy
2018-08-01 14:30 ` [tarantool-patches] [PATCH] Check self arg passed for router objects AKhatskevich
2018-08-03 20:07 ` [tarantool-patches] [PATCH] Refactor config templates AKhatskevich
2018-08-06 15:49   ` [tarantool-patches] " Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae988884e373544e9ba3b39401b60033b08eb295.1533054045.git.avkhatskevich@tarantool.org \
    --to=avkhatskevich@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH 2/3] Move lua gc to a dedicated module' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox