[Tarantool-patches] [PATCH v2] Check schema version after tarantool update

[Tarantool-patches] [PATCH v2] Check schema version after tarantool update

[Sergey Voinov]

Some users forget to call box.schema.upgrade() after updating tarantool,
and get stuck with an old schema version until they encounter
some hard to debug problems.

This change checks schema version (stored in box.space._schema) on start
and prints a warning if it doesn't match current tarantool version.

Closes: #4574
---
 Changes in v2:
 - snapshot schema version is now checked against current schema version
   from src/box/lua/upgrade.lua upgrade_handlers table
 
 issue: https://github.com/tarantool/tarantool/issues/4574
 branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
 src/box/lua/load_cfg.lua   | 19 +++++++++++++++
 src/box/lua/upgrade.lua    | 42 +++++++++++++++++++++-----------
 test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
 test/box/cfg.test.lua      | 18 ++++++++++++++
 test/box/lua/cfg_test7.lua |  9 +++++++
 test/box/lua/cfg_test8.lua |  9 +++++++
 test/box/stat.result       | 16 ++++++------
 7 files changed, 141 insertions(+), 22 deletions(-)
 create mode 100644 test/box/lua/cfg_test7.lua
 create mode 100644 test/box/lua/cfg_test8.lua

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 85617c8f0..1261cb08c 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -555,6 +555,25 @@ local function load_cfg(cfg)
     if not box.cfg.read_only and not box.cfg.replication then
         box.schema.upgrade{auto = true}
     end
+
+    -- 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 schema_needs_upgrade() then
+            -- Print the warning
+            local msg = string.format(
+                "Schema version %s doesn't match Tarantool version %s " ..
+                "(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 e71b7fb41..0f7e82275 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -944,6 +944,20 @@ end
 
 --------------------------------------------------------------------------------
 
+local upgrade_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},
+}
+
+-- Schema version of the snapshot
 local function get_version()
     local version = box.space._schema:get{'version'}
     if version == nil then
@@ -956,6 +970,19 @@ local function get_version()
     return mkversion(major, minor, patch)
 end
 
+-- Current Tarantool release schema version
+local function get_current_version()
+    local count = 0
+    for _ in pairs(upgrade_handlers) do count = count + 1 end
+    return upgrade_handlers[count].version
+end
+
+function schema_needs_upgrade()
+    -- Schema needs upgrade if current schema version 
+    -- is greater than schema version of the snapshot
+    return get_current_version() > get_version()
+end
+
 local function upgrade(options)
     options = options or {}
     setmetatable(options, {__index = {auto = false}})
@@ -966,20 +993,7 @@ 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},
-    }
-
-    for _, handler in ipairs(handlers) do
+    for _, handler in ipairs(upgrade_handlers) do
         if version >= handler.version then
             goto continue
         end
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 5370bb870..905bc4828 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -580,3 +580,53 @@ test_run:cmd("cleanup server cfg_tester6")
  | ---
  | - true
  | ...
+
+--
+-- gh-4574: Check schema version after tarantool update
+--
+test_run:cmd('create server cfg_tester7 with script = "box/lua/cfg_test7.lua", workdir="sql/upgrade/2.1.0/"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester7")
+ | ---
+ | - true
+ | ...
+--- check that the warning is printed
+version_warning = "Schema version [0-9]%.[0-9]%.[0-9] doesn't match Tarantool version"
+ | ---
+ | ...
+test_run:grep_log('cfg_tester7', version_warning, 1000) ~= nil
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server cfg_tester7")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server cfg_tester7")
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server cfg_tester8")
+ | ---
+ | - true
+ | ...
+--- check that the warning is NOT printed
+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
+ | ...
diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
index 56ccb6767..05b7bb53e 100644
--- a/test/box/cfg.test.lua
+++ b/test/box/cfg.test.lua
@@ -141,3 +141,21 @@ test_run:cmd("start server cfg_tester6")
 test_run:grep_log('cfg_tester6', 'set \'vinyl_memory\' configuration option to 1073741824', 1000)
 test_run:cmd("stop server cfg_tester6")
 test_run:cmd("cleanup server cfg_tester6")
