From: Roman Khabibov <roman.habibov@tarantool.org> To: Serge Petrenko <sergepetrenko@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update Date: Wed, 2 Dec 2020 03:16:17 +0300 [thread overview] Message-ID: <5F726C10-5FE5-4494-B08A-5657AD44C894@tarantool.org> (raw) In-Reply-To: <68080f1b-a50d-3e7b-cb3a-7b8e8d6e437b@tarantool.org> 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 —
next prev parent reply other threads:[~2020-12-02 0:16 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-15 23:54 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 [this message] 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
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5F726C10-5FE5-4494-B08A-5657AD44C894@tarantool.org \ --to=roman.habibov@tarantool.org \ --cc=sergepetrenko@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox