Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] box: check schema version after tarantool update
@ 2020-11-15 23:54 Roman Khabibov
  2020-11-24 12:11 ` Sergey Ostanevich
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Roman Khabibov @ 2020-11-15 23:54 UTC (permalink / raw)
  To: tarantool-patches

From: Sergey Voinov <sergeiv@tarantool.org>

Check schema version (stored in box.space._schema) on start and
print a warning if it doesn't match last avaliable schema version.
It is needed bcause some users forget to call box.schema.upgrade()
after Tarantool update and get stuck with an old schema version
until they encounter some hard to debug problems.

Closes #4574
---

@ChangeLog: Print log warning if schema version is older than last available schema version (gh-4574).

Branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
Issue: https://github.com/tarantool/tarantool/issues/4574

 src/box/lua/load_cfg.lua   | 19 +++++++++++++++
 src/box/lua/upgrade.lua    | 36 ++++++++++++++++-----------
 test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
 test/box/cfg.test.lua      | 18 ++++++++++++++
 test/box/lua/cfg_test8.lua |  9 +++++++
 test/box/lua/cfg_test9.lua |  9 +++++++
 test/box/stat.result       | 16 ++++++------
 7 files changed, 135 insertions(+), 22 deletions(-)
 create mode 100644 test/box/lua/cfg_test8.lua
 create mode 100644 test/box/lua/cfg_test9.lua

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 76e2e92c2..13088fe73 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -702,6 +702,25 @@ local function load_cfg(cfg)
     box_configured = nil
 
     box_is_configured = true
+
+    -- Check if schema version matches Tarantool version
+    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
+    local version = box.space._schema:get{'version'}
+    if version ~= nil then
+        local major = version[2]
+        local minor = version[3]
+        local patch = version[4] or 0
+        local schema_version = string.format("%s.%s.%s", major, minor, patch)
+        local tarantool_version = box.info.version
+        if private.schema_needs_upgrade() then
+            -- Print the warning
+            local msg = string.format(
+                'Your schema version is %s while Tarantool %s requires a more'..
+                ' recent schema version. Please, consider using '..
+                'box.schema.upgrade().', schema_version, tarantool_version)
+            log.warn(msg)
+        end
+    end
 end
 box.cfg = locked(load_cfg)
 
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index add791cd7..5ea2fb23d 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -973,6 +973,21 @@ end
 
 --------------------------------------------------------------------------------
 
+local handlers = {
+    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
+    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
+    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
+    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
+    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
+    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
+    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
+    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
+    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
+    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
+    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
+}
+
+-- Schema version of the snapshot.
 local function get_version()
     local version = box.space._schema:get{'version'}
     if version == nil then
@@ -985,6 +1000,12 @@ local function get_version()
     return mkversion(major, minor, patch)
 end
 
+local function schema_needs_upgrade()
+    -- Schema needs upgrade if current schema version is greater
+    -- than schema version of the snapshot.
+    return handlers[#handlers].version > get_version()
+end
+
 local function upgrade(options)
     options = options or {}
     setmetatable(options, {__index = {auto = false}})
@@ -995,20 +1016,6 @@ local function upgrade(options)
         return
     end
 
-    local handlers = {
-        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
-        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
-        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
-        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
-        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
-        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
-        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
-        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
-        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
-        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
-        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
-    }
-
     for _, handler in ipairs(handlers) do
         if version >= handler.version then
             goto continue
@@ -1047,3 +1054,4 @@ end
 
 box.schema.upgrade = upgrade;
 box.internal.bootstrap = bootstrap;
+box.internal.schema_needs_upgrade = schema_needs_upgrade;
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 4ad3c6493..676324662 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
  | ---
  | - true
  | ...
+
+--
+-- gh-4574: Check schema version after Tarantool update.
+--
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester8")
+ | ---
+ | - true
+ | ...
+--- Check that the warning is printed.
+version_warning = "Please, consider using box.schema.upgrade()."
+ | ---
+ | ...
+test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester8")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester8")
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester9")
+ | ---
+ | - true
+ | ...
+--- Check that the warning isn't printed.
+test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester9")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester9")
+ | ---
+ | - true
+ | ...
diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
index 56018b1a0..015af1a91 100644
--- a/test/box/cfg.test.lua
+++ b/test/box/cfg.test.lua
@@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
 test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
 test_run:cmd("stop server cfg_tester7")
 test_run:cmd("cleanup server cfg_tester7")
+
+--
+-- gh-4574: Check schema version after Tarantool update.
+--
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
+test_run:cmd("start server cfg_tester8")
+--- Check that the warning is printed.
+version_warning = "Please, consider using box.schema.upgrade()."
+test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
+test_run:cmd("stop server cfg_tester8")
+test_run:cmd("cleanup server cfg_tester8")
+
+test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
+test_run:cmd("start server cfg_tester9")
+--- Check that the warning isn't printed.
+test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
+test_run:cmd("stop server cfg_tester9")
+test_run:cmd("cleanup server cfg_tester9")
diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
new file mode 100644
index 000000000..c61b86ae3
--- /dev/null
+++ b/test/box/lua/cfg_test8.lua
@@ -0,0 +1,9 @@
+#!/usr/bin/env tarantool
+os = require('os')
+
+box.cfg{
+    listen = os.getenv("LISTEN"),
+    read_only = true
+}
+
+require('console').listen(os.getenv('ADMIN'))
diff --git a/test/box/lua/cfg_test9.lua b/test/box/lua/cfg_test9.lua
new file mode 100644
index 000000000..fa8b303f1
--- /dev/null
+++ b/test/box/lua/cfg_test9.lua
@@ -0,0 +1,9 @@
+#!/usr/bin/env tarantool
+os = require('os')
+
+box.cfg{
+    listen = os.getenv("LISTEN"),
+    read_only = false
+}
+
+require('console').listen(os.getenv('ADMIN'))
diff --git a/test/box/stat.result b/test/box/stat.result
index 55f29fe59..e808678eb 100644
--- a/test/box/stat.result
+++ b/test/box/stat.result
@@ -24,7 +24,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 1
+- 3
 ...
 box.stat.ERROR.total
 ---
@@ -59,7 +59,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 5
+- 7
 ...
 -- check exceptions
 space:get('Impossible value')
@@ -77,14 +77,14 @@ space:get(1)
 ...
 box.stat.SELECT.total
 ---
-- 6
+- 8
 ...
 space:get(11)
 ---
 ...
 box.stat.SELECT.total
 ---
-- 7
+- 9
 ...
 space:select(5)
 ---
@@ -92,7 +92,7 @@ space:select(5)
 ...
 box.stat.SELECT.total
 ---
-- 8
+- 10
 ...
 space:select(15)
 ---
@@ -100,14 +100,14 @@ space:select(15)
 ...
 box.stat.SELECT.total
 ---
-- 9
+- 11
 ...
 for _ in space:pairs() do end
 ---
 ...
 box.stat.SELECT.total
 ---
-- 10
+- 12
 ...
 -- reset
 box.stat.reset()
@@ -157,7 +157,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 1
+- 3
 ...
 box.stat.ERROR.total
 ---
-- 
2.24.3 (Apple Git-128)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-11-15 23:54 [Tarantool-patches] [PATCH] box: check schema version after tarantool update Roman Khabibov
@ 2020-11-24 12:11 ` Sergey Ostanevich
  2020-11-24 12:21   ` Sergey Ostanevich
  2020-11-24 18:53   ` Roman Khabibov
  2020-12-03 12:40 ` Alexander V. Tikhonov
  2020-12-03 12:56 ` Kirill Yukhin
  2 siblings, 2 replies; 14+ messages in thread
From: Sergey Ostanevich @ 2020-11-24 12:11 UTC (permalink / raw)
  To: Roman Khabibov; +Cc: tarantool-patches

Hi!

Thanks for the patch!
See my 2 comments inline.

Sergos


> On 16 Nov 2020, at 02:54, Roman Khabibov <roman.habibov@tarantool.org> wrote:
> 
> From: Sergey Voinov <sergeiv@tarantool.org>
> 
> Check schema version (stored in box.space._schema) on start and
> print a warning if it doesn't match last avaliable schema version.
> It is needed bcause some users forget to call box.schema.upgrade()
> after Tarantool update and get stuck with an old schema version
> until they encounter some hard to debug problems.
> 
> Closes #4574
> ---
> 
> @ChangeLog: Print log warning if schema version is older than last available schema version (gh-4574).
> 
> Branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
> Issue: https://github.com/tarantool/tarantool/issues/4574
> 
> src/box/lua/load_cfg.lua   | 19 +++++++++++++++
> src/box/lua/upgrade.lua    | 36 ++++++++++++++++-----------
> test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
> test/box/cfg.test.lua      | 18 ++++++++++++++
> test/box/lua/cfg_test8.lua |  9 +++++++
> test/box/lua/cfg_test9.lua |  9 +++++++
> test/box/stat.result       | 16 ++++++------
> 7 files changed, 135 insertions(+), 22 deletions(-)
> create mode 100644 test/box/lua/cfg_test8.lua
> create mode 100644 test/box/lua/cfg_test9.lua
> 
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 76e2e92c2..13088fe73 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -702,6 +702,25 @@ local function load_cfg(cfg)
>     box_configured = nil
> 
>     box_is_configured = true
> +
> +    -- Check if schema version matches Tarantool version
> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())

1. The code starting from here 
> +    local version = box.space._schema:get{'version'}
> +    if version ~= nil then
> +        local major = version[2]
> +        local minor = version[3]
> +        local patch = version[4] or 0
> +        local schema_version = string.format("%s.%s.%s", major, minor, patch)
> +        local tarantool_version = box.info.version

to here doesn’t needed, as you do exactly the same inside the schema_needs_upgrade()

