Re: [Tarantool-patches] [PATCH v2 vshard 1/2] storage: fix reload applying migration twice

[Yaroslav Dynnikov <yaroslav.dynnikov@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 3285 bytes --]

Hi! Nice patchset, LGTM.

Best regards
Yaroslav Dynnikov


On Thu, 6 Aug 2020 at 01:15, Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
wrote:

> Reload evolution is a subsystem for storage hot code reload. When
> code is reloaded, it applies migration scripts one by one. But
> there was a bug - the last migration script was applied even when
> code version didn't really change.
>
> The bug didn't affect anything, since so far the reload evolution
> was not used for anything and had just one dummy migration script
> doing nothing.
>
> Now a first migration script is going to be added, so the bug is
> fixed before that.
>
> Needed for #237
> ---
>  test/reload_evolution/storage.result   | 6 +++++-
>  test/reload_evolution/storage.test.lua | 6 +++++-
>  vshard/storage/reload_evolution.lua    | 2 +-
>  3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/test/reload_evolution/storage.result
> b/test/reload_evolution/storage.result
> index ebf4fdc..dde0a1f 100644
> --- a/test/reload_evolution/storage.result
> +++ b/test/reload_evolution/storage.result
> @@ -86,7 +86,11 @@ package.loaded['vshard.storage'] = nil
>  vshard.storage = require("vshard.storage")
>  ---
>  ...
> -test_run:grep_log('storage_2_a', 'vshard.storage.reload_evolution:
> upgraded to') ~= nil
> +-- Should be nil. Because there was a bug that reload always reapplied
> the last
> +-- migration callback. When it was fixed, the last callback wasn't called
> twice.
> +-- But since the callback was only one, now nothing is called, and
> nothing is
> +-- logged.
> +test_run:grep_log('storage_2_a', 'vshard.storage.reload_evolution:
> upgraded to') == nil
>  ---
>  - true
>  ...
> diff --git a/test/reload_evolution/storage.test.lua
> b/test/reload_evolution/storage.test.lua
> index 56c1693..7858112 100644
> --- a/test/reload_evolution/storage.test.lua
> +++ b/test/reload_evolution/storage.test.lua
> @@ -35,7 +35,11 @@ box.space.test:insert({42, bucket_id_to_move})
>  package.path = original_package_path
>  package.loaded['vshard.storage'] = nil
>  vshard.storage = require("vshard.storage")
> -test_run:grep_log('storage_2_a', 'vshard.storage.reload_evolution:
> upgraded to') ~= nil
> +-- Should be nil. Because there was a bug that reload always reapplied
> the last
> +-- migration callback. When it was fixed, the last callback wasn't called
> twice.
> +-- But since the callback was only one, now nothing is called, and
> nothing is
> +-- logged.
> +test_run:grep_log('storage_2_a', 'vshard.storage.reload_evolution:
> upgraded to') == nil
>  vshard.storage.internal.reload_version
>  -- Make sure storage operates well.
>  vshard.storage.bucket_force_drop(2000)
> diff --git a/vshard/storage/reload_evolution.lua
> b/vshard/storage/reload_evolution.lua
> index 5d09b11..1abc9e2 100644
> --- a/vshard/storage/reload_evolution.lua
> +++ b/vshard/storage/reload_evolution.lua
> @@ -33,7 +33,7 @@ local function upgrade(M)
>          log.error(err_msg)
>          error(err_msg)
>      end
> -    for i = start_version, #migrations  do
> +    for i = start_version + 1, #migrations do
>          local ok, err = pcall(migrations[i], M)
>          if ok then
>              log.info('vshard.storage.reload_evolution: upgraded to %d
> version',
> --
> 2.21.1 (Apple Git-122.3)
>
>

[-- Attachment #2: Type: text/html, Size: 4194 bytes --]