[tarantool-patches] Re: [PATCH] Updata replicaser mt on package reload

[Vladislav Shpilevoy]

Thanks for the patch! Verbally we have decided to make this patch
more system. Alexey will rework the way how replica and replicaset
objects are built in replicaset.buildall method. It will allow to
reconfig and reload with no replica/replicaset objects recreation,
but with reset.

On 04/06/2018 19:27, AKhatskevich wrote:
> Update metatables of `replicaset` and `replica` objects on package
> reload in a way that updates metatables of already existing objects.
> 
> Closes: #112
> ---
> Branch: https://github.com/tarantool/vshard/tree/kh/gh-112-reload-mt
> Issue: https://github.com/tarantool/vshard/issues/112
> 
> Note: There is an idea that all objects should be recreated instead.
> 
>   test/router/reload.result   | 43 +++++++++++++++++++++++++++++++++++++++++++
>   test/router/reload.test.lua | 14 ++++++++++++++
>   vshard/replicaset.lua       | 39 +++++++++++++++++++++++++++++++++++----
>   3 files changed, 92 insertions(+), 4 deletions(-)
> 
> diff --git a/test/router/reload.result b/test/router/reload.result
> index 19a9ead..9c0a19b 100644
> --- a/test/router/reload.result
> +++ b/test/router/reload.result
> @@ -174,6 +174,49 @@ vshard.router.module_version()
>   check_reloaded()
>   ---
>   ...
> +assert(rawget(_G, '__module_vshard_replicaset') ~= nil)
> +---
> +- true
> +...
> +_, replicaset = next(vshard.router.internal.replicasets)
> +---
> +...
> +old_mt_method = getmetatable(replicaset).__index.call
> +---
> +...
> +M = require('vshard.replicaset').internal
> +---
> +...
> +M.errinj.ERRINJ_RELOAD = true
> +---
> +...
> +package.loaded["vshard.replicaset"] = nil
> +---
> +...
> +ok, msg = pcall(require, 'vshard.replicaset')
> +---
> +...
> +assert(ok == false)
> +---
> +- true
> +...
> +old_mt_method == getmetatable(replicaset).__index.call
> +---
> +- true
> +...
> +M.errinj.ERRINJ_RELOAD = false
> +---
> +...
> +package.loaded["vshard.replicaset"] = nil
> +---
> +...
> +_ = require('vshard.replicaset')
> +---
> +...
> +old_mt_method == getmetatable(replicaset).__index.call
> +---
> +- false
> +...
>   test_run:switch('default')
>   ---
>   - true
> diff --git a/test/router/reload.test.lua b/test/router/reload.test.lua
> index 6e21b74..cdb5d50 100644
> --- a/test/router/reload.test.lua
> +++ b/test/router/reload.test.lua
> @@ -86,6 +86,20 @@ _ = require('vshard.router')
>   vshard.router.module_version()
>   check_reloaded()
>   
> +assert(rawget(_G, '__module_vshard_replicaset') ~= nil)
> +_, replicaset = next(vshard.router.internal.replicasets)
> +old_mt_method = getmetatable(replicaset).__index.call
> +M = require('vshard.replicaset').internal
> +M.errinj.ERRINJ_RELOAD = true
> +package.loaded["vshard.replicaset"] = nil
> +ok, msg = pcall(require, 'vshard.replicaset')
> +assert(ok == false)
> +old_mt_method == getmetatable(replicaset).__index.call
> +M.errinj.ERRINJ_RELOAD = false
> +package.loaded["vshard.replicaset"] = nil
> +_ = require('vshard.replicaset')
> +old_mt_method == getmetatable(replicaset).__index.call
> +
>   test_run:switch('default')
>   test_run:cmd('stop server router_1')
>   test_run:cmd('cleanup server router_1')
> diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua
> index 7fcf2df..62a9b9e 100644
> --- a/vshard/replicaset.lua
> +++ b/vshard/replicaset.lua
> @@ -50,6 +50,21 @@ local luri = require('uri')
>   local ffi = require('ffi')
>   local gsc = require('vshard.util').generate_self_checker
>   
> +local M = rawget(_G, '__module_vshard_replicaset')
> +if not M then
> +    --
> +    -- The module is loaded for the first time.
> +    --
> +    M = {
> +        errinj = {
> +            ERRINJ_RELOAD = false,
> +        },
> +        -- Cache metatables of old objects to update them.
> +        replicaset_mt = {},
> +        replica_mt = {},
> +    }
> +end
> +
>   --
>   -- on_connect() trigger for net.box
>   --
> @@ -383,7 +398,7 @@ local replicaset_mt = {
>   --
>   local index = {}
>   for name, func in pairs(replicaset_mt.__index) do
> -    index[name] = gsc("replicaset", name, replicaset_mt, func)
> +    index[name] = gsc("replicaset", name, M.replicaset_mt, func)
>   end
>   replicaset_mt.__index = index
>   
> @@ -408,7 +423,7 @@ local replica_mt = {
>   }
>   index = {}
>   for name, func in pairs(replica_mt.__index) do
> -    index[name] = gsc("replica", name, replica_mt, func)
> +    index[name] = gsc("replica", name, M.replica_mt, func)
>   end
>   replica_mt.__index = index
>   
> @@ -523,7 +538,7 @@ local function buildall(sharding_cfg, old_replicasets)
>               weight = replicaset.weight,
>               bucket_count = 0,
>               lock = replicaset.lock,
> -        }, replicaset_mt)
> +        }, M.replicaset_mt)
>           local priority_list = {}
>           for replica_uuid, replica in pairs(replicaset.replicas) do
>               local old_replica = old_replicaset and
> @@ -536,7 +551,7 @@ local function buildall(sharding_cfg, old_replicasets)
>                   zone = replica.zone, net_timeout = consts.CALL_TIMEOUT_MIN,
>                   net_sequential_ok = 0, net_sequential_fail = 0,
>                   down_ts = curr_ts, old_replica = old_replica,
> -            }, replica_mt)
> +            }, M.replica_mt)
>               new_replicaset.replicas[replica_uuid] = new_replica
>               if replica.master then
>                   new_replicaset.master = new_replica
> @@ -592,8 +607,24 @@ local function wait_masters_connect(replicasets)
>       end
>   end
>   
> +if M.errinj.ERRINJ_RELOAD then
> +    error('Error injection: reload')
> +end
> +
> +--
> +-- In case the module is loaded correctly, do externally-visible changes.
> +--
> +if not rawget(_G, '__module_vshard_replicaset') then
> +    rawset(_G, '__module_vshard_replicaset', M)
> +end
> +M.replicaset_mt.__index = replicaset_mt.__index
> +M.replicaset_mt.__tostring = replicaset_mt.__tostring
> +M.replica_mt.__index = replica_mt.__index
> +M.replica_mt.__tostring = replica_mt.__tostring
> +
>   return {
>       buildall = buildall,
>       calculate_etalon_balance = cluster_calculate_etalon_balance,
> +    internal = M,
>       wait_masters_connect = wait_masters_connect,
>   }
>