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)