> +        if private.schema_needs_upgrade() then
> +            -- Print the warning
> +            local msg = string.format(

1.1 The returning value of schema_needs_upgrade() may contain the expected upgrade version,
so you can reuse it for the message itself.
> +                'Your schema version is %s while Tarantool %s requires a more'..
> +                ' recent schema version. Please, consider using '..
> +                'box.schema.upgrade().', schema_version, tarantool_version)
> +            log.warn(msg)
> +        end
> +    end
> end
> box.cfg = locked(load_cfg)
> 
> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index add791cd7..5ea2fb23d 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -973,6 +973,21 @@ end
> 
> --------------------------------------------------------------------------------
> 
> +local handlers = {
> +    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
> +    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
> +    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
> +    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
> +    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
> +    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
> +    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
> +    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
> +    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
> +    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
> +    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
> +}
> +
> +-- Schema version of the snapshot.
> local function get_version()
>     local version = box.space._schema:get{'version'}
>     if version == nil then
> @@ -985,6 +1000,12 @@ local function get_version()
>     return mkversion(major, minor, patch)
> end
> 
> +local function schema_needs_upgrade()
> +    -- Schema needs upgrade if current schema version is greater
> +    -- than schema version of the snapshot.
> +    return handlers[#handlers].version > get_version()

1.2 Update the return value here.

> +end
> +
> local function upgrade(options)
>     options = options or {}
>     setmetatable(options, {__index = {auto = false}})
> @@ -995,20 +1016,6 @@ local function upgrade(options)
>         return
>     end
> 
> -    local handlers = {
> -        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
> -        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
> -        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
> -        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
> -        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
> -        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
> -        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
> -        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
> -        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
> -        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
> -        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
> -    }
> -
>     for _, handler in ipairs(handlers) do
>         if version >= handler.version then
>             goto continue
> @@ -1047,3 +1054,4 @@ end
> 
> box.schema.upgrade = upgrade;
> box.internal.bootstrap = bootstrap;
> +box.internal.schema_needs_upgrade = schema_needs_upgrade;
> diff --git a/test/box/cfg.result b/test/box/cfg.result
> index 4ad3c6493..676324662 100644
> --- a/test/box/cfg.result
> +++ b/test/box/cfg.result
> @@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
>  | ---
>  | - true
>  | ...
> +
> +--
> +-- gh-4574: Check schema version after Tarantool update.
> +--
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd("start server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +--- Check that the warning is printed.
> +version_warning = "Please, consider using box.schema.upgrade()."
> + | ---
> + | ...
> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
> + | ---
> + | - true
> + | ...
> +test_run:cmd("stop server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +test_run:cmd("cleanup server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd("start server cfg_tester9")
> + | ---
> + | - true
> + | ...
> +--- Check that the warning isn't printed.
> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
> + | ---
> + | - true
> + | ...
> +test_run:cmd("stop server cfg_tester9")
> + | ---
> + | - true
> + | ...
> +test_run:cmd("cleanup server cfg_tester9")
> + | ---
> + | - true
> + | ...
> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
> index 56018b1a0..015af1a91 100644
> --- a/test/box/cfg.test.lua
> +++ b/test/box/cfg.test.lua
> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
> test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
> test_run:cmd("stop server cfg_tester7")
> test_run:cmd("cleanup server cfg_tester7")
> +
> +--
> +-- gh-4574: Check schema version after Tarantool update.
> +--
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')

> +test_run:cmd("start server cfg_tester8")
> +--- Check that the warning is printed.
> +version_warning = "Please, consider using box.schema.upgrade()."
> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
> +test_run:cmd("stop server cfg_tester8")
> +test_run:cmd("cleanup server cfg_tester8")
> +
> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua”’)

2. You can reuse the cfg_test1.lua for the writable node, no need in extra file.

> +test_run:cmd("start server cfg_tester9")
> +--- Check that the warning isn't printed.
> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
> +test_run:cmd("stop server cfg_tester9")
> +test_run:cmd("cleanup server cfg_tester9")
> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
> new file mode 100644
> index 000000000..c61b86ae3
> --- /dev/null
> +++ b/test/box/lua/cfg_test8.lua
> @@ -0,0 +1,9 @@
> +#!/usr/bin/env tarantool
> +os = require('os')
> +
> +box.cfg{
> +    listen = os.getenv("LISTEN"),
> +    read_only = true
> +}
> +
> +require('console').listen(os.getenv('ADMIN'))
> diff --git a/test/box/lua/cfg_test9.lua b/test/box/lua/cfg_test9.lua
> new file mode 100644
> index 000000000..fa8b303f1
> --- /dev/null
> +++ b/test/box/lua/cfg_test9.lua
> @@ -0,0 +1,9 @@
> +#!/usr/bin/env tarantool
> +os = require('os')
> +
> +box.cfg{
> +    listen = os.getenv("LISTEN"),
> +    read_only = false
> +}
> +
> +require('console').listen(os.getenv('ADMIN'))
> diff --git a/test/box/stat.result b/test/box/stat.result
> index 55f29fe59..e808678eb 100644
> --- a/test/box/stat.result
> +++ b/test/box/stat.result
> @@ -24,7 +24,7 @@ box.stat.REPLACE.total
> ...
> box.stat.SELECT.total
> ---
> -- 1
> +- 3
> ...
> box.stat.ERROR.total
> ---
> @@ -59,7 +59,7 @@ box.stat.REPLACE.total
> ...
> box.stat.SELECT.total
> ---
> -- 5
> +- 7
> ...
> -- check exceptions
> space:get('Impossible value')
> @@ -77,14 +77,14 @@ space:get(1)
> ...
> box.stat.SELECT.total
> ---
> -- 6
> +- 8
> ...
> space:get(11)
> ---
> ...
> box.stat.SELECT.total
> ---
> -- 7
> +- 9
> ...
> space:select(5)
> ---
> @@ -92,7 +92,7 @@ space:select(5)
> ...
> box.stat.SELECT.total
> ---
> -- 8
> +- 10
> ...
> space:select(15)
> ---
> @@ -100,14 +100,14 @@ space:select(15)
> ...
> box.stat.SELECT.total
> ---
> -- 9
> +- 11
> ...
> for _ in space:pairs() do end
> ---
> ...
> box.stat.SELECT.total
> ---
> -- 10
> +- 12
> ...
> -- reset
> box.stat.reset()
> @@ -157,7 +157,7 @@ box.stat.REPLACE.total
> ...
> box.stat.SELECT.total
> ---
> -- 1
> +- 3
> ...
> box.stat.ERROR.total
> ---
> -- 
> 2.24.3 (Apple Git-128)
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-11-24 12:11 ` Sergey Ostanevich
@ 2020-11-24 12:21   ` Sergey Ostanevich
  2020-11-24 18:53   ` Roman Khabibov
  1 sibling, 0 replies; 14+ messages in thread
From: Sergey Ostanevich @ 2020-11-24 12:21 UTC (permalink / raw)
  To: Roman Khabibov; +Cc: tarantool-patches

And two misprints.

> On 24 Nov 2020, at 15:11, Sergey Ostanevich <sergos@tarantool.org> wrote:
> 
> Hi!
> 
> Thanks for the patch!
> See my 2 comments inline.
> 
> Sergos
> 
> 
>> On 16 Nov 2020, at 02:54, Roman Khabibov <roman.habibov@tarantool.org> wrote:
>> 
>> From: Sergey Voinov <sergeiv@tarantool.org>
>> 
>> Check schema version (stored in box.space._schema) on start and
>> print a warning if it doesn't match last avaliable schema version.
Misprint                                    ^^^^^^^^^
>> It is needed bcause some users forget to call box.schema.upgrade()
Another one     ^^^^^^
>> after Tarantool update and get stuck with an old schema version
>> until they encounter some hard to debug problems.
>> 
>> Closes #4574
>> ---
>> 
>> @ChangeLog: Print log warning if schema version is older than last available schema version (gh-4574).
>> 
>> Branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
>> Issue: https://github.com/tarantool/tarantool/issues/4574
>> 
>> src/box/lua/load_cfg.lua   | 19 +++++++++++++++
>> src/box/lua/upgrade.lua    | 36 ++++++++++++++++-----------
>> test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
>> test/box/cfg.test.lua      | 18 ++++++++++++++
>> test/box/lua/cfg_test8.lua |  9 +++++++
>> test/box/lua/cfg_test9.lua |  9 +++++++
>> test/box/stat.result       | 16 ++++++------
>> 7 files changed, 135 insertions(+), 22 deletions(-)
>> create mode 100644 test/box/lua/cfg_test8.lua
>> create mode 100644 test/box/lua/cfg_test9.lua
>> 
>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>> index 76e2e92c2..13088fe73 100644
>> --- a/src/box/lua/load_cfg.lua
>> +++ b/src/box/lua/load_cfg.lua
>> @@ -702,6 +702,25 @@ local function load_cfg(cfg)
>>    box_configured = nil
>> 
>>    box_is_configured = true
>> +
>> +    -- Check if schema version matches Tarantool version
>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
> 
> 1. The code starting from here 
>> +    local version = box.space._schema:get{'version'}
>> +    if version ~= nil then
>> +        local major = version[2]
>> +        local minor = version[3]
>> +        local patch = version[4] or 0
>> +        local schema_version = string.format("%s.%s.%s", major, minor, patch)
>> +        local tarantool_version = box.info.version
> 
> to here doesn’t needed, as you do exactly the same inside the schema_needs_upgrade()
> 
>> +        if private.schema_needs_upgrade() then
>> +            -- Print the warning
>> +            local msg = string.format(
> 
> 1.1 The returning value of schema_needs_upgrade() may contain the expected upgrade version,
> so you can reuse it for the message itself.
>> +                'Your schema version is %s while Tarantool %s requires a more'..
>> +                ' recent schema version. Please, consider using '..
>> +                'box.schema.upgrade().', schema_version, tarantool_version)
>> +            log.warn(msg)
>> +        end
>> +    end
>> end
>> box.cfg = locked(load_cfg)
>> 
>> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>> index add791cd7..5ea2fb23d 100644
>> --- a/src/box/lua/upgrade.lua
>> +++ b/src/box/lua/upgrade.lua
>> @@ -973,6 +973,21 @@ end
>> 
>> --------------------------------------------------------------------------------
>> 
>> +local handlers = {
>> +    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>> +    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>> +    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>> +    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>> +    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>> +    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>> +    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>> +    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>> +    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>> +    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>> +    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>> +}
>> +
>> +-- Schema version of the snapshot.
>> local function get_version()
>>    local version = box.space._schema:get{'version'}
>>    if version == nil then
>> @@ -985,6 +1000,12 @@ local function get_version()
>>    return mkversion(major, minor, patch)
>> end
>> 
>> +local function schema_needs_upgrade()
>> +    -- Schema needs upgrade if current schema version is greater
>> +    -- than schema version of the snapshot.
>> +    return handlers[#handlers].version > get_version()
> 
> 1.2 Update the return value here.
> 
>> +end
>> +
>> local function upgrade(options)
>>    options = options or {}
>>    setmetatable(options, {__index = {auto = false}})
>> @@ -995,20 +1016,6 @@ local function upgrade(options)
>>        return
>>    end
>> 
>> -    local handlers = {
>> -        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>> -        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>> -        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>> -        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>> -        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>> -        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>> -        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>> -        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>> -        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>> -        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>> -        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>> -    }
>> -
>>    for _, handler in ipairs(handlers) do
>>        if version >= handler.version then
>>            goto continue
>> @@ -1047,3 +1054,4 @@ end
>> 
>> box.schema.upgrade = upgrade;
>> box.internal.bootstrap = bootstrap;
>> +box.internal.schema_needs_upgrade = schema_needs_upgrade;
>> diff --git a/test/box/cfg.result b/test/box/cfg.result
>> index 4ad3c6493..676324662 100644
>> --- a/test/box/cfg.result
>> +++ b/test/box/cfg.result
>> @@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
>> | ---
>> | - true
>> | ...
>> +
>> +--
>> +-- gh-4574: Check schema version after Tarantool update.
>> +--
>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("start server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +--- Check that the warning is printed.
>> +version_warning = "Please, consider using box.schema.upgrade()."
>> + | ---
>> + | ...
>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("stop server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("cleanup server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +
>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("start server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> +--- Check that the warning isn't printed.
>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("stop server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("cleanup server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>> index 56018b1a0..015af1a91 100644
>> --- a/test/box/cfg.test.lua
>> +++ b/test/box/cfg.test.lua
>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>> test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>> test_run:cmd("stop server cfg_tester7")
>> test_run:cmd("cleanup server cfg_tester7")
>> +
>> +--
>> +-- gh-4574: Check schema version after Tarantool update.
>> +--
>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
> 
>> +test_run:cmd("start server cfg_tester8")
>> +--- Check that the warning is printed.
>> +version_warning = "Please, consider using box.schema.upgrade()."
>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>> +test_run:cmd("stop server cfg_tester8")
>> +test_run:cmd("cleanup server cfg_tester8")
>> +
>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua”’)
> 
> 2. You can reuse the cfg_test1.lua for the writable node, no need in extra file.
> 
>> +test_run:cmd("start server cfg_tester9")
>> +--- Check that the warning isn't printed.
>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>> +test_run:cmd("stop server cfg_tester9")
>> +test_run:cmd("cleanup server cfg_tester9")
>> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
>> new file mode 100644
>> index 000000000..c61b86ae3
>> --- /dev/null
>> +++ b/test/box/lua/cfg_test8.lua
>> @@ -0,0 +1,9 @@
>> +#!/usr/bin/env tarantool
>> +os = require('os')
>> +
>> +box.cfg{
>> +    listen = os.getenv("LISTEN"),
>> +    read_only = true
>> +}
>> +
>> +require('console').listen(os.getenv('ADMIN'))
>> diff --git a/test/box/lua/cfg_test9.lua b/test/box/lua/cfg_test9.lua
>> new file mode 100644
>> index 000000000..fa8b303f1
>> --- /dev/null
>> +++ b/test/box/lua/cfg_test9.lua
>> @@ -0,0 +1,9 @@
>> +#!/usr/bin/env tarantool
>> +os = require('os')
>> +
>> +box.cfg{
>> +    listen = os.getenv("LISTEN"),
>> +    read_only = false
>> +}
>> +
>> +require('console').listen(os.getenv('ADMIN'))
>> diff --git a/test/box/stat.result b/test/box/stat.result
>> index 55f29fe59..e808678eb 100644
>> --- a/test/box/stat.result
>> +++ b/test/box/stat.result
>> @@ -24,7 +24,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 1
>> +- 3
>> ...
>> box.stat.ERROR.total
>> ---
>> @@ -59,7 +59,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 5
>> +- 7
>> ...
>> -- check exceptions
>> space:get('Impossible value')
>> @@ -77,14 +77,14 @@ space:get(1)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 6
>> +- 8
>> ...
>> space:get(11)
>> ---
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 7
>> +- 9
>> ...
>> space:select(5)
>> ---
>> @@ -92,7 +92,7 @@ space:select(5)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 8
>> +- 10
>> ...
>> space:select(15)
>> ---
>> @@ -100,14 +100,14 @@ space:select(15)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 9
>> +- 11
>> ...
>> for _ in space:pairs() do end
>> ---
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 10
>> +- 12
>> ...
>> -- reset
>> box.stat.reset()
>> @@ -157,7 +157,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 1
>> +- 3
>> ...
>> box.stat.ERROR.total
>> ---
>> -- 
>> 2.24.3 (Apple Git-128)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-11-24 12:11 ` Sergey Ostanevich
  2020-11-24 12:21   ` Sergey Ostanevich
@ 2020-11-24 18:53   ` Roman Khabibov
  2020-11-30 11:00     ` Sergey Ostanevich
  1 sibling, 1 reply; 14+ messages in thread
From: Roman Khabibov @ 2020-11-24 18:53 UTC (permalink / raw)
  To: Sergey Ostanevich; +Cc: tarantool-patches

Hi! Thanks for the review.

> On Nov 24, 2020, at 15:11, Sergey Ostanevich <sergos@tarantool.org> wrote:
> 
> Hi!
> 
> Thanks for the patch!
> See my 2 comments inline.
> 
> Sergos
> 
> 
>> On 16 Nov 2020, at 02:54, Roman Khabibov <roman.habibov@tarantool.org> wrote:
>> 
>> From: Sergey Voinov <sergeiv@tarantool.org>
>> 
>> Check schema version (stored in box.space._schema) on start and
>> print a warning if it doesn't match last avaliable schema version.
>> It is needed bcause some users forget to call box.schema.upgrade()
>> after Tarantool update and get stuck with an old schema version
>> until they encounter some hard to debug problems.
>> 
>> Closes #4574
>> ---
>> 
>> @ChangeLog: Print log warning if schema version is older than last available schema version (gh-4574).
>> 
>> Branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
>> Issue: https://github.com/tarantool/tarantool/issues/4574
>> 
>> src/box/lua/load_cfg.lua   | 19 +++++++++++++++
>> src/box/lua/upgrade.lua    | 36 ++++++++++++++++-----------
>> test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
>> test/box/cfg.test.lua      | 18 ++++++++++++++
>> test/box/lua/cfg_test8.lua |  9 +++++++
>> test/box/lua/cfg_test9.lua |  9 +++++++
>> test/box/stat.result       | 16 ++++++------
>> 7 files changed, 135 insertions(+), 22 deletions(-)
>> create mode 100644 test/box/lua/cfg_test8.lua
>> create mode 100644 test/box/lua/cfg_test9.lua
>> 
>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>> index 76e2e92c2..13088fe73 100644
>> --- a/src/box/lua/load_cfg.lua
>> +++ b/src/box/lua/load_cfg.lua
>> @@ -702,6 +702,25 @@ local function load_cfg(cfg)
>>    box_configured = nil
>> 
>>    box_is_configured = true
>> +
>> +    -- Check if schema version matches Tarantool version
>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
> 
> 1. The code starting from here

>> +    local version = box.space._schema:get{'version'}
>> +    if version ~= nil then
>> +        local major = version[2]
>> +        local minor = version[3]
>> +        local patch = version[4] or 0
>> +        local schema_version = string.format("%s.%s.%s", major, minor, patch)
>> +        local tarantool_version = box.info.version
> 
> to here doesn’t needed, as you do exactly the same inside the schema_needs_upgrade()
> 
>> +        if private.schema_needs_upgrade() then
>> +            -- Print the warning
>> +            local msg = string.format(
> 
> 1.1 The returning value of schema_needs_upgrade() may contain the expected upgrade version,
> so you can reuse it for the message itself.
>> +                'Your schema version is %s while Tarantool %s requires a more'..
>> +                ' recent schema version. Please, consider using '..
>> +                'box.schema.upgrade().', schema_version, tarantool_version)
>> +            log.warn(msg)
>> +        end
>> +    end
>> end
>> box.cfg = locked(load_cfg)
>> 
>> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>> index add791cd7..5ea2fb23d 100644
>> --- a/src/box/lua/upgrade.lua
>> +++ b/src/box/lua/upgrade.lua
>> @@ -973,6 +973,21 @@ end
>> 
>> --------------------------------------------------------------------------------
>> 
>> +local handlers = {
>> +    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>> +    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>> +    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>> +    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>> +    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>> +    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>> +    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>> +    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>> +    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>> +    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>> +    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>> +}
>> +
>> +-- Schema version of the snapshot.
>> local function get_version()
>>    local version = box.space._schema:get{'version'}
>>    if version == nil then
>> @@ -985,6 +1000,12 @@ local function get_version()
>>    return mkversion(major, minor, patch)
>> end
>> 
>> +local function schema_needs_upgrade()
>> +    -- Schema needs upgrade if current schema version is greater
>> +    -- than schema version of the snapshot.
>> +    return handlers[#handlers].version > get_version()
> 
> 1.2 Update the return value here.
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 76e2e92c2..451247dcf 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -702,6 +702,22 @@ local function load_cfg(cfg)
     box_configured = nil
 
     box_is_configured = true
+
+    -- Check if schema version matches Tarantool version
+    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
+    local version = box.space._schema:get{'version'}
+    if version ~= nil then
+        local needs, schema_version_str = private.schema_needs_upgrade()
+        local tarantool_version_str = box.info.version
+        if needs then
+            -- Print the warning
+            local msg = string.format(
+                'Your schema version is %s while Tarantool %s requires a more'..
+                ' recent schema version. Please, consider using box.'..
+                'schema.upgrade().', schema_version_str, tarantool_version_str)
+            log.warn(msg)
+        end
+    end
 end

>> +end
>> +
>> local function upgrade(options)
>>    options = options or {}
>>    setmetatable(options, {__index = {auto = false}})
>> @@ -995,20 +1016,6 @@ local function upgrade(options)
>>        return
>>    end
>> 
>> -    local handlers = {
>> -        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>> -        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>> -        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>> -        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>> -        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>> -        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>> -        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>> -        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>> -        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>> -        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>> -        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>> -    }
>> -
>>    for _, handler in ipairs(handlers) do
>>        if version >= handler.version then
>>            goto continue
>> @@ -1047,3 +1054,4 @@ end
>> 
>> box.schema.upgrade = upgrade;
>> box.internal.bootstrap = bootstrap;
>> +box.internal.schema_needs_upgrade = schema_needs_upgrade;
>> diff --git a/test/box/cfg.result b/test/box/cfg.result
>> index 4ad3c6493..676324662 100644
>> --- a/test/box/cfg.result
>> +++ b/test/box/cfg.result
>> @@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
>> | ---
>> | - true
>> | ...
>> +
>> +--
>> +-- gh-4574: Check schema version after Tarantool update.
>> +--
>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("start server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +--- Check that the warning is printed.
>> +version_warning = "Please, consider using box.schema.upgrade()."
>> + | ---
>> + | ...
>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("stop server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("cleanup server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +
>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("start server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> +--- Check that the warning isn't printed.
>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("stop server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("cleanup server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>> index 56018b1a0..015af1a91 100644
>> --- a/test/box/cfg.test.lua
>> +++ b/test/box/cfg.test.lua
>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>> test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>> test_run:cmd("stop server cfg_tester7")
>> test_run:cmd("cleanup server cfg_tester7")
>> +
>> +--
>> +-- gh-4574: Check schema version after Tarantool update.
>> +--
>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
> 
>> +test_run:cmd("start server cfg_tester8")
>> +--- Check that the warning is printed.
>> +version_warning = "Please, consider using box.schema.upgrade()."
>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>> +test_run:cmd("stop server cfg_tester8")
>> +test_run:cmd("cleanup server cfg_tester8")
>> +
>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua”’)
> 
> 2. You can reuse the cfg_test1.lua for the writable node, no need in extra file.
Done.

>> +test_run:cmd("start server cfg_tester9")
>> +--- Check that the warning isn't printed.
>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>> +test_run:cmd("stop server cfg_tester9")
>> +test_run:cmd("cleanup server cfg_tester9")
>> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
>> new file mode 100644
>> index 000000000..c61b86ae3
>> --- /dev/null
>> +++ b/test/box/lua/cfg_test8.lua
>> @@ -0,0 +1,9 @@
>> +#!/usr/bin/env tarantool
>> +os = require('os')
>> +
>> +box.cfg{
>> +    listen = os.getenv("LISTEN"),
>> +    read_only = true
>> +}
>> +
>> +require('console').listen(os.getenv('ADMIN'))
>> diff --git a/test/box/lua/cfg_test9.lua b/test/box/lua/cfg_test9.lua
>> new file mode 100644
>> index 000000000..fa8b303f1
>> --- /dev/null
>> +++ b/test/box/lua/cfg_test9.lua
>> @@ -0,0 +1,9 @@
>> +#!/usr/bin/env tarantool
>> +os = require('os')
>> +
>> +box.cfg{
>> +    listen = os.getenv("LISTEN"),
>> +    read_only = false
>> +}
>> +
>> +require('console').listen(os.getenv('ADMIN'))
>> diff --git a/test/box/stat.result b/test/box/stat.result
>> index 55f29fe59..e808678eb 100644
>> --- a/test/box/stat.result
>> +++ b/test/box/stat.result
>> @@ -24,7 +24,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 1
>> +- 3
>> ...
>> box.stat.ERROR.total
>> ---
>> @@ -59,7 +59,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 5
>> +- 7
>> ...
>> -- check exceptions
>> space:get('Impossible value')
>> @@ -77,14 +77,14 @@ space:get(1)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 6
>> +- 8
>> ...
>> space:get(11)
>> ---
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 7
>> +- 9
>> ...
>> space:select(5)
>> ---
>> @@ -92,7 +92,7 @@ space:select(5)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 8
>> +- 10
>> ...
>> space:select(15)
>> ---
>> @@ -100,14 +100,14 @@ space:select(15)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 9
>> +- 11
>> ...
>> for _ in space:pairs() do end
>> ---
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 10
>> +- 12
>> ...
>> -- reset
>> box.stat.reset()
>> @@ -157,7 +157,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 1
>> +- 3
>> ...
>> box.stat.ERROR.total
>> ---
>> -- 
>> 2.24.3 (Apple Git-128)

commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
Author: Sergey Voinov <sergeiv@tarantool.org>
Date:   Wed Dec 11 17:28:39 2019 +0300

    box: check schema version after tarantool update
    
    Check schema version (stored in box.space._schema) on start and
    print a warning if it doesn't match last available schema version.
    It is needed because some users forget to call
    box.schema.upgrade() after Tarantool update and get stuck with an
    old schema version until they encounter some hard to debug
    problems.
    
    Closes #4574

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 76e2e92c2..451247dcf 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -702,6 +702,22 @@ local function load_cfg(cfg)
     box_configured = nil
 
     box_is_configured = true
+
+    -- Check if schema version matches Tarantool version
+    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
+    local version = box.space._schema:get{'version'}
+    if version ~= nil then
+        local needs, schema_version_str = private.schema_needs_upgrade()
+        local tarantool_version_str = box.info.version
+        if needs then
+            -- Print the warning
+            local msg = string.format(
+                'Your schema version is %s while Tarantool %s requires a more'..
+                ' recent schema version. Please, consider using box.'..
+                'schema.upgrade().', schema_version_str, tarantool_version_str)
+            log.warn(msg)
+        end
+    end
 end
 box.cfg = locked(load_cfg)
 
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index add791cd7..bbb9abab8 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -973,6 +973,21 @@ end
 
 --------------------------------------------------------------------------------
 
+local handlers = {
+    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
+    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
+    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
+    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
+    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
+    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
+    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
+    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
+    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
+    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
+    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
+}
+
+-- Schema version of the snapshot.
 local function get_version()
     local version = box.space._schema:get{'version'}
     if version == nil then
@@ -982,7 +997,18 @@ local function get_version()
     local minor = version[3]
     local patch = version[4] or 0
 
-    return mkversion(major, minor, patch)
+    return mkversion(major, minor, patch),
+           string.format("%s.%s.%s", major, minor, patch)
+end
+
+local function schema_needs_upgrade()
+    -- Schema needs upgrade if current schema version is greater
+    -- than schema version of the snapshot.
+    local schema_version, schema_version_str = get_version()
+    if handlers[#handlers].version > schema_version then
+        return true, schema_version_str
+    end
+    return false
 end
 
 local function upgrade(options)
@@ -995,20 +1021,6 @@ local function upgrade(options)
         return
     end
 
-    local handlers = {
-        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
-        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
-        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
-        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
-        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
-        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
-        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
-        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
-        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
-        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
-        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
-    }
-
     for _, handler in ipairs(handlers) do
         if version >= handler.version then
             goto continue
@@ -1047,3 +1059,4 @@ end
 
 box.schema.upgrade = upgrade;
 box.internal.bootstrap = bootstrap;
+box.internal.schema_needs_upgrade = schema_needs_upgrade;
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 4ad3c6493..9326dfa0c 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
  | ---
  | - true
  | ...
+
+--
+-- gh-4574: Check schema version after Tarantool update.
+--
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester8")
+ | ---
+ | - true
+ | ...
+--- Check that the warning is printed.
+version_warning = "Please, consider using box.schema.upgrade()."
+ | ---
+ | ...
+test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester8")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester8")
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester9")
+ | ---
+ | - true
+ | ...
+--- Check that the warning isn't printed.
+test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester9")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester9")
+ | ---
+ | - true
+ | ...
diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
index 56018b1a0..e806c9efe 100644
--- a/test/box/cfg.test.lua
+++ b/test/box/cfg.test.lua
@@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
 test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
 test_run:cmd("stop server cfg_tester7")
 test_run:cmd("cleanup server cfg_tester7")
+
+--
+-- gh-4574: Check schema version after Tarantool update.
+--
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
+test_run:cmd("start server cfg_tester8")
+--- Check that the warning is printed.
+version_warning = "Please, consider using box.schema.upgrade()."
+test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
+test_run:cmd("stop server cfg_tester8")
+test_run:cmd("cleanup server cfg_tester8")
+
+test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
+test_run:cmd("start server cfg_tester9")
+--- Check that the warning isn't printed.
+test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
+test_run:cmd("stop server cfg_tester9")
+test_run:cmd("cleanup server cfg_tester9")
diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
new file mode 100644
index 000000000..c61b86ae3
--- /dev/null
+++ b/test/box/lua/cfg_test8.lua
@@ -0,0 +1,9 @@
+#!/usr/bin/env tarantool
+os = require('os')
+
+box.cfg{
+    listen = os.getenv("LISTEN"),
+    read_only = true
+}
+
+require('console').listen(os.getenv('ADMIN'))
diff --git a/test/box/stat.result b/test/box/stat.result
index 55f29fe59..e808678eb 100644
--- a/test/box/stat.result
+++ b/test/box/stat.result
@@ -24,7 +24,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 1
+- 3
 ...
 box.stat.ERROR.total
 ---
@@ -59,7 +59,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 5
+- 7
 ...
 -- check exceptions
 space:get('Impossible value')
@@ -77,14 +77,14 @@ space:get(1)
 ...
 box.stat.SELECT.total
 ---
-- 6
+- 8
 ...
 space:get(11)
 ---
 ...
 box.stat.SELECT.total
 ---
-- 7
+- 9
 ...
 space:select(5)
 ---
@@ -92,7 +92,7 @@ space:select(5)
 ...
 box.stat.SELECT.total
 ---
-- 8
+- 10
 ...
 space:select(15)
 ---
@@ -100,14 +100,14 @@ space:select(15)
 ...
 box.stat.SELECT.total
 ---
-- 9
+- 11
 ...
 for _ in space:pairs() do end
 ---
 ...
 box.stat.SELECT.total
 ---
-- 10
+- 12
 ...
 -- reset
 box.stat.reset()
@@ -157,7 +157,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 1
+- 3
 ...
 box.stat.ERROR.total
 ---

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-11-24 18:53   ` Roman Khabibov
@ 2020-11-30 11:00     ` Sergey Ostanevich
  2020-11-30 13:43       ` Roman Khabibov
  0 siblings, 1 reply; 14+ messages in thread
From: Sergey Ostanevich @ 2020-11-30 11:00 UTC (permalink / raw)
  To: Roman Khabibov; +Cc: tarantool-patches

Thanks, LGTM.

Sergos

> On 24 Nov 2020, at 21:53, Roman Khabibov <roman.habibov@tarantool.org> wrote:
> 
> Hi! Thanks for the review.
> 
>> On Nov 24, 2020, at 15:11, Sergey Ostanevich <sergos@tarantool.org> wrote:
>> 
>> Hi!
>> 
>> Thanks for the patch!
>> See my 2 comments inline.
>> 
>> Sergos
>> 
>> 
>>> On 16 Nov 2020, at 02:54, Roman Khabibov <roman.habibov@tarantool.org> wrote:
>>> 
>>> From: Sergey Voinov <sergeiv@tarantool.org>
>>> 
>>> Check schema version (stored in box.space._schema) on start and
>>> print a warning if it doesn't match last avaliable schema version.
>>> It is needed bcause some users forget to call box.schema.upgrade()
>>> after Tarantool update and get stuck with an old schema version
>>> until they encounter some hard to debug problems.
>>> 
>>> Closes #4574
>>> ---
>>> 
>>> @ChangeLog: Print log warning if schema version is older than last available schema version (gh-4574).
>>> 
>>> Branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
>>> Issue: https://github.com/tarantool/tarantool/issues/4574
>>> 
>>> src/box/lua/load_cfg.lua   | 19 +++++++++++++++
>>> src/box/lua/upgrade.lua    | 36 ++++++++++++++++-----------
>>> test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
>>> test/box/cfg.test.lua      | 18 ++++++++++++++
>>> test/box/lua/cfg_test8.lua |  9 +++++++
>>> test/box/lua/cfg_test9.lua |  9 +++++++
>>> test/box/stat.result       | 16 ++++++------
>>> 7 files changed, 135 insertions(+), 22 deletions(-)
>>> create mode 100644 test/box/lua/cfg_test8.lua
>>> create mode 100644 test/box/lua/cfg_test9.lua
>>> 
>>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>>> index 76e2e92c2..13088fe73 100644
>>> --- a/src/box/lua/load_cfg.lua
>>> +++ b/src/box/lua/load_cfg.lua
>>> @@ -702,6 +702,25 @@ local function load_cfg(cfg)
>>>   box_configured = nil
>>> 
>>>   box_is_configured = true
>>> +
>>> +    -- Check if schema version matches Tarantool version
>>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
>> 
>> 1. The code starting from here
> 
>>> +    local version = box.space._schema:get{'version'}
>>> +    if version ~= nil then
>>> +        local major = version[2]
>>> +        local minor = version[3]
>>> +        local patch = version[4] or 0
>>> +        local schema_version = string.format("%s.%s.%s", major, minor, patch)
>>> +        local tarantool_version = box.info.version
>> 
>> to here doesn’t needed, as you do exactly the same inside the schema_needs_upgrade()
>> 
>>> +        if private.schema_needs_upgrade() then
>>> +            -- Print the warning
>>> +            local msg = string.format(
>> 
>> 1.1 The returning value of schema_needs_upgrade() may contain the expected upgrade version,
>> so you can reuse it for the message itself.
>>> +                'Your schema version is %s while Tarantool %s requires a more'..
>>> +                ' recent schema version. Please, consider using '..
>>> +                'box.schema.upgrade().', schema_version, tarantool_version)
>>> +            log.warn(msg)
>>> +        end
>>> +    end
>>> end
>>> box.cfg = locked(load_cfg)
>>> 
>>> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>>> index add791cd7..5ea2fb23d 100644
>>> --- a/src/box/lua/upgrade.lua
>>> +++ b/src/box/lua/upgrade.lua
>>> @@ -973,6 +973,21 @@ end
>>> 
>>> --------------------------------------------------------------------------------
>>> 
>>> +local handlers = {
>>> +    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>>> +    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>>> +    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>>> +    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>>> +    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>>> +    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>>> +    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>>> +    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>>> +    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>>> +    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>>> +    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>>> +}
>>> +
>>> +-- Schema version of the snapshot.
>>> local function get_version()
>>>   local version = box.space._schema:get{'version'}
>>>   if version == nil then
>>> @@ -985,6 +1000,12 @@ local function get_version()
>>>   return mkversion(major, minor, patch)
>>> end
>>> 
>>> +local function schema_needs_upgrade()
>>> +    -- Schema needs upgrade if current schema version is greater
>>> +    -- than schema version of the snapshot.
>>> +    return handlers[#handlers].version > get_version()
>> 
>> 1.2 Update the return value here.
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 76e2e92c2..451247dcf 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>     box_configured = nil
> 
>     box_is_configured = true
> +
> +    -- Check if schema version matches Tarantool version
> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
> +    local version = box.space._schema:get{'version'}
> +    if version ~= nil then
> +        local needs, schema_version_str = private.schema_needs_upgrade()
> +        local tarantool_version_str = box.info.version
> +        if needs then
> +            -- Print the warning
> +            local msg = string.format(
> +                'Your schema version is %s while Tarantool %s requires a more'..
> +                ' recent schema version. Please, consider using box.'..
> +                'schema.upgrade().', schema_version_str, tarantool_version_str)
> +            log.warn(msg)
> +        end
> +    end
> end
> 
>>> +end
>>> +
>>> local function upgrade(options)
>>>   options = options or {}
>>>   setmetatable(options, {__index = {auto = false}})
>>> @@ -995,20 +1016,6 @@ local function upgrade(options)
>>>       return
>>>   end
>>> 
>>> -    local handlers = {
>>> -        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>>> -        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>>> -        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>>> -        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>>> -        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>>> -        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>>> -        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>>> -        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>>> -        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>>> -        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>>> -        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>>> -    }
>>> -
>>>   for _, handler in ipairs(handlers) do
>>>       if version >= handler.version then
>>>           goto continue
>>> @@ -1047,3 +1054,4 @@ end
>>> 
>>> box.schema.upgrade = upgrade;
>>> box.internal.bootstrap = bootstrap;
>>> +box.internal.schema_needs_upgrade = schema_needs_upgrade;
>>> diff --git a/test/box/cfg.result b/test/box/cfg.result
>>> index 4ad3c6493..676324662 100644
>>> --- a/test/box/cfg.result
>>> +++ b/test/box/cfg.result
>>> @@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
>>> | ---
>>> | - true
>>> | ...
>>> +
>>> +--
>>> +-- gh-4574: Check schema version after Tarantool update.
>>> +--
>>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>>> + | ---
>>> + | - true
>>> + | ...
>>> +test_run:cmd("start server cfg_tester8")
>>> + | ---
>>> + | - true
>>> + | ...
>>> +--- Check that the warning is printed.
>>> +version_warning = "Please, consider using box.schema.upgrade()."
>>> + | ---
>>> + | ...
>>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>>> + | ---
>>> + | - true
>>> + | ...
>>> +test_run:cmd("stop server cfg_tester8")
>>> + | ---
>>> + | - true
>>> + | ...
>>> +test_run:cmd("cleanup server cfg_tester8")
>>> + | ---
>>> + | - true
>>> + | ...
>>> +
>>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
>>> + | ---
>>> + | - true
>>> + | ...
>>> +test_run:cmd("start server cfg_tester9")
>>> + | ---
>>> + | - true
>>> + | ...
>>> +--- Check that the warning isn't printed.
>>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>>> + | ---
>>> + | - true
>>> + | ...
>>> +test_run:cmd("stop server cfg_tester9")
>>> + | ---
>>> + | - true
>>> + | ...
>>> +test_run:cmd("cleanup server cfg_tester9")
>>> + | ---
>>> + | - true
>>> + | ...
>>> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>>> index 56018b1a0..015af1a91 100644
>>> --- a/test/box/cfg.test.lua
>>> +++ b/test/box/cfg.test.lua
>>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>>> test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>>> test_run:cmd("stop server cfg_tester7")
>>> test_run:cmd("cleanup server cfg_tester7")
>>> +
>>> +--
>>> +-- gh-4574: Check schema version after Tarantool update.
>>> +--
>>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>> 
>>> +test_run:cmd("start server cfg_tester8")
>>> +--- Check that the warning is printed.
>>> +version_warning = "Please, consider using box.schema.upgrade()."
>>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>>> +test_run:cmd("stop server cfg_tester8")
>>> +test_run:cmd("cleanup server cfg_tester8")
>>> +
>>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua”’)
>> 
>> 2. You can reuse the cfg_test1.lua for the writable node, no need in extra file.
> Done.
> 
>>> +test_run:cmd("start server cfg_tester9")
>>> +--- Check that the warning isn't printed.
>>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>>> +test_run:cmd("stop server cfg_tester9")
>>> +test_run:cmd("cleanup server cfg_tester9")
>>> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
>>> new file mode 100644
>>> index 000000000..c61b86ae3
>>> --- /dev/null
>>> +++ b/test/box/lua/cfg_test8.lua
>>> @@ -0,0 +1,9 @@
>>> +#!/usr/bin/env tarantool
>>> +os = require('os')
>>> +
>>> +box.cfg{
>>> +    listen = os.getenv("LISTEN"),
>>> +    read_only = true
>>> +}
>>> +
>>> +require('console').listen(os.getenv('ADMIN'))
>>> diff --git a/test/box/lua/cfg_test9.lua b/test/box/lua/cfg_test9.lua
>>> new file mode 100644
>>> index 000000000..fa8b303f1
>>> --- /dev/null
>>> +++ b/test/box/lua/cfg_test9.lua
>>> @@ -0,0 +1,9 @@
>>> +#!/usr/bin/env tarantool
>>> +os = require('os')
>>> +
>>> +box.cfg{
>>> +    listen = os.getenv("LISTEN"),
>>> +    read_only = false
>>> +}
>>> +
>>> +require('console').listen(os.getenv('ADMIN'))
>>> diff --git a/test/box/stat.result b/test/box/stat.result
>>> index 55f29fe59..e808678eb 100644
>>> --- a/test/box/stat.result
>>> +++ b/test/box/stat.result
>>> @@ -24,7 +24,7 @@ box.stat.REPLACE.total
>>> ...
>>> box.stat.SELECT.total
>>> ---
>>> -- 1
>>> +- 3
>>> ...
>>> box.stat.ERROR.total
>>> ---
>>> @@ -59,7 +59,7 @@ box.stat.REPLACE.total
>>> ...
>>> box.stat.SELECT.total
>>> ---
>>> -- 5
>>> +- 7
>>> ...
>>> -- check exceptions
>>> space:get('Impossible value')
>>> @@ -77,14 +77,14 @@ space:get(1)
>>> ...
>>> box.stat.SELECT.total
>>> ---
>>> -- 6
>>> +- 8
>>> ...
>>> space:get(11)
>>> ---
>>> ...
>>> box.stat.SELECT.total
>>> ---
>>> -- 7
>>> +- 9
>>> ...
>>> space:select(5)
>>> ---
>>> @@ -92,7 +92,7 @@ space:select(5)
>>> ...
>>> box.stat.SELECT.total
>>> ---
>>> -- 8
>>> +- 10
>>> ...
>>> space:select(15)
>>> ---
>>> @@ -100,14 +100,14 @@ space:select(15)
>>> ...
>>> box.stat.SELECT.total
>>> ---
>>> -- 9
>>> +- 11
>>> ...
>>> for _ in space:pairs() do end
>>> ---
>>> ...
>>> box.stat.SELECT.total
>>> ---
>>> -- 10
>>> +- 12
>>> ...
>>> -- reset
>>> box.stat.reset()
>>> @@ -157,7 +157,7 @@ box.stat.REPLACE.total
>>> ...
>>> box.stat.SELECT.total
>>> ---
>>> -- 1
>>> +- 3
>>> ...
>>> box.stat.ERROR.total
>>> ---
>>> -- 
>>> 2.24.3 (Apple Git-128)
> 
> commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
> Author: Sergey Voinov <sergeiv@tarantool.org>
> Date:   Wed Dec 11 17:28:39 2019 +0300
> 
>    box: check schema version after tarantool update
> 
>    Check schema version (stored in box.space._schema) on start and
>    print a warning if it doesn't match last available schema version.
>    It is needed because some users forget to call
>    box.schema.upgrade() after Tarantool update and get stuck with an
>    old schema version until they encounter some hard to debug
>    problems.
> 
>    Closes #4574
> 
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 76e2e92c2..451247dcf 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>     box_configured = nil
> 
>     box_is_configured = true
> +
> +    -- Check if schema version matches Tarantool version
> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
> +    local version = box.space._schema:get{'version'}
> +    if version ~= nil then
> +        local needs, schema_version_str = private.schema_needs_upgrade()
> +        local tarantool_version_str = box.info.version
> +        if needs then
> +            -- Print the warning
> +            local msg = string.format(
> +                'Your schema version is %s while Tarantool %s requires a more'..
> +                ' recent schema version. Please, consider using box.'..
> +                'schema.upgrade().', schema_version_str, tarantool_version_str)
> +            log.warn(msg)
> +        end
> +    end
> end
> box.cfg = locked(load_cfg)
> 
> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index add791cd7..bbb9abab8 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -973,6 +973,21 @@ end
> 
> --------------------------------------------------------------------------------
> 
> +local handlers = {
> +    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
> +    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
> +    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
> +    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
> +    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
> +    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
> +    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
> +    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
> +    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
> +    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
> +    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
> +}
> +
> +-- Schema version of the snapshot.
> local function get_version()
>     local version = box.space._schema:get{'version'}
>     if version == nil then
> @@ -982,7 +997,18 @@ local function get_version()
>     local minor = version[3]
>     local patch = version[4] or 0
> 
> -    return mkversion(major, minor, patch)
> +    return mkversion(major, minor, patch),
> +           string.format("%s.%s.%s", major, minor, patch)
> +end
> +
> +local function schema_needs_upgrade()
> +    -- Schema needs upgrade if current schema version is greater
> +    -- than schema version of the snapshot.
> +    local schema_version, schema_version_str = get_version()
> +    if handlers[#handlers].version > schema_version then
> +        return true, schema_version_str
> +    end
> +    return false
> end
> 
> local function upgrade(options)
> @@ -995,20 +1021,6 @@ local function upgrade(options)
>         return
>     end
> 
> -    local handlers = {
> -        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
> -        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
> -        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
> -        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
> -        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
> -        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
> -        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
> -        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
> -        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
> -        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
> -        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
> -    }
> -
>     for _, handler in ipairs(handlers) do
>         if version >= handler.version then
>             goto continue
> @@ -1047,3 +1059,4 @@ end
> 
> box.schema.upgrade = upgrade;
> box.internal.bootstrap = bootstrap;
> +box.internal.schema_needs_upgrade = schema_needs_upgrade;
> diff --git a/test/box/cfg.result b/test/box/cfg.result
> index 4ad3c6493..9326dfa0c 100644
> --- a/test/box/cfg.result
> +++ b/test/box/cfg.result
> @@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
>  | ---
>  | - true
>  | ...
> +
> +--
> +-- gh-4574: Check schema version after Tarantool update.
> +--
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd("start server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +--- Check that the warning is printed.
> +version_warning = "Please, consider using box.schema.upgrade()."
> + | ---
> + | ...
> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
> + | ---
> + | - true
> + | ...
> +test_run:cmd("stop server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +test_run:cmd("cleanup server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd("start server cfg_tester9")
> + | ---
> + | - true
> + | ...
> +--- Check that the warning isn't printed.
> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
> + | ---
> + | - true
> + | ...
> +test_run:cmd("stop server cfg_tester9")
> + | ---
> + | - true
> + | ...
> +test_run:cmd("cleanup server cfg_tester9")
> + | ---
> + | - true
> + | ...
> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
> index 56018b1a0..e806c9efe 100644
> --- a/test/box/cfg.test.lua
> +++ b/test/box/cfg.test.lua
> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
> test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
> test_run:cmd("stop server cfg_tester7")
> test_run:cmd("cleanup server cfg_tester7")
> +
> +--
> +-- gh-4574: Check schema version after Tarantool update.
> +--
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
> +test_run:cmd("start server cfg_tester8")
> +--- Check that the warning is printed.
> +version_warning = "Please, consider using box.schema.upgrade()."
> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
> +test_run:cmd("stop server cfg_tester8")
> +test_run:cmd("cleanup server cfg_tester8")
> +
> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
> +test_run:cmd("start server cfg_tester9")
> +--- Check that the warning isn't printed.
> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
> +test_run:cmd("stop server cfg_tester9")
> +test_run:cmd("cleanup server cfg_tester9")
> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
> new file mode 100644
> index 000000000..c61b86ae3
> --- /dev/null
> +++ b/test/box/lua/cfg_test8.lua
> @@ -0,0 +1,9 @@
> +#!/usr/bin/env tarantool
> +os = require('os')
> +
> +box.cfg{
> +    listen = os.getenv("LISTEN"),
> +    read_only = true
> +}
> +
> +require('console').listen(os.getenv('ADMIN'))
> diff --git a/test/box/stat.result b/test/box/stat.result
> index 55f29fe59..e808678eb 100644
> --- a/test/box/stat.result
> +++ b/test/box/stat.result
> @@ -24,7 +24,7 @@ box.stat.REPLACE.total
> ...
> box.stat.SELECT.total
> ---
> -- 1
> +- 3
> ...
> box.stat.ERROR.total
> ---
> @@ -59,7 +59,7 @@ box.stat.REPLACE.total
> ...
> box.stat.SELECT.total
> ---
> -- 5
> +- 7
> ...
> -- check exceptions
> space:get('Impossible value')
> @@ -77,14 +77,14 @@ space:get(1)
> ...
> box.stat.SELECT.total
> ---
> -- 6
> +- 8
> ...
> space:get(11)
> ---
> ...
> box.stat.SELECT.total
> ---
> -- 7
> +- 9
> ...
> space:select(5)
> ---
> @@ -92,7 +92,7 @@ space:select(5)
> ...
> box.stat.SELECT.total
> ---
> -- 8
> +- 10
> ...
> space:select(15)
> ---
> @@ -100,14 +100,14 @@ space:select(15)
> ...
> box.stat.SELECT.total
> ---
> -- 9
> +- 11
> ...
> for _ in space:pairs() do end
> ---
> ...
> box.stat.SELECT.total
> ---
> -- 10
> +- 12
> ...
> -- reset
> box.stat.reset()
> @@ -157,7 +157,7 @@ box.stat.REPLACE.total
> ...
> box.stat.SELECT.total
> ---
> -- 1
> +- 3
> ...
> box.stat.ERROR.total
> ---

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-11-30 11:00     ` Sergey Ostanevich
@ 2020-11-30 13:43       ` Roman Khabibov
  2020-12-01  9:58         ` Serge Petrenko
  0 siblings, 1 reply; 14+ messages in thread
From: Roman Khabibov @ 2020-11-30 13:43 UTC (permalink / raw)
  To: sergepetrenko; +Cc: tarantool-patches

Thanks.

Serge, could you, please, look through the patch?

> On Nov 30, 2020, at 14:00, Sergey Ostanevich <sergos@tarantool.org> wrote:
> 
> Thanks, LGTM.
> 
> Sergos
> 
>> On 24 Nov 2020, at 21:53, Roman Khabibov <roman.habibov@tarantool.org> wrote:
>> 
>> Hi! Thanks for the review.
>> 
>>> On Nov 24, 2020, at 15:11, Sergey Ostanevich <sergos@tarantool.org> wrote:
>>> 
>>> Hi!
>>> 
>>> Thanks for the patch!
>>> See my 2 comments inline.
>>> 
>>> Sergos
>>> 
>>> 
>>>> On 16 Nov 2020, at 02:54, Roman Khabibov <roman.habibov@tarantool.org> wrote:
>>>> 
>>>> From: Sergey Voinov <sergeiv@tarantool.org>
>>>> 
>>>> Check schema version (stored in box.space._schema) on start and
>>>> print a warning if it doesn't match last avaliable schema version.
>>>> It is needed bcause some users forget to call box.schema.upgrade()
>>>> after Tarantool update and get stuck with an old schema version
>>>> until they encounter some hard to debug problems.
>>>> 
>>>> Closes #4574
>>>> ---
>>>> 
>>>> @ChangeLog: Print log warning if schema version is older than last available schema version (gh-4574).
>>>> 
>>>> Branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
>>>> Issue: https://github.com/tarantool/tarantool/issues/4574
>>>> 
>>>> src/box/lua/load_cfg.lua   | 19 +++++++++++++++
>>>> src/box/lua/upgrade.lua    | 36 ++++++++++++++++-----------
>>>> test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
>>>> test/box/cfg.test.lua      | 18 ++++++++++++++
>>>> test/box/lua/cfg_test8.lua |  9 +++++++
>>>> test/box/lua/cfg_test9.lua |  9 +++++++
>>>> test/box/stat.result       | 16 ++++++------
>>>> 7 files changed, 135 insertions(+), 22 deletions(-)
>>>> create mode 100644 test/box/lua/cfg_test8.lua
>>>> create mode 100644 test/box/lua/cfg_test9.lua
>>>> 
>>>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>>>> index 76e2e92c2..13088fe73 100644
>>>> --- a/src/box/lua/load_cfg.lua
>>>> +++ b/src/box/lua/load_cfg.lua
>>>> @@ -702,6 +702,25 @@ local function load_cfg(cfg)
>>>>  box_configured = nil
>>>> 
>>>>  box_is_configured = true
>>>> +
>>>> +    -- Check if schema version matches Tarantool version
>>>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
>>> 
>>> 1. The code starting from here
>> 
>>>> +    local version = box.space._schema:get{'version'}
>>>> +    if version ~= nil then
>>>> +        local major = version[2]
>>>> +        local minor = version[3]
>>>> +        local patch = version[4] or 0
>>>> +        local schema_version = string.format("%s.%s.%s", major, minor, patch)
>>>> +        local tarantool_version = box.info.version
>>> 
>>> to here doesn’t needed, as you do exactly the same inside the schema_needs_upgrade()
>>> 
>>>> +        if private.schema_needs_upgrade() then
>>>> +            -- Print the warning
>>>> +            local msg = string.format(
>>> 
>>> 1.1 The returning value of schema_needs_upgrade() may contain the expected upgrade version,
>>> so you can reuse it for the message itself.
>>>> +                'Your schema version is %s while Tarantool %s requires a more'..
>>>> +                ' recent schema version. Please, consider using '..
>>>> +                'box.schema.upgrade().', schema_version, tarantool_version)
>>>> +            log.warn(msg)
>>>> +        end
>>>> +    end
>>>> end
>>>> box.cfg = locked(load_cfg)
>>>> 
>>>> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>>>> index add791cd7..5ea2fb23d 100644
>>>> --- a/src/box/lua/upgrade.lua
>>>> +++ b/src/box/lua/upgrade.lua
>>>> @@ -973,6 +973,21 @@ end
>>>> 
>>>> --------------------------------------------------------------------------------
>>>> 
>>>> +local handlers = {
>>>> +    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>>>> +    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>>>> +    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>>>> +    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>>>> +    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>>>> +    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>>>> +    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>>>> +    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>>>> +    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>>>> +    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>>>> +    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>>>> +}
>>>> +
>>>> +-- Schema version of the snapshot.
>>>> local function get_version()
>>>>  local version = box.space._schema:get{'version'}
>>>>  if version == nil then
>>>> @@ -985,6 +1000,12 @@ local function get_version()
>>>>  return mkversion(major, minor, patch)
>>>> end
>>>> 
>>>> +local function schema_needs_upgrade()
>>>> +    -- Schema needs upgrade if current schema version is greater
>>>> +    -- than schema version of the snapshot.
>>>> +    return handlers[#handlers].version > get_version()
>>> 
>>> 1.2 Update the return value here.
>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>> index 76e2e92c2..451247dcf 100644
>> --- a/src/box/lua/load_cfg.lua
>> +++ b/src/box/lua/load_cfg.lua
>> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>>    box_configured = nil
>> 
>>    box_is_configured = true
>> +
>> +    -- Check if schema version matches Tarantool version
>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
>> +    local version = box.space._schema:get{'version'}
>> +    if version ~= nil then
>> +        local needs, schema_version_str = private.schema_needs_upgrade()
>> +        local tarantool_version_str = box.info.version
>> +        if needs then
>> +            -- Print the warning
>> +            local msg = string.format(
>> +                'Your schema version is %s while Tarantool %s requires a more'..
>> +                ' recent schema version. Please, consider using box.'..
>> +                'schema.upgrade().', schema_version_str, tarantool_version_str)
>> +            log.warn(msg)
>> +        end
>> +    end
>> end
>> 
>>>> +end
>>>> +
>>>> local function upgrade(options)
>>>>  options = options or {}
>>>>  setmetatable(options, {__index = {auto = false}})
>>>> @@ -995,20 +1016,6 @@ local function upgrade(options)
>>>>      return
>>>>  end
>>>> 
>>>> -    local handlers = {
>>>> -        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>>>> -        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>>>> -        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>>>> -        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>>>> -        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>>>> -        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>>>> -        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>>>> -        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>>>> -        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>>>> -        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>>>> -        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>>>> -    }
>>>> -
>>>>  for _, handler in ipairs(handlers) do
>>>>      if version >= handler.version then
>>>>          goto continue
>>>> @@ -1047,3 +1054,4 @@ end
>>>> 
>>>> box.schema.upgrade = upgrade;
>>>> box.internal.bootstrap = bootstrap;
>>>> +box.internal.schema_needs_upgrade = schema_needs_upgrade;
>>>> diff --git a/test/box/cfg.result b/test/box/cfg.result
>>>> index 4ad3c6493..676324662 100644
>>>> --- a/test/box/cfg.result
>>>> +++ b/test/box/cfg.result
>>>> @@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
>>>> | ---
>>>> | - true
>>>> | ...
>>>> +
>>>> +--
>>>> +-- gh-4574: Check schema version after Tarantool update.
>>>> +--
>>>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +test_run:cmd("start server cfg_tester8")
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +--- Check that the warning is printed.
>>>> +version_warning = "Please, consider using box.schema.upgrade()."
>>>> + | ---
>>>> + | ...
>>>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +test_run:cmd("stop server cfg_tester8")
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +test_run:cmd("cleanup server cfg_tester8")
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +
>>>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +test_run:cmd("start server cfg_tester9")
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +--- Check that the warning isn't printed.
>>>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +test_run:cmd("stop server cfg_tester9")
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> +test_run:cmd("cleanup server cfg_tester9")
>>>> + | ---
>>>> + | - true
>>>> + | ...
>>>> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>>>> index 56018b1a0..015af1a91 100644
>>>> --- a/test/box/cfg.test.lua
>>>> +++ b/test/box/cfg.test.lua
>>>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>>>> test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>>>> test_run:cmd("stop server cfg_tester7")
>>>> test_run:cmd("cleanup server cfg_tester7")
>>>> +
>>>> +--
>>>> +-- gh-4574: Check schema version after Tarantool update.
>>>> +--
>>>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>>> 
>>>> +test_run:cmd("start server cfg_tester8")
>>>> +--- Check that the warning is printed.
>>>> +version_warning = "Please, consider using box.schema.upgrade()."
>>>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>>>> +test_run:cmd("stop server cfg_tester8")
>>>> +test_run:cmd("cleanup server cfg_tester8")
>>>> +
>>>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua”’)
>>> 
>>> 2. You can reuse the cfg_test1.lua for the writable node, no need in extra file.
>> Done.
>> 
>>>> +test_run:cmd("start server cfg_tester9")
>>>> +--- Check that the warning isn't printed.
>>>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>>>> +test_run:cmd("stop server cfg_tester9")
>>>> +test_run:cmd("cleanup server cfg_tester9")
>>>> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
>>>> new file mode 100644
>>>> index 000000000..c61b86ae3
>>>> --- /dev/null
>>>> +++ b/test/box/lua/cfg_test8.lua
>>>> @@ -0,0 +1,9 @@
>>>> +#!/usr/bin/env tarantool
>>>> +os = require('os')
>>>> +
>>>> +box.cfg{
>>>> +    listen = os.getenv("LISTEN"),
>>>> +    read_only = true
>>>> +}
>>>> +
>>>> +require('console').listen(os.getenv('ADMIN'))
>>>> diff --git a/test/box/lua/cfg_test9.lua b/test/box/lua/cfg_test9.lua
>>>> new file mode 100644
>>>> index 000000000..fa8b303f1
>>>> --- /dev/null
>>>> +++ b/test/box/lua/cfg_test9.lua
>>>> @@ -0,0 +1,9 @@
>>>> +#!/usr/bin/env tarantool
>>>> +os = require('os')
>>>> +
>>>> +box.cfg{
>>>> +    listen = os.getenv("LISTEN"),
>>>> +    read_only = false
>>>> +}
>>>> +
>>>> +require('console').listen(os.getenv('ADMIN'))
>>>> diff --git a/test/box/stat.result b/test/box/stat.result
>>>> index 55f29fe59..e808678eb 100644
>>>> --- a/test/box/stat.result
>>>> +++ b/test/box/stat.result
>>>> @@ -24,7 +24,7 @@ box.stat.REPLACE.total
>>>> ...
>>>> box.stat.SELECT.total
>>>> ---
>>>> -- 1
>>>> +- 3
>>>> ...
>>>> box.stat.ERROR.total
>>>> ---
>>>> @@ -59,7 +59,7 @@ box.stat.REPLACE.total
>>>> ...
>>>> box.stat.SELECT.total
>>>> ---
>>>> -- 5
>>>> +- 7
>>>> ...
>>>> -- check exceptions
>>>> space:get('Impossible value')
>>>> @@ -77,14 +77,14 @@ space:get(1)
>>>> ...
>>>> box.stat.SELECT.total
>>>> ---
>>>> -- 6
>>>> +- 8
>>>> ...
>>>> space:get(11)
>>>> ---
>>>> ...
>>>> box.stat.SELECT.total
>>>> ---
>>>> -- 7
>>>> +- 9
>>>> ...
>>>> space:select(5)
>>>> ---
>>>> @@ -92,7 +92,7 @@ space:select(5)
>>>> ...
>>>> box.stat.SELECT.total
>>>> ---
>>>> -- 8
>>>> +- 10
>>>> ...
>>>> space:select(15)
>>>> ---
>>>> @@ -100,14 +100,14 @@ space:select(15)
>>>> ...
>>>> box.stat.SELECT.total
>>>> ---
>>>> -- 9
>>>> +- 11
>>>> ...
>>>> for _ in space:pairs() do end
>>>> ---
>>>> ...
>>>> box.stat.SELECT.total
>>>> ---
>>>> -- 10
>>>> +- 12
>>>> ...
>>>> -- reset
>>>> box.stat.reset()
>>>> @@ -157,7 +157,7 @@ box.stat.REPLACE.total
>>>> ...
>>>> box.stat.SELECT.total
>>>> ---
>>>> -- 1
>>>> +- 3
>>>> ...
>>>> box.stat.ERROR.total
>>>> ---
>>>> -- 
>>>> 2.24.3 (Apple Git-128)
>> 
>> commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
>> Author: Sergey Voinov <sergeiv@tarantool.org>
>> Date:   Wed Dec 11 17:28:39 2019 +0300
>> 
>>   box: check schema version after tarantool update
>> 
>>   Check schema version (stored in box.space._schema) on start and
>>   print a warning if it doesn't match last available schema version.
>>   It is needed because some users forget to call
>>   box.schema.upgrade() after Tarantool update and get stuck with an
>>   old schema version until they encounter some hard to debug
>>   problems.
>> 
>>   Closes #4574
>> 
>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>> index 76e2e92c2..451247dcf 100644
>> --- a/src/box/lua/load_cfg.lua
>> +++ b/src/box/lua/load_cfg.lua
>> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>>    box_configured = nil
>> 
>>    box_is_configured = true
>> +
>> +    -- Check if schema version matches Tarantool version
>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
>> +    local version = box.space._schema:get{'version'}
>> +    if version ~= nil then
>> +        local needs, schema_version_str = private.schema_needs_upgrade()
>> +        local tarantool_version_str = box.info.version
>> +        if needs then
>> +            -- Print the warning
>> +            local msg = string.format(
>> +                'Your schema version is %s while Tarantool %s requires a more'..
>> +                ' recent schema version. Please, consider using box.'..
>> +                'schema.upgrade().', schema_version_str, tarantool_version_str)
>> +            log.warn(msg)
>> +        end
>> +    end
>> end
>> box.cfg = locked(load_cfg)
>> 
>> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>> index add791cd7..bbb9abab8 100644
>> --- a/src/box/lua/upgrade.lua
>> +++ b/src/box/lua/upgrade.lua
>> @@ -973,6 +973,21 @@ end
>> 
>> --------------------------------------------------------------------------------
>> 
>> +local handlers = {
>> +    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>> +    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>> +    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>> +    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>> +    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>> +    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>> +    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>> +    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>> +    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>> +    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>> +    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>> +}
>> +
>> +-- Schema version of the snapshot.
>> local function get_version()
>>    local version = box.space._schema:get{'version'}
>>    if version == nil then
>> @@ -982,7 +997,18 @@ local function get_version()
>>    local minor = version[3]
>>    local patch = version[4] or 0
>> 
>> -    return mkversion(major, minor, patch)
>> +    return mkversion(major, minor, patch),
>> +           string.format("%s.%s.%s", major, minor, patch)
>> +end
>> +
>> +local function schema_needs_upgrade()
>> +    -- Schema needs upgrade if current schema version is greater
>> +    -- than schema version of the snapshot.
>> +    local schema_version, schema_version_str = get_version()
>> +    if handlers[#handlers].version > schema_version then
>> +        return true, schema_version_str
>> +    end
>> +    return false
>> end
>> 
>> local function upgrade(options)
>> @@ -995,20 +1021,6 @@ local function upgrade(options)
>>        return
>>    end
>> 
>> -    local handlers = {
>> -        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>> -        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>> -        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>> -        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>> -        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
>> -        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
>> -        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
>> -        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
>> -        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
>> -        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
>> -        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
>> -    }
>> -
>>    for _, handler in ipairs(handlers) do
>>        if version >= handler.version then
>>            goto continue
>> @@ -1047,3 +1059,4 @@ end
>> 
>> box.schema.upgrade = upgrade;
>> box.internal.bootstrap = bootstrap;
>> +box.internal.schema_needs_upgrade = schema_needs_upgrade;
>> diff --git a/test/box/cfg.result b/test/box/cfg.result
>> index 4ad3c6493..9326dfa0c 100644
>> --- a/test/box/cfg.result
>> +++ b/test/box/cfg.result
>> @@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
>> | ---
>> | - true
>> | ...
>> +
>> +--
>> +-- gh-4574: Check schema version after Tarantool update.
>> +--
>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("start server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +--- Check that the warning is printed.
>> +version_warning = "Please, consider using box.schema.upgrade()."
>> + | ---
>> + | ...
>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("stop server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("cleanup server cfg_tester8")
>> + | ---
>> + | - true
>> + | ...
>> +
>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("start server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> +--- Check that the warning isn't printed.
>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("stop server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd("cleanup server cfg_tester9")
>> + | ---
>> + | - true
>> + | ...
>> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>> index 56018b1a0..e806c9efe 100644
>> --- a/test/box/cfg.test.lua
>> +++ b/test/box/cfg.test.lua
>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>> test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>> test_run:cmd("stop server cfg_tester7")
>> test_run:cmd("cleanup server cfg_tester7")
>> +
>> +--
>> +-- gh-4574: Check schema version after Tarantool update.
>> +--
>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>> +test_run:cmd("start server cfg_tester8")
>> +--- Check that the warning is printed.
>> +version_warning = "Please, consider using box.schema.upgrade()."
>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>> +test_run:cmd("stop server cfg_tester8")
>> +test_run:cmd("cleanup server cfg_tester8")
>> +
>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
>> +test_run:cmd("start server cfg_tester9")
>> +--- Check that the warning isn't printed.
>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>> +test_run:cmd("stop server cfg_tester9")
>> +test_run:cmd("cleanup server cfg_tester9")
>> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
>> new file mode 100644
>> index 000000000..c61b86ae3
>> --- /dev/null
>> +++ b/test/box/lua/cfg_test8.lua
>> @@ -0,0 +1,9 @@
>> +#!/usr/bin/env tarantool
>> +os = require('os')
>> +
>> +box.cfg{
>> +    listen = os.getenv("LISTEN"),
>> +    read_only = true
>> +}
>> +
>> +require('console').listen(os.getenv('ADMIN'))
>> diff --git a/test/box/stat.result b/test/box/stat.result
>> index 55f29fe59..e808678eb 100644
>> --- a/test/box/stat.result
>> +++ b/test/box/stat.result
>> @@ -24,7 +24,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 1
>> +- 3
>> ...
>> box.stat.ERROR.total
>> ---
>> @@ -59,7 +59,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 5
>> +- 7
>> ...
>> -- check exceptions
>> space:get('Impossible value')
>> @@ -77,14 +77,14 @@ space:get(1)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 6
>> +- 8
>> ...
>> space:get(11)
>> ---
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 7
>> +- 9
>> ...
>> space:select(5)
>> ---
>> @@ -92,7 +92,7 @@ space:select(5)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 8
>> +- 10
>> ...
>> space:select(15)
>> ---
>> @@ -100,14 +100,14 @@ space:select(15)
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 9
>> +- 11
>> ...
>> for _ in space:pairs() do end
>> ---
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 10
>> +- 12
>> ...
>> -- reset
>> box.stat.reset()
>> @@ -157,7 +157,7 @@ box.stat.REPLACE.total
>> ...
>> box.stat.SELECT.total
>> ---
>> -- 1
>> +- 3
>> ...
>> box.stat.ERROR.total
>> —
> 

commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
Author: Sergey Voinov <sergeiv@tarantool.org>
Date:   Wed Dec 11 17:28:39 2019 +0300

    box: check schema version after tarantool update
    
    Check schema version (stored in box.space._schema) on start and
    print a warning if it doesn't match last available schema version.
    It is needed because some users forget to call
    box.schema.upgrade() after Tarantool update and get stuck with an
    old schema version until they encounter some hard to debug
    problems.
    
    Closes #4574

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 76e2e92c2..451247dcf 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -702,6 +702,22 @@ local function load_cfg(cfg)
     box_configured = nil
 
     box_is_configured = true
+
+    -- Check if schema version matches Tarantool version
+    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
+    local version = box.space._schema:get{'version'}
+    if version ~= nil then
+        local needs, schema_version_str = private.schema_needs_upgrade()
+        local tarantool_version_str = box.info.version
+        if needs then
+            -- Print the warning
+            local msg = string.format(
+                'Your schema version is %s while Tarantool %s requires a more'..
+                ' recent schema version. Please, consider using box.'..
+                'schema.upgrade().', schema_version_str, tarantool_version_str)
+            log.warn(msg)
+        end
+    end
 end
 box.cfg = locked(load_cfg)
 
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index add791cd7..bbb9abab8 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -973,6 +973,21 @@ end
 
 --------------------------------------------------------------------------------
 
+local handlers = {
+    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
+    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
+    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
+    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
+    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
+    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
+    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
+    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
+    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
+    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
+    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
+}
+
+-- Schema version of the snapshot.
 local function get_version()
     local version = box.space._schema:get{'version'}
     if version == nil then
@@ -982,7 +997,18 @@ local function get_version()
     local minor = version[3]
     local patch = version[4] or 0
 
-    return mkversion(major, minor, patch)
+    return mkversion(major, minor, patch),
+           string.format("%s.%s.%s", major, minor, patch)
+end
+
+local function schema_needs_upgrade()
+    -- Schema needs upgrade if current schema version is greater
+    -- than schema version of the snapshot.
+    local schema_version, schema_version_str = get_version()
+    if handlers[#handlers].version > schema_version then
+        return true, schema_version_str
+    end
+    return false
 end
 
 local function upgrade(options)
@@ -995,20 +1021,6 @@ local function upgrade(options)
         return
     end
 
-    local handlers = {
-        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
-        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
-        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
-        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
-        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
-        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
-        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
-        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
-        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
-        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
-        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
-    }
-
     for _, handler in ipairs(handlers) do
         if version >= handler.version then
             goto continue
@@ -1047,3 +1059,4 @@ end
 
 box.schema.upgrade = upgrade;
 box.internal.bootstrap = bootstrap;
+box.internal.schema_needs_upgrade = schema_needs_upgrade;
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 4ad3c6493..9326dfa0c 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
  | ---
  | - true
  | ...
+
+--
+-- gh-4574: Check schema version after Tarantool update.
+--
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester8")
+ | ---
+ | - true
+ | ...
+--- Check that the warning is printed.
+version_warning = "Please, consider using box.schema.upgrade()."
+ | ---
+ | ...
+test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester8")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester8")
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester9")
+ | ---
+ | - true
+ | ...
+--- Check that the warning isn't printed.
+test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester9")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester9")
+ | ---
+ | - true
+ | ...
diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
index 56018b1a0..e806c9efe 100644
--- a/test/box/cfg.test.lua
+++ b/test/box/cfg.test.lua
@@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
 test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
 test_run:cmd("stop server cfg_tester7")
 test_run:cmd("cleanup server cfg_tester7")
+
+--
+-- gh-4574: Check schema version after Tarantool update.
+--
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
+test_run:cmd("start server cfg_tester8")
+--- Check that the warning is printed.
+version_warning = "Please, consider using box.schema.upgrade()."
+test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
+test_run:cmd("stop server cfg_tester8")
+test_run:cmd("cleanup server cfg_tester8")
+
+test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
+test_run:cmd("start server cfg_tester9")
+--- Check that the warning isn't printed.
+test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
+test_run:cmd("stop server cfg_tester9")
+test_run:cmd("cleanup server cfg_tester9")
diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
new file mode 100644
index 000000000..c61b86ae3
--- /dev/null
+++ b/test/box/lua/cfg_test8.lua
@@ -0,0 +1,9 @@
+#!/usr/bin/env tarantool
+os = require('os')
+
+box.cfg{
+    listen = os.getenv("LISTEN"),
+    read_only = true
+}
+
+require('console').listen(os.getenv('ADMIN'))
diff --git a/test/box/stat.result b/test/box/stat.result
index 55f29fe59..e808678eb 100644
--- a/test/box/stat.result
+++ b/test/box/stat.result
@@ -24,7 +24,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 1
+- 3
 ...
 box.stat.ERROR.total
 ---
@@ -59,7 +59,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 5
+- 7
 ...
 -- check exceptions
 space:get('Impossible value')
@@ -77,14 +77,14 @@ space:get(1)
 ...
 box.stat.SELECT.total
 ---
-- 6
+- 8
 ...
 space:get(11)
 ---
 ...
 box.stat.SELECT.total
 ---
-- 7
+- 9
 ...
 space:select(5)
 ---
@@ -92,7 +92,7 @@ space:select(5)
 ...
 box.stat.SELECT.total
 ---
-- 8
+- 10
 ...
 space:select(15)
 ---
@@ -100,14 +100,14 @@ space:select(15)
 ...
 box.stat.SELECT.total
 ---
-- 9
+- 11
 ...
 for _ in space:pairs() do end
 ---
 ...
 box.stat.SELECT.total
 ---
-- 10
+- 12
 ...
 -- reset
 box.stat.reset()
@@ -157,7 +157,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 1
+- 3
 ...
 box.stat.ERROR.total
 —

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-11-30 13:43       ` Roman Khabibov
@ 2020-12-01  9:58         ` Serge Petrenko
  2020-12-02  0:16           ` Roman Khabibov
  0 siblings, 1 reply; 14+ messages in thread
From: Serge Petrenko @ 2020-12-01  9:58 UTC (permalink / raw)
  To: Roman Khabibov; +Cc: tarantool-patches


30.11.2020 16:43, Roman Khabibov пишет:
> Thanks.
>
> Serge, could you, please, look through the patch?


Hi! Thanks for the patch!

Since you're working on this instead of Sergey now, you may add yourself to

the Co-developed-by field in the commit message, like it is done here:

https://github.com/tarantool/tarantool/commit/cfccfd449c890c18615185ba4895d9081e50c318


Please see 3 more comments below.


> commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
> Author: Sergey Voinov <sergeiv@tarantool.org>
> Date:   Wed Dec 11 17:28:39 2019 +0300
>
>      box: check schema version after tarantool update
>      
>      Check schema version (stored in box.space._schema) on start and
>      print a warning if it doesn't match last available schema version.
>      It is needed because some users forget to call
>      box.schema.upgrade() after Tarantool update and get stuck with an
>      old schema version until they encounter some hard to debug
>      problems.
>      
>      Closes #4574
>
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 76e2e92c2..451247dcf 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>       box_configured = nil
>   
>       box_is_configured = true
> +
> +    -- Check if schema version matches Tarantool version
> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
> +    local version = box.space._schema:get{'version'}


1. Version unused. You get schema version in `schema_needs_upgrade()` 
anyway.

    Also you may omit testing for nil here. You may just test schema version
    inside `schema_needs_upgrade()` and simply return false, if it is nil.


    You'll also need to update test/box/stat.result after this is done.


> +    if version ~= nil then
> +        local needs, schema_version_str = private.schema_needs_upgrade()
> +        local tarantool_version_str = box.info.version
> +        if needs then
> +            -- Print the warning
> +            local msg = string.format(
> +                'Your schema version is %s while Tarantool %s requires a more'..
> +                ' recent schema version. Please, consider using box.'..
> +                'schema.upgrade().', schema_version_str, tarantool_version_str)
> +            log.warn(msg)
> +        end
> +    end
>   end
>   box.cfg = locked(load_cfg)
>   

> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
> index 56018b1a0..e806c9efe 100644
> --- a/test/box/cfg.test.lua
> +++ b/test/box/cfg.test.lua
> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>   test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>   test_run:cmd("stop server cfg_tester7")
>   test_run:cmd("cleanup server cfg_tester7")
> +
> +--
> +-- gh-4574: Check schema version after Tarantool update.
> +--
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')


2. Can you reuse `cfg_test1.lua` here?


> +test_run:cmd("start server cfg_tester8")
> +--- Check that the warning is printed.
> +version_warning = "Please, consider using box.schema.upgrade()."
> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil


3. Better use `wait_log` instead of `grep_log`. It's not guaranteed that the
    server will print this message by the time you grep for it.


> +test_run:cmd("stop server cfg_tester8")
> +test_run:cmd("cleanup server cfg_tester8")
> +
> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
> +test_run:cmd("start server cfg_tester9")
> +--- Check that the warning isn't printed.
> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
> +test_run:cmd("stop server cfg_tester9")
> +test_run:cmd("cleanup server cfg_tester9")
> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
> new file mode 100644
> index 000000000..c61b86ae3
> --- /dev/null
> +++ b/test/box/lua/cfg_test8.lua
> @@ -0,0 +1,9 @@
> +#!/usr/bin/env tarantool
> +os = require('os')
> +
> +box.cfg{
> +    listen = os.getenv("LISTEN"),
> +    read_only = true
> +}
> +
> +require('console').listen(os.getenv('ADMIN'))

-- 
Serge Petrenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-12-01  9:58         ` Serge Petrenko
@ 2020-12-02  0:16           ` Roman Khabibov
  2020-12-02  9:17             ` Serge Petrenko
  0 siblings, 1 reply; 14+ messages in thread
From: Roman Khabibov @ 2020-12-02  0:16 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: tarantool-patches

Hi! Thanks for the review.

> On Dec 1, 2020, at 12:58, Serge Petrenko <sergepetrenko@tarantool.org> wrote:
> 
> 
> 30.11.2020 16:43, Roman Khabibov пишет:
>> Thanks.
>> 
>> Serge, could you, please, look through the patch?
> 
> 
> Hi! Thanks for the patch!
> 
> Since you're working on this instead of Sergey now, you may add yourself to
> 
> the Co-developed-by field in the commit message, like it is done here:
> 
> https://github.com/tarantool/tarantool/commit/cfccfd449c890c18615185ba4895d9081e50c318
> 
> 
> Please see 3 more comments below.
> 
> 
>> commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
>> Author: Sergey Voinov <sergeiv@tarantool.org>
>> Date:   Wed Dec 11 17:28:39 2019 +0300
>> 
>>     box: check schema version after tarantool update
>>          Check schema version (stored in box.space._schema) on start and
>>     print a warning if it doesn't match last available schema version.
>>     It is needed because some users forget to call
>>     box.schema.upgrade() after Tarantool update and get stuck with an
>>     old schema version until they encounter some hard to debug
>>     problems.
>>          Closes #4574
>> 
>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>> index 76e2e92c2..451247dcf 100644
>> --- a/src/box/lua/load_cfg.lua
>> +++ b/src/box/lua/load_cfg.lua
>> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>>      box_configured = nil
>>        box_is_configured = true
>> +
>> +    -- Check if schema version matches Tarantool version
>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
>> +    local version = box.space._schema:get{'version'}
> 
> 
> 1. Version unused. You get schema version in `schema_needs_upgrade()` anyway.
> 
>    Also you may omit testing for nil here. You may just test schema version
>    inside `schema_needs_upgrade()` and simply return false, if it is nil.
> 
> 
>    You'll also need to update test/box/stat.result after this is done.
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 76e2e92c2..770442052 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -702,6 +702,18 @@ local function load_cfg(cfg)
     box_configured = nil
 
     box_is_configured = true
+
+    -- Check if schema version matches Tarantool version and print
+    -- warning if it's not (in case user forgot to call
+    -- box.schema.upgrade()).
+    local needs, schema_version_str = private.schema_needs_upgrade()
+    if needs then
+        local msg = string.format(
+            'Your schema version is %s while Tarantool %s requires a more'..
+            ' recent schema version. Please, consider using box.'..
+            'schema.upgrade().', schema_version_str, box.info.version)
+        log.warn(msg)
+    end
 end
 box.cfg = locked(load_cfg)

> 
>> +    if version ~= nil then
>> +        local needs, schema_version_str = private.schema_needs_upgrade()
>> +        local tarantool_version_str = box.info.version
>> +        if needs then
>> +            -- Print the warning
>> +            local msg = string.format(
>> +                'Your schema version is %s while Tarantool %s requires a more'..
>> +                ' recent schema version. Please, consider using box.'..
>> +                'schema.upgrade().', schema_version_str, tarantool_version_str)
>> +            log.warn(msg)
>> +        end
>> +    end
>>  end
>>  box.cfg = locked(load_cfg)
>>  
> 
>> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>> index 56018b1a0..e806c9efe 100644
>> --- a/test/box/cfg.test.lua
>> +++ b/test/box/cfg.test.lua
>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>>  test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>>  test_run:cmd("stop server cfg_tester7")
>>  test_run:cmd("cleanup server cfg_tester7")
>> +
>> +--
>> +-- gh-4574: Check schema version after Tarantool update.
>> +--
>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
> 
> 
> 2. Can you reuse `cfg_test1.lua` here?
No, I need to have "read_only = true”.

> 
>> +test_run:cmd("start server cfg_tester8")
>> +--- Check that the warning is printed.
>> +version_warning = "Please, consider using box.schema.upgrade()."
>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
> 
> 
> 3. Better use `wait_log` instead of `grep_log`. It's not guaranteed that the
>    server will print this message by the time you grep for it.
Done.

> 
>> +test_run:cmd("stop server cfg_tester8")
>> +test_run:cmd("cleanup server cfg_tester8")
>> +
>> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
>> +test_run:cmd("start server cfg_tester9")
>> +--- Check that the warning isn't printed.
>> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
>> +test_run:cmd("stop server cfg_tester9")
>> +test_run:cmd("cleanup server cfg_tester9")
>> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
>> new file mode 100644
>> index 000000000..c61b86ae3
>> --- /dev/null
>> +++ b/test/box/lua/cfg_test8.lua
>> @@ -0,0 +1,9 @@
>> +#!/usr/bin/env tarantool
>> +os = require('os')
>> +
>> +box.cfg{
>> +    listen = os.getenv("LISTEN"),
>> +    read_only = true
>> +}
>> +
>> +require('console').listen(os.getenv('ADMIN'))
> 
> -- 
> Serge Petrenko
> 

commit ca9744ddab1f04663ef5fe0c1e7dc872cf6d55fd
Author: Sergey Voinov <sergeiv@tarantool.org>
Date:   Wed Dec 11 17:28:39 2019 +0300

    box: check schema version after tarantool update
    
    Check schema version (stored in box.space._schema) on start and
    print a warning if it doesn't match last available schema version.
    It is needed because some users forget to call
    box.schema.upgrade() after Tarantool update and get stuck with an
    old schema version until they encounter some hard to debug
    problems.
    
    Closes #4574
    
    Co-developed-by: Roman Khabibov <roman.habibov@tarantool.org>

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 76e2e92c2..770442052 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -702,6 +702,18 @@ local function load_cfg(cfg)
     box_configured = nil
 
     box_is_configured = true
+
+    -- Check if schema version matches Tarantool version and print
+    -- warning if it's not (in case user forgot to call
+    -- box.schema.upgrade()).
+    local needs, schema_version_str = private.schema_needs_upgrade()
+    if needs then
+        local msg = string.format(
+            'Your schema version is %s while Tarantool %s requires a more'..
+            ' recent schema version. Please, consider using box.'..
+            'schema.upgrade().', schema_version_str, box.info.version)
+        log.warn(msg)
+    end
 end
 box.cfg = locked(load_cfg)
 
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index add791cd7..a86a0d410 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -973,6 +973,21 @@ end
 
 --------------------------------------------------------------------------------
 
+local handlers = {
+    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
+    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
+    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
+    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
+    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
+    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
+    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
+    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
+    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
+    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
+    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
+}
+
+-- Schema version of the snapshot.
 local function get_version()
     local version = box.space._schema:get{'version'}
     if version == nil then
@@ -982,7 +997,19 @@ local function get_version()
     local minor = version[3]
     local patch = version[4] or 0
 
-    return mkversion(major, minor, patch)
+    return mkversion(major, minor, patch),
+           string.format("%s.%s.%s", major, minor, patch)
+end
+
+local function schema_needs_upgrade()
+    -- Schema needs upgrade if current schema version is greater
+    -- than schema version of the snapshot.
+    local schema_version, schema_version_str = get_version()
+    if schema_version ~= nil and
+        handlers[#handlers].version > schema_version then
+        return true, schema_version_str
+    end
+    return false
 end
 
 local function upgrade(options)
@@ -995,20 +1022,6 @@ local function upgrade(options)
         return
     end
 
-    local handlers = {
-        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
-        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
-        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
-        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
-        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
-        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
-        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
-        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
-        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
-        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
-        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
-    }
-
     for _, handler in ipairs(handlers) do
         if version >= handler.version then
             goto continue
@@ -1047,3 +1060,4 @@ end
 
 box.schema.upgrade = upgrade;
 box.internal.bootstrap = bootstrap;
+box.internal.schema_needs_upgrade = schema_needs_upgrade;
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 4ad3c6493..5ca6ce72b 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
  | ---
  | - true
  | ...
+
+--
+-- gh-4574: Check schema version after Tarantool update.
+--
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester8")
+ | ---
+ | - true
+ | ...
+--- Check that the warning is printed.
+version_warning = "Please, consider using box.schema.upgrade()."
+ | ---
+ | ...
+test_run:wait_log('cfg_tester8', version_warning, 1000, 1.0) ~= nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester8")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester8")
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester9")
+ | ---
+ | - true
+ | ...
+--- Check that the warning isn't printed.
+test_run:wait_log('cfg_tester9', version_warning, 1000, 1.0) == nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester9")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester9")
+ | ---
+ | - true
+ | ...
diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
index 56018b1a0..74100adaa 100644
--- a/test/box/cfg.test.lua
+++ b/test/box/cfg.test.lua
@@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
 test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
 test_run:cmd("stop server cfg_tester7")
 test_run:cmd("cleanup server cfg_tester7")
+
+--
+-- gh-4574: Check schema version after Tarantool update.
+--
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
+test_run:cmd("start server cfg_tester8")
+--- Check that the warning is printed.
+version_warning = "Please, consider using box.schema.upgrade()."
+test_run:wait_log('cfg_tester8', version_warning, 1000, 1.0) ~= nil
+test_run:cmd("stop server cfg_tester8")
+test_run:cmd("cleanup server cfg_tester8")
+
+test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test1.lua"')
+test_run:cmd("start server cfg_tester9")
+--- Check that the warning isn't printed.
+test_run:wait_log('cfg_tester9', version_warning, 1000, 1.0) == nil
+test_run:cmd("stop server cfg_tester9")
+test_run:cmd("cleanup server cfg_tester9")
diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
new file mode 100644
index 000000000..c61b86ae3
--- /dev/null
+++ b/test/box/lua/cfg_test8.lua
@@ -0,0 +1,9 @@
+#!/usr/bin/env tarantool
+os = require('os')
+
+box.cfg{
+    listen = os.getenv("LISTEN"),
+    read_only = true
+}
+
+require('console').listen(os.getenv('ADMIN'))
diff --git a/test/box/stat.result b/test/box/stat.result
index 55f29fe59..1ed243410 100644
--- a/test/box/stat.result
+++ b/test/box/stat.result
@@ -24,7 +24,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 1
+- 2
 ...
 box.stat.ERROR.total
 ---
@@ -59,7 +59,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 5
+- 6
 ...
 -- check exceptions
 space:get('Impossible value')
@@ -77,14 +77,14 @@ space:get(1)
 ...
 box.stat.SELECT.total
 ---
-- 6
+- 7
 ...
 space:get(11)
 ---
 ...
 box.stat.SELECT.total
 ---
-- 7
+- 8
 ...
 space:select(5)
 ---
@@ -92,7 +92,7 @@ space:select(5)
 ...
 box.stat.SELECT.total
 ---
-- 8
+- 9
 ...
 space:select(15)
 ---
@@ -100,14 +100,14 @@ space:select(15)
 ...
 box.stat.SELECT.total
 ---
-- 9
+- 10
 ...
 for _ in space:pairs() do end
 ---
 ...
 box.stat.SELECT.total
 ---
-- 10
+- 11
 ...
 -- reset
 box.stat.reset()
@@ -157,7 +157,7 @@ box.stat.REPLACE.total
 ...
 box.stat.SELECT.total
 ---
-- 1
+- 2
 ...
 box.stat.ERROR.total
 —

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-12-02  0:16           ` Roman Khabibov
@ 2020-12-02  9:17             ` Serge Petrenko
  2020-12-02 15:25               ` roman
  0 siblings, 1 reply; 14+ messages in thread
From: Serge Petrenko @ 2020-12-02  9:17 UTC (permalink / raw)
  To: Roman Khabibov; +Cc: tarantool-patches


02.12.2020 03:16, Roman Khabibov пишет:
> Hi! Thanks for the review.
>
>> On Dec 1, 2020, at 12:58, Serge Petrenko <sergepetrenko@tarantool.org> wrote:
>>
>>
>> 30.11.2020 16:43, Roman Khabibov пишет:
>>> Thanks.
>>>
>>> Serge, could you, please, look through the patch?
>>
>> Hi! Thanks for the patch!
>>
>> Since you're working on this instead of Sergey now, you may add yourself to
>>
>> the Co-developed-by field in the commit message, like it is done here:
>>
>> https://github.com/tarantool/tarantool/commit/cfccfd449c890c18615185ba4895d9081e50c318
>>
>>
>> Please see 3 more comments below.
>>
>>
>>> commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
>>> Author: Sergey Voinov <sergeiv@tarantool.org>
>>> Date:   Wed Dec 11 17:28:39 2019 +0300
>>>
>>>      box: check schema version after tarantool update
>>>           Check schema version (stored in box.space._schema) on start and
>>>      print a warning if it doesn't match last available schema version.
>>>      It is needed because some users forget to call
>>>      box.schema.upgrade() after Tarantool update and get stuck with an
>>>      old schema version until they encounter some hard to debug
>>>      problems.
>>>           Closes #4574
>>>
>>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>>> index 76e2e92c2..451247dcf 100644
>>> --- a/src/box/lua/load_cfg.lua
>>> +++ b/src/box/lua/load_cfg.lua
>>> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>>>       box_configured = nil
>>>         box_is_configured = true
>>> +
>>> +    -- Check if schema version matches Tarantool version
>>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
>>> +    local version = box.space._schema:get{'version'}
>>
>> 1. Version unused. You get schema version in `schema_needs_upgrade()` anyway.
>>
>>     Also you may omit testing for nil here. You may just test schema version
>>     inside `schema_needs_upgrade()` and simply return false, if it is nil.
>>
>>
>>     You'll also need to update test/box/stat.result after this is done.
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 76e2e92c2..770442052 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -702,6 +702,18 @@ local function load_cfg(cfg)
>       box_configured = nil
>   
>       box_is_configured = true
> +
> +    -- Check if schema version matches Tarantool version and print
> +    -- warning if it's not (in case user forgot to call
> +    -- box.schema.upgrade()).
> +    local needs, schema_version_str = private.schema_needs_upgrade()
> +    if needs then
> +        local msg = string.format(
> +            'Your schema version is %s while Tarantool %s requires a more'..
> +            ' recent schema version. Please, consider using box.'..
> +            'schema.upgrade().', schema_version_str, box.info.version)
> +        log.warn(msg)
> +    end
>   end
>   box.cfg = locked(load_cfg)
>
>>> +    if version ~= nil then
>>> +        local needs, schema_version_str = private.schema_needs_upgrade()
>>> +        local tarantool_version_str = box.info.version
>>> +        if needs then
>>> +            -- Print the warning
>>> +            local msg = string.format(
>>> +                'Your schema version is %s while Tarantool %s requires a more'..
>>> +                ' recent schema version. Please, consider using box.'..
>>> +                'schema.upgrade().', schema_version_str, tarantool_version_str)
>>> +            log.warn(msg)
>>> +        end
>>> +    end
>>>   end
>>>   box.cfg = locked(load_cfg)
>>>   
>>> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>>> index 56018b1a0..e806c9efe 100644
>>> --- a/test/box/cfg.test.lua
>>> +++ b/test/box/cfg.test.lua
>>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>>>   test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>>>   test_run:cmd("stop server cfg_tester7")
>>>   test_run:cmd("cleanup server cfg_tester7")
>>> +
>>> +--
>>> +-- gh-4574: Check schema version after Tarantool update.
>>> +--
>>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
>>
>> 2. Can you reuse `cfg_test1.lua` here?
> No, I need to have "read_only = true”.


Oh, now I see. So that the instance doesn't auto-upgrade.

Thanks for the fixes! LGTM.


>>> +test_run:cmd("start server cfg_tester8")
>>> +--- Check that the warning is printed.
>>> +version_warning = "Please, consider using box.schema.upgrade()."
>>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
>>
>> 3. Better use `wait_log` instead of `grep_log`. It's not guaranteed that the
>>     server will print this message by the time you grep for it.
> Done.
>
>>
-- 
Serge Petrenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-12-02  9:17             ` Serge Petrenko
@ 2020-12-02 15:25               ` roman
  2020-12-03 12:19                 ` Kirill Yukhin
  2020-12-03 12:21                 ` Roman Khabibov
  0 siblings, 2 replies; 14+ messages in thread
From: roman @ 2020-12-02 15:25 UTC (permalink / raw)
  To: kyukhin; +Cc: tarantool-patches

Hi! Thanks for LGTM.

Kirill, you can push it.

On 02.12.2020 12:17, Serge Petrenko wrote:
>
> 02.12.2020 03:16, Roman Khabibov пишет:
>> Hi! Thanks for the review.
>>> On Dec 1, 2020, at 12:58, Serge Petrenko 
>>> <sergepetrenko@tarantool.org> wrote:
>>>
>>>
>>> 30.11.2020 16:43, Roman Khabibov пишет:
>>>> Thanks.
>>>>
>>>> Serge, could you, please, look through the patch? 
>>>
>>> Hi! Thanks for the patch!
>>>
>>> Since you're working on this instead of Sergey now, you may add 
>>> yourself to
>>>
>>> the Co-developed-by field in the commit message, like it is done here:
>>>
>>> https://github.com/tarantool/tarantool/commit/cfccfd449c890c18615185ba4895d9081e50c318 
>>>
>>>
>>>
>>> Please see 3 more comments below.
>>>
>>>> commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
>>>> Author: Sergey Voinov <sergeiv@tarantool.org>
>>>> Date:   Wed Dec 11 17:28:39 2019 +0300
>>>>
>>>>      box: check schema version after tarantool update
>>>>           Check schema version (stored in box.space._schema) on 
>>>> start and
>>>>      print a warning if it doesn't match last available schema 
>>>> version.
>>>>      It is needed because some users forget to call
>>>>      box.schema.upgrade() after Tarantool update and get stuck with an
>>>>      old schema version until they encounter some hard to debug
>>>>      problems.
>>>>           Closes #4574
>>>>
>>>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>>>> index 76e2e92c2..451247dcf 100644
>>>> --- a/src/box/lua/load_cfg.lua
>>>> +++ b/src/box/lua/load_cfg.lua
>>>> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>>>>       box_configured = nil
>>>>         box_is_configured = true
>>>> +
>>>> +    -- Check if schema version matches Tarantool version
>>>> +    -- and print warning if it's not (in case user forgot to call 
>>>> box.schema.upgrade())
>>>> +    local version = box.space._schema:get{'version'} 
>>>
>>> 1. Version unused. You get schema version in 
>>> `schema_needs_upgrade()` anyway.
>>>
>>>     Also you may omit testing for nil here. You may just test schema 
>>> version
>>>     inside `schema_needs_upgrade()` and simply return false, if it 
>>> is nil.
>>>
>>>
>>>     You'll also need to update test/box/stat.result after this is done. 
>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>> index 76e2e92c2..770442052 100644
>> --- a/src/box/lua/load_cfg.lua
>> +++ b/src/box/lua/load_cfg.lua
>> @@ -702,6 +702,18 @@ local function load_cfg(cfg)
>>       box_configured = nil
>>         box_is_configured = true
>> +
>> +    -- Check if schema version matches Tarantool version and print
>> +    -- warning if it's not (in case user forgot to call
>> +    -- box.schema.upgrade()).
>> +    local needs, schema_version_str = private.schema_needs_upgrade()
>> +    if needs then
>> +        local msg = string.format(
>> +            'Your schema version is %s while Tarantool %s requires a 
>> more'..
>> +            ' recent schema version. Please, consider using box.'..
>> +            'schema.upgrade().', schema_version_str, box.info.version)
>> +        log.warn(msg)
>> +    end
>>   end
>>   box.cfg = locked(load_cfg)
>>>> +    if version ~= nil then
>>>> +        local needs, schema_version_str = 
>>>> private.schema_needs_upgrade()
>>>> +        local tarantool_version_str = box.info.version
>>>> +        if needs then
>>>> +            -- Print the warning
>>>> +            local msg = string.format(
>>>> +                'Your schema version is %s while Tarantool %s 
>>>> requires a more'..
>>>> +                ' recent schema version. Please, consider using 
>>>> box.'..
>>>> +                'schema.upgrade().', schema_version_str, 
>>>> tarantool_version_str)
>>>> +            log.warn(msg)
>>>> +        end
>>>> +    end
>>>>   end
>>>>   box.cfg = locked(load_cfg)
>>>>   diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>>>> index 56018b1a0..e806c9efe 100644
>>>> --- a/test/box/cfg.test.lua
>>>> +++ b/test/box/cfg.test.lua
>>>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set 
>>>> \'replication\' configuration option to',
>>>>   test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>>>>   test_run:cmd("stop server cfg_tester7")
>>>>   test_run:cmd("cleanup server cfg_tester7")
>>>> +
>>>> +--
>>>> +-- gh-4574: Check schema version after Tarantool update.
>>>> +--
>>>> +test_run:cmd('create server cfg_tester8 with script = 
>>>> "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"') 
>>>
>>> 2. Can you reuse `cfg_test1.lua` here? 
>> No, I need to have "read_only = true”. 
>
>
> Oh, now I see. So that the instance doesn't auto-upgrade.
>
> Thanks for the fixes! LGTM.
>
>>>> +test_run:cmd("start server cfg_tester8")
>>>> +--- Check that the warning is printed.
>>>> +version_warning = "Please, consider using box.schema.upgrade()."
>>>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil 
>>>
>>> 3. Better use `wait_log` instead of `grep_log`. It's not guaranteed 
>>> that the
>>>     server will print this message by the time you grep for it. 
>> Done.
>>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-12-02 15:25               ` roman
@ 2020-12-03 12:19                 ` Kirill Yukhin
  2020-12-03 12:21                 ` Roman Khabibov
  1 sibling, 0 replies; 14+ messages in thread
From: Kirill Yukhin @ 2020-12-03 12:19 UTC (permalink / raw)
  To: roman; +Cc: tarantool-patches

Hello,

On 02 Dec 18:25, roman wrote:
> Hi! Thanks for LGTM.
> 
> Kirill, you can push it.

Let's ask Sasha first.

--
Kirill

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-12-02 15:25               ` roman
  2020-12-03 12:19                 ` Kirill Yukhin
@ 2020-12-03 12:21                 ` Roman Khabibov
  1 sibling, 0 replies; 14+ messages in thread
From: Roman Khabibov @ 2020-12-03 12:21 UTC (permalink / raw)
  To: Alexander V. Tikhonov; +Cc: tarantool-patches

Alexander, can you check it6 please?

> On Dec 2, 2020, at 18:25, roman via Tarantool-patches <tarantool-patches@dev.tarantool.org> wrote:
> 
> Hi! Thanks for LGTM.
> 
> Kirill, you can push it.
> 
> On 02.12.2020 12:17, Serge Petrenko wrote:
>> 
>> 02.12.2020 03:16, Roman Khabibov пишет:
>>> Hi! Thanks for the review.
>>>> On Dec 1, 2020, at 12:58, Serge Petrenko <sergepetrenko@tarantool.org> wrote:
>>>> 
>>>> 
>>>> 30.11.2020 16:43, Roman Khabibov пишет:
>>>>> Thanks.
>>>>> 
>>>>> Serge, could you, please, look through the patch? 
>>>> 
>>>> Hi! Thanks for the patch!
>>>> 
>>>> Since you're working on this instead of Sergey now, you may add yourself to
>>>> 
>>>> the Co-developed-by field in the commit message, like it is done here:
>>>> 
>>>> https://github.com/tarantool/tarantool/commit/cfccfd449c890c18615185ba4895d9081e50c318 
>>>> 
>>>> 
>>>> Please see 3 more comments below.
>>>> 
>>>>> commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d
>>>>> Author: Sergey Voinov <sergeiv@tarantool.org>
>>>>> Date:   Wed Dec 11 17:28:39 2019 +0300
>>>>> 
>>>>>      box: check schema version after tarantool update
>>>>>           Check schema version (stored in box.space._schema) on start and
>>>>>      print a warning if it doesn't match last available schema version.
>>>>>      It is needed because some users forget to call
>>>>>      box.schema.upgrade() after Tarantool update and get stuck with an
>>>>>      old schema version until they encounter some hard to debug
>>>>>      problems.
>>>>>           Closes #4574
>>>>> 
>>>>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>>>>> index 76e2e92c2..451247dcf 100644
>>>>> --- a/src/box/lua/load_cfg.lua
>>>>> +++ b/src/box/lua/load_cfg.lua
>>>>> @@ -702,6 +702,22 @@ local function load_cfg(cfg)
>>>>>       box_configured = nil
>>>>>         box_is_configured = true
>>>>> +
>>>>> +    -- Check if schema version matches Tarantool version
>>>>> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
>>>>> +    local version = box.space._schema:get{'version'} 
>>>> 
>>>> 1. Version unused. You get schema version in `schema_needs_upgrade()` anyway.
>>>> 
>>>>     Also you may omit testing for nil here. You may just test schema version
>>>>     inside `schema_needs_upgrade()` and simply return false, if it is nil.
>>>> 
>>>> 
>>>>     You'll also need to update test/box/stat.result after this is done. 
>>> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>>> index 76e2e92c2..770442052 100644
>>> --- a/src/box/lua/load_cfg.lua
>>> +++ b/src/box/lua/load_cfg.lua
>>> @@ -702,6 +702,18 @@ local function load_cfg(cfg)
>>>       box_configured = nil
>>>         box_is_configured = true
>>> +
>>> +    -- Check if schema version matches Tarantool version and print
>>> +    -- warning if it's not (in case user forgot to call
>>> +    -- box.schema.upgrade()).
>>> +    local needs, schema_version_str = private.schema_needs_upgrade()
>>> +    if needs then
>>> +        local msg = string.format(
>>> +            'Your schema version is %s while Tarantool %s requires a more'..
>>> +            ' recent schema version. Please, consider using box.'..
>>> +            'schema.upgrade().', schema_version_str, box.info.version)
>>> +        log.warn(msg)
>>> +    end
>>>   end
>>>   box.cfg = locked(load_cfg)
>>>>> +    if version ~= nil then
>>>>> +        local needs, schema_version_str = private.schema_needs_upgrade()
>>>>> +        local tarantool_version_str = box.info.version
>>>>> +        if needs then
>>>>> +            -- Print the warning
>>>>> +            local msg = string.format(
>>>>> +                'Your schema version is %s while Tarantool %s requires a more'..
>>>>> +                ' recent schema version. Please, consider using box.'..
>>>>> +                'schema.upgrade().', schema_version_str, tarantool_version_str)
>>>>> +            log.warn(msg)
>>>>> +        end
>>>>> +    end
>>>>>   end
>>>>>   box.cfg = locked(load_cfg)
>>>>>   diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
>>>>> index 56018b1a0..e806c9efe 100644
>>>>> --- a/test/box/cfg.test.lua
>>>>> +++ b/test/box/cfg.test.lua
>>>>> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>>>>>   test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>>>>>   test_run:cmd("stop server cfg_tester7")
>>>>>   test_run:cmd("cleanup server cfg_tester7")
>>>>> +
>>>>> +--
>>>>> +-- gh-4574: Check schema version after Tarantool update.
>>>>> +--
>>>>> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"') 
>>>> 
>>>> 2. Can you reuse `cfg_test1.lua` here? 
>>> No, I need to have "read_only = true”. 
>> 
>> 
>> Oh, now I see. So that the instance doesn't auto-upgrade.
>> 
>> Thanks for the fixes! LGTM.
>> 
>>>>> +test_run:cmd("start server cfg_tester8")
>>>>> +--- Check that the warning is printed.
>>>>> +version_warning = "Please, consider using box.schema.upgrade()."
>>>>> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil 
>>>> 
>>>> 3. Better use `wait_log` instead of `grep_log`. It's not guaranteed that the
>>>>     server will print this message by the time you grep for it. 
>>> Done.
>>>> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-11-15 23:54 [Tarantool-patches] [PATCH] box: check schema version after tarantool update Roman Khabibov
  2020-11-24 12:11 ` Sergey Ostanevich
@ 2020-12-03 12:40 ` Alexander V. Tikhonov
  2020-12-03 12:56 ` Kirill Yukhin
  2 siblings, 0 replies; 14+ messages in thread
From: Alexander V. Tikhonov @ 2020-12-03 12:40 UTC (permalink / raw)
  To: Roman Khabibov; +Cc: tarantool-patches

Hi, thanks for the patch, as I see no new degradation found in
gitlab-ci testing commit criteria pipeline [1], patch LGTM.

[1] - https://gitlab.com/tarantool/tarantool/-/pipelines/223982512

On Mon, Nov 16, 2020 at 02:54:16AM +0300, Roman Khabibov wrote:
> From: Sergey Voinov <sergeiv@tarantool.org>
> 
> Check schema version (stored in box.space._schema) on start and
> print a warning if it doesn't match last avaliable schema version.
> It is needed bcause some users forget to call box.schema.upgrade()
> after Tarantool update and get stuck with an old schema version
> until they encounter some hard to debug problems.
> 
> Closes #4574
> ---
> 
> @ChangeLog: Print log warning if schema version is older than last available schema version (gh-4574).
> 
> Branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
> Issue: https://github.com/tarantool/tarantool/issues/4574
> 
>  src/box/lua/load_cfg.lua   | 19 +++++++++++++++
>  src/box/lua/upgrade.lua    | 36 ++++++++++++++++-----------
>  test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
>  test/box/cfg.test.lua      | 18 ++++++++++++++
>  test/box/lua/cfg_test8.lua |  9 +++++++
>  test/box/lua/cfg_test9.lua |  9 +++++++
>  test/box/stat.result       | 16 ++++++------
>  7 files changed, 135 insertions(+), 22 deletions(-)
>  create mode 100644 test/box/lua/cfg_test8.lua
>  create mode 100644 test/box/lua/cfg_test9.lua
> 
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 76e2e92c2..13088fe73 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -702,6 +702,25 @@ local function load_cfg(cfg)
>      box_configured = nil
>  
>      box_is_configured = true
> +
> +    -- Check if schema version matches Tarantool version
> +    -- and print warning if it's not (in case user forgot to call box.schema.upgrade())
> +    local version = box.space._schema:get{'version'}
> +    if version ~= nil then
> +        local major = version[2]
> +        local minor = version[3]
> +        local patch = version[4] or 0
> +        local schema_version = string.format("%s.%s.%s", major, minor, patch)
> +        local tarantool_version = box.info.version
> +        if private.schema_needs_upgrade() then
> +            -- Print the warning
> +            local msg = string.format(
> +                'Your schema version is %s while Tarantool %s requires a more'..
> +                ' recent schema version. Please, consider using '..
> +                'box.schema.upgrade().', schema_version, tarantool_version)
> +            log.warn(msg)
> +        end
> +    end
>  end
>  box.cfg = locked(load_cfg)
>  
> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index add791cd7..5ea2fb23d 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -973,6 +973,21 @@ end
>  
>  --------------------------------------------------------------------------------
>  
> +local handlers = {
> +    {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
> +    {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
> +    {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
> +    {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
> +    {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
> +    {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
> +    {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
> +    {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
> +    {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
> +    {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
> +    {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
> +}
> +
> +-- Schema version of the snapshot.
>  local function get_version()
>      local version = box.space._schema:get{'version'}
>      if version == nil then
> @@ -985,6 +1000,12 @@ local function get_version()
>      return mkversion(major, minor, patch)
>  end
>  
> +local function schema_needs_upgrade()
> +    -- Schema needs upgrade if current schema version is greater
> +    -- than schema version of the snapshot.
> +    return handlers[#handlers].version > get_version()
> +end
> +
>  local function upgrade(options)
>      options = options or {}
>      setmetatable(options, {__index = {auto = false}})
> @@ -995,20 +1016,6 @@ local function upgrade(options)
>          return
>      end
>  
> -    local handlers = {
> -        {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
> -        {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
> -        {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
> -        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
> -        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
> -        {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
> -        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
> -        {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
> -        {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
> -        {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
> -        {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
> -    }
> -
>      for _, handler in ipairs(handlers) do
>          if version >= handler.version then
>              goto continue
> @@ -1047,3 +1054,4 @@ end
>  
>  box.schema.upgrade = upgrade;
>  box.internal.bootstrap = bootstrap;
> +box.internal.schema_needs_upgrade = schema_needs_upgrade;
> diff --git a/test/box/cfg.result b/test/box/cfg.result
> index 4ad3c6493..676324662 100644
> --- a/test/box/cfg.result
> +++ b/test/box/cfg.result
> @@ -656,3 +656,53 @@ test_run:cmd("cleanup server cfg_tester7")
>   | ---
>   | - true
>   | ...
> +
> +--
> +-- gh-4574: Check schema version after Tarantool update.
> +--
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd("start server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +--- Check that the warning is printed.
> +version_warning = "Please, consider using box.schema.upgrade()."
> + | ---
> + | ...
> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
> + | ---
> + | - true
> + | ...
> +test_run:cmd("stop server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +test_run:cmd("cleanup server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd("start server cfg_tester9")
> + | ---
> + | - true
> + | ...
> +--- Check that the warning isn't printed.
> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
> + | ---
> + | - true
> + | ...
> +test_run:cmd("stop server cfg_tester9")
> + | ---
> + | - true
> + | ...
> +test_run:cmd("cleanup server cfg_tester9")
> + | ---
> + | - true
> + | ...
> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
> index 56018b1a0..015af1a91 100644
> --- a/test/box/cfg.test.lua
> +++ b/test/box/cfg.test.lua
> @@ -159,3 +159,21 @@ test_run:grep_log('cfg_tester7', 'set \'replication\' configuration option to',
>  test_run:grep_log('cfg_tester7', 'test%-cluster%-cookie', 1000)
>  test_run:cmd("stop server cfg_tester7")
>  test_run:cmd("cleanup server cfg_tester7")
> +
> +--
> +-- gh-4574: Check schema version after Tarantool update.
> +--
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua", workdir="sql/upgrade/2.1.0/"')
> +test_run:cmd("start server cfg_tester8")
> +--- Check that the warning is printed.
> +version_warning = "Please, consider using box.schema.upgrade()."
> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~= nil
> +test_run:cmd("stop server cfg_tester8")
> +test_run:cmd("cleanup server cfg_tester8")
> +
> +test_run:cmd('create server cfg_tester9 with script = "box/lua/cfg_test9.lua"')
> +test_run:cmd("start server cfg_tester9")
> +--- Check that the warning isn't printed.
> +test_run:grep_log('cfg_tester9', version_warning, 1000) == nil
> +test_run:cmd("stop server cfg_tester9")
> +test_run:cmd("cleanup server cfg_tester9")
> diff --git a/test/box/lua/cfg_test8.lua b/test/box/lua/cfg_test8.lua
> new file mode 100644
> index 000000000..c61b86ae3
> --- /dev/null
> +++ b/test/box/lua/cfg_test8.lua
> @@ -0,0 +1,9 @@
> +#!/usr/bin/env tarantool
> +os = require('os')
> +
> +box.cfg{
> +    listen = os.getenv("LISTEN"),
> +    read_only = true
> +}
> +
> +require('console').listen(os.getenv('ADMIN'))
> diff --git a/test/box/lua/cfg_test9.lua b/test/box/lua/cfg_test9.lua
> new file mode 100644
> index 000000000..fa8b303f1
> --- /dev/null
> +++ b/test/box/lua/cfg_test9.lua
> @@ -0,0 +1,9 @@
> +#!/usr/bin/env tarantool
> +os = require('os')
> +
> +box.cfg{
> +    listen = os.getenv("LISTEN"),
> +    read_only = false
> +}
> +
> +require('console').listen(os.getenv('ADMIN'))
> diff --git a/test/box/stat.result b/test/box/stat.result
> index 55f29fe59..e808678eb 100644
> --- a/test/box/stat.result
> +++ b/test/box/stat.result
> @@ -24,7 +24,7 @@ box.stat.REPLACE.total
>  ...
>  box.stat.SELECT.total
>  ---
> -- 1
> +- 3
>  ...
>  box.stat.ERROR.total
>  ---
> @@ -59,7 +59,7 @@ box.stat.REPLACE.total
>  ...
>  box.stat.SELECT.total
>  ---
> -- 5
> +- 7
>  ...
>  -- check exceptions
>  space:get('Impossible value')
> @@ -77,14 +77,14 @@ space:get(1)
>  ...
>  box.stat.SELECT.total
>  ---
> -- 6
> +- 8
>  ...
>  space:get(11)
>  ---
>  ...
>  box.stat.SELECT.total
>  ---
> -- 7
> +- 9
>  ...
>  space:select(5)
>  ---
> @@ -92,7 +92,7 @@ space:select(5)
>  ...
>  box.stat.SELECT.total
>  ---
> -- 8
> +- 10
>  ...
>  space:select(15)
>  ---
> @@ -100,14 +100,14 @@ space:select(15)
>  ...
>  box.stat.SELECT.total
>  ---
> -- 9
> +- 11
>  ...
>  for _ in space:pairs() do end
>  ---
>  ...
>  box.stat.SELECT.total
>  ---
> -- 10
> +- 12
>  ...
>  -- reset
>  box.stat.reset()
> @@ -157,7 +157,7 @@ box.stat.REPLACE.total
>  ...
>  box.stat.SELECT.total
>  ---
> -- 1
> +- 3
>  ...
>  box.stat.ERROR.total
>  ---
> -- 
> 2.24.3 (Apple Git-128)
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update
  2020-11-15 23:54 [Tarantool-patches] [PATCH] box: check schema version after tarantool update Roman Khabibov
  2020-11-24 12:11 ` Sergey Ostanevich
  2020-12-03 12:40 ` Alexander V. Tikhonov
@ 2020-12-03 12:56 ` Kirill Yukhin
  2 siblings, 0 replies; 14+ messages in thread
From: Kirill Yukhin @ 2020-12-03 12:56 UTC (permalink / raw)
  To: Roman Khabibov; +Cc: tarantool-patches

Hello,

On 16 Nov 02:54, Roman Khabibov wrote:
> From: Sergey Voinov <sergeiv@tarantool.org>
> 
> Check schema version (stored in box.space._schema) on start and
> print a warning if it doesn't match last avaliable schema version.
> It is needed bcause some users forget to call box.schema.upgrade()
> after Tarantool update and get stuck with an old schema version
> until they encounter some hard to debug problems.
> 
> Closes #4574
> ---
> 
> @ChangeLog: Print log warning if schema version is older than last available schema version (gh-4574).
> 
> Branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
> Issue: https://github.com/tarantool/tarantool/issues/4574

I've checked your patch into master.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-12-03 12:56 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-15 23:54 [Tarantool-patches] [PATCH] box: check schema version after tarantool update Roman Khabibov
2020-11-24 12:11 ` Sergey Ostanevich
2020-11-24 12:21   ` Sergey Ostanevich
2020-11-24 18:53   ` Roman Khabibov
2020-11-30 11:00     ` Sergey Ostanevich
2020-11-30 13:43       ` Roman Khabibov
2020-12-01  9:58         ` Serge Petrenko
2020-12-02  0:16           ` Roman Khabibov
2020-12-02  9:17             ` Serge Petrenko
2020-12-02 15:25               ` roman
2020-12-03 12:19                 ` Kirill Yukhin
2020-12-03 12:21                 ` Roman Khabibov
2020-12-03 12:40 ` Alexander V. Tikhonov
2020-12-03 12:56 ` Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox