From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3D56C45C304 for ; Thu, 3 Dec 2020 15:40:14 +0300 (MSK) Date: Thu, 3 Dec 2020 15:40:11 +0300 From: "Alexander V. Tikhonov" Message-ID: <20201203124011.GA21126@hpalx> References: <20201115235416.72858-1-roman.habibov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201115235416.72858-1-roman.habibov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] box: check schema version after tarantool update List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org 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 > > 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) >