[tarantool-patches] Re: [PATCH 2/3] Complete module reload

[Vladislav Shpilevoy]

Thanks for the fixes! See 2 comments below.

On 20/07/2018 14:34, Alex Khatskevich wrote:
> here is a full diff:
> 
> commit cd3740a45322458cde10e67f8018bc4787f443aa
> Author: AKhatskevich <avkhatskevich at tarantool.org>
> Date:   Sat Jun 9 17:23:40 2018 +0300
> 
>      Complete module reload
> 
>      In case one need to upgrade vshard to a new version, this commit
>      improves reload mechanism to allow to do that for a wider variety of
>      possible changes (between two versions).
> 
>      Changes:
>       * introduce cfg option `connection_outdate_delay`
>       * improve reload mechanism
>       * add `util.async_task` method, which runs a function after a
>         delay
>       * delete replicaset:rebind_connections method as it is replaced
>         with `rebind_replicasets` which updates all replicasets at once
> 
>      Reload mechanism:
>       * reload all vshard modules
>       * create new `replicaset` and `replica` objects
>       * reuse old netbox connections in new replica objects if
>         possible
>       * update router/storage.internal table
>       * after a `connection_outdate_delay` disable old instances of
>         `replicaset` and `replica` objects
> 
>      Reload works for modules:
>       * vshard.router
>       * vshard.storage
> 
>      Here is a module reload algorithm:
>       * old vshard is working
>       * delete old vshard src
>       * install new vshard
>       * call: package.loaded['vshard.router'] = nil
>       * call: old_router = vshard.router -- Save working router copy.
>       * call: vshard.router = require('vshard.router')
>       * if require fails: continue using old_router
>       * if require succeeds: use vshard.router
> 
>      In case reload process fails, old router/storage module, replicaset and
>      replica objects continue working properly. If reload succeeds, all old
>      objects would be deprecated.
> 
>      Extra changes:
>       * introduce MODULE_INTERNALS which stores name of the module
>         internal data in the global namespace
> 
>      Part of #112
> 
> diff --git a/test/router/reload.result b/test/router/reload.result
> index 47f3c2e..71f82b2 100644
> --- a/test/router/reload.result
> +++ b/test/router/reload.result
> @@ -174,6 +174,132 @@ vshard.router.module_version()
>   check_reloaded()
>   ---
>   ...
> +--
> +-- Outdate old replicaset and replica objets.

1. Typo: objets. Please, check the spelling in a text editor
before sending the patch.

> diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua
> index 99f59aa..f6e971b 100644
> --- a/vshard/replicaset.lua
> +++ b/vshard/replicaset.lua
> @@ -21,6 +21,7 @@
>   --                                  requests to the replica>,
>   --             net_sequential_fail = <count of sequential failed
>   --                                    requests to the replica>,
> +--             outdated = nil/true,

2. In scope of the point 11 of the previous review I asked you to
rename it to 'is_outdated', since it is a flag.

>   --          }
>   --      },
>   --      master = <master server from the array above>,