<div dir="ltr"><div>Hi! Nice patchset, LGTM.</div><div><br></div><div><div><div dir="ltr" data-smartmail="gmail_signature"><div dir="ltr"><span><div><div dir="ltr">Best regards</div><div>Yaroslav Dynnikov<br></div></div></span></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 6 Aug 2020 at 01:15, Vladislav Shpilevoy <<a href="mailto:v.shpilevoy@tarantool.org" target="_blank">v.shpilevoy@tarantool.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Reload evolution is a subsystem for storage hot code reload. When<br>
code is reloaded, it applies migration scripts one by one. But<br>
there was a bug - the last migration script was applied even when<br>
code version didn't really change.<br>
<br>
The bug didn't affect anything, since so far the reload evolution<br>
was not used for anything and had just one dummy migration script<br>
doing nothing.<br>
<br>
Now a first migration script is going to be added, so the bug is<br>
fixed before that.<br>
<br>
Needed for #237<br>
---<br>
 test/reload_evolution/storage.result   | 6 +++++-<br>
 test/reload_evolution/storage.test.lua | 6 +++++-<br>
 vshard/storage/reload_evolution.lua    | 2 +-<br>
 3 files changed, 11 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/test/reload_evolution/storage.result b/test/reload_evolution/storage.result<br>
index ebf4fdc..dde0a1f 100644<br>
--- a/test/reload_evolution/storage.result<br>
+++ b/test/reload_evolution/storage.result<br>
@@ -86,7 +86,11 @@ package.loaded['vshard.storage'] = nil<br>
 vshard.storage = require("vshard.storage")<br>
 ---<br>
 ...<br>
-test_run:grep_log('storage_2_a', 'vshard.storage.reload_evolution: upgraded to') ~= nil<br>
+-- Should be nil. Because there was a bug that reload always reapplied the last<br>
+-- migration callback. When it was fixed, the last callback wasn't called twice.<br>
+-- But since the callback was only one, now nothing is called, and nothing is<br>
+-- logged.<br>
+test_run:grep_log('storage_2_a', 'vshard.storage.reload_evolution: upgraded to') == nil<br>
 ---<br>
 - true<br>
 ...<br>
diff --git a/test/reload_evolution/storage.test.lua b/test/reload_evolution/storage.test.lua<br>
index 56c1693..7858112 100644<br>
--- a/test/reload_evolution/storage.test.lua<br>
+++ b/test/reload_evolution/storage.test.lua<br>
@@ -35,7 +35,11 @@ box.space.test:insert({42, bucket_id_to_move})<br>
 package.path = original_package_path<br>
 package.loaded['vshard.storage'] = nil<br>
 vshard.storage = require("vshard.storage")<br>
-test_run:grep_log('storage_2_a', 'vshard.storage.reload_evolution: upgraded to') ~= nil<br>
+-- Should be nil. Because there was a bug that reload always reapplied the last<br>
+-- migration callback. When it was fixed, the last callback wasn't called twice.<br>
+-- But since the callback was only one, now nothing is called, and nothing is<br>
+-- logged.<br>
+test_run:grep_log('storage_2_a', 'vshard.storage.reload_evolution: upgraded to') == nil<br>
 vshard.storage.internal.reload_version<br>
 -- Make sure storage operates well.<br>
 vshard.storage.bucket_force_drop(2000)<br>
diff --git a/vshard/storage/reload_evolution.lua b/vshard/storage/reload_evolution.lua<br>
index 5d09b11..1abc9e2 100644<br>
--- a/vshard/storage/reload_evolution.lua<br>
+++ b/vshard/storage/reload_evolution.lua<br>
@@ -33,7 +33,7 @@ local function upgrade(M)<br>
         log.error(err_msg)<br>
         error(err_msg)<br>
     end<br>
-    for i = start_version, #migrations  do<br>
+    for i = start_version + 1, #migrations do<br>
         local ok, err = pcall(migrations[i], M)<br>
         if ok then<br>
             <a href="http://log.info" rel="noreferrer" target="_blank">log.info</a>('vshard.storage.reload_evolution: upgraded to %d version',<br>
-- <br>
2.21.1 (Apple Git-122.3)<br>
<br>
</blockquote></div>