From: Sergey Ostanevich <sergos@tarantool.org> To: Roman Khabibov <roman.habibov@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update Date: Tue, 24 Nov 2020 15:21:22 +0300 [thread overview] Message-ID: <35699A19-472D-42C3-86A1-153641769C24@tarantool.org> (raw) In-Reply-To: <39A73765-A387-4EA2-83DA-4B80534AAAF0@tarantool.org> 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)
next prev parent reply other threads:[~2020-11-24 12:21 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 [this message] 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
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=35699A19-472D-42C3-86A1-153641769C24@tarantool.org \ --to=sergos@tarantool.org \ --cc=roman.habibov@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