+
+--
+-- gh-4574: Check schema version after tarantool update
+--
+test_run:cmd('create server cfg_tester7 with script = "box/lua/cfg_test7.lua", workdir="sql/upgrade/2.1.0/"')
+test_run:cmd("start server cfg_tester7")
+--- check that the warning is printed
+version_warning = "Schema version [0-9]%.[0-9]%.[0-9] doesn't match Tarantool version"
+test_run:grep_log('cfg_tester7', version_warning, 1000) ~= nil
+test_run:cmd("stop server cfg_tester7")
+test_run:cmd("cleanup server cfg_tester7")
+
+test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua"')
+test_run:cmd("start server cfg_tester8")
+--- check that the warning is NOT printed
+test_run:grep_log('cfg_tester8', version_warning, 1000) == nil
+test_run:cmd("stop server cfg_tester8")
+test_run:cmd("cleanup server cfg_tester8")
diff --git a/test/box/lua/cfg_test7.lua b/test/box/lua/cfg_test7.lua
new file mode 100644
index 000000000..c61b86ae3
--- /dev/null
+++ b/test/box/lua/cfg_test7.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_test8.lua b/test/box/lua/cfg_test8.lua
new file mode 100644
index 000000000..fa8b303f1
--- /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 = 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.17.1

Re: [Tarantool-patches] [PATCH v2] Check schema version after tarantool update

[Sergey Ostanevich]

Hi! Thanks for the patch!

The only problem I envision is that it'll be hard for user to figure out
the reasoning behind warining, since schema vesrion doesn't need to
match the Tarantool version and in most cases it can't match.
Can you please rephrase like 'Your schema version is %s.%s.%s, while
Tarantool %s.%s.%s requires schema version %s.%s.%s. Please,
consider...'

Regards,
Sergos


On 22 янв 09:57, Sergey Voinov wrote:
> Some users forget to call box.schema.upgrade() after updating tarantool,
> and get stuck with an old schema version until they encounter
> some hard to debug problems.
> 
> This change checks schema version (stored in box.space._schema) on start
> and prints a warning if it doesn't match current tarantool version.
> 
> Closes: #4574
> ---
>  Changes in v2:
>  - snapshot schema version is now checked against current schema version
>    from src/box/lua/upgrade.lua upgrade_handlers table
>  
>  issue: https://github.com/tarantool/tarantool/issues/4574
>  branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version
>  src/box/lua/load_cfg.lua   | 19 +++++++++++++++
>  src/box/lua/upgrade.lua    | 42 +++++++++++++++++++++-----------
>  test/box/cfg.result        | 50 ++++++++++++++++++++++++++++++++++++++
>  test/box/cfg.test.lua      | 18 ++++++++++++++
>  test/box/lua/cfg_test7.lua |  9 +++++++
>  test/box/lua/cfg_test8.lua |  9 +++++++
>  test/box/stat.result       | 16 ++++++------
>  7 files changed, 141 insertions(+), 22 deletions(-)
>  create mode 100644 test/box/lua/cfg_test7.lua
>  create mode 100644 test/box/lua/cfg_test8.lua
> 
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 85617c8f0..1261cb08c 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -555,6 +555,25 @@ local function load_cfg(cfg)
>      if not box.cfg.read_only and not box.cfg.replication then
>          box.schema.upgrade{auto = true}
>      end
> +
> +    -- 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 schema_needs_upgrade() then
> +            -- Print the warning
> +            local msg = string.format(
> +                "Schema version %s doesn't match Tarantool version %s " ..
> +                "(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 e71b7fb41..0f7e82275 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -944,6 +944,20 @@ end
>  
>  --------------------------------------------------------------------------------
>  
> +local upgrade_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},
> +}
> +
> +-- Schema version of the snapshot
>  local function get_version()
>      local version = box.space._schema:get{'version'}
>      if version == nil then
> @@ -956,6 +970,19 @@ local function get_version()
>      return mkversion(major, minor, patch)
>  end
>  
> +-- Current Tarantool release schema version
> +local function get_current_version()
> +    local count = 0
> +    for _ in pairs(upgrade_handlers) do count = count + 1 end
> +    return upgrade_handlers[count].version
> +end
> +
> +function schema_needs_upgrade()
> +    -- Schema needs upgrade if current schema version 
> +    -- is greater than schema version of the snapshot
> +    return get_current_version() > get_version()
> +end
> +
>  local function upgrade(options)
>      options = options or {}
>      setmetatable(options, {__index = {auto = false}})
> @@ -966,20 +993,7 @@ 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},
> -    }
> -
> -    for _, handler in ipairs(handlers) do
> +    for _, handler in ipairs(upgrade_handlers) do
>          if version >= handler.version then
>              goto continue
>          end
> diff --git a/test/box/cfg.result b/test/box/cfg.result
> index 5370bb870..905bc4828 100644
> --- a/test/box/cfg.result
> +++ b/test/box/cfg.result
> @@ -580,3 +580,53 @@ test_run:cmd("cleanup server cfg_tester6")
>   | ---
>   | - true
>   | ...
> +
> +--
> +-- gh-4574: Check schema version after tarantool update
> +--
> +test_run:cmd('create server cfg_tester7 with script = "box/lua/cfg_test7.lua", workdir="sql/upgrade/2.1.0/"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd("start server cfg_tester7")
> + | ---
> + | - true
> + | ...
> +--- check that the warning is printed
> +version_warning = "Schema version [0-9]%.[0-9]%.[0-9] doesn't match Tarantool version"
> + | ---
> + | ...
> +test_run:grep_log('cfg_tester7', version_warning, 1000) ~= nil
> + | ---
> + | - true
> + | ...
> +test_run:cmd("stop server cfg_tester7")
> + | ---
> + | - true
> + | ...
> +test_run:cmd("cleanup server cfg_tester7")
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd("start server cfg_tester8")
> + | ---
> + | - true
> + | ...
> +--- check that the warning is NOT printed
> +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
> + | ...
> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
> index 56ccb6767..05b7bb53e 100644
> --- a/test/box/cfg.test.lua
> +++ b/test/box/cfg.test.lua
> @@ -141,3 +141,21 @@ test_run:cmd("start server cfg_tester6")
>  test_run:grep_log('cfg_tester6', 'set \'vinyl_memory\' configuration option to 1073741824', 1000)
>  test_run:cmd("stop server cfg_tester6")
>  test_run:cmd("cleanup server cfg_tester6")
> +
> +--
> +-- gh-4574: Check schema version after tarantool update
> +--
> +test_run:cmd('create server cfg_tester7 with script = "box/lua/cfg_test7.lua", workdir="sql/upgrade/2.1.0/"')
> +test_run:cmd("start server cfg_tester7")
> +--- check that the warning is printed
> +version_warning = "Schema version [0-9]%.[0-9]%.[0-9] doesn't match Tarantool version"
> +test_run:grep_log('cfg_tester7', version_warning, 1000) ~= nil
> +test_run:cmd("stop server cfg_tester7")
> +test_run:cmd("cleanup server cfg_tester7")
> +
> +test_run:cmd('create server cfg_tester8 with script = "box/lua/cfg_test8.lua"')
> +test_run:cmd("start server cfg_tester8")
> +--- check that the warning is NOT printed
> +test_run:grep_log('cfg_tester8', version_warning, 1000) == nil
> +test_run:cmd("stop server cfg_tester8")
> +test_run:cmd("cleanup server cfg_tester8")
> diff --git a/test/box/lua/cfg_test7.lua b/test/box/lua/cfg_test7.lua
> new file mode 100644
> index 000000000..c61b86ae3
> --- /dev/null
> +++ b/test/box/lua/cfg_test7.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_test8.lua b/test/box/lua/cfg_test8.lua
> new file mode 100644
> index 000000000..fa8b303f1
> --- /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 = 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.17.1
>