From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (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 9B13B469710 for ; Tue, 24 Nov 2020 15:21:24 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\)) From: Sergey Ostanevich In-Reply-To: <39A73765-A387-4EA2-83DA-4B80534AAAF0@tarantool.org> Date: Tue, 24 Nov 2020 15:21:22 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <35699A19-472D-42C3-86A1-153641769C24@tarantool.org> References: <20201115235416.72858-1-roman.habibov@tarantool.org> <39A73765-A387-4EA2-83DA-4B80534AAAF0@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 And two misprints. > On 24 Nov 2020, at 15:11, Sergey Ostanevich = wrote: >=20 > Hi! >=20 > Thanks for the patch! > See my 2 comments inline. >=20 > Sergos >=20 >=20 >> On 16 Nov 2020, at 02:54, Roman Khabibov = wrote: >>=20 >> From: Sergey Voinov >>=20 >> 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. >>=20 >> Closes #4574 >> --- >>=20 >> @ChangeLog: Print log warning if schema version is older than last = available schema version (gh-4574). >>=20 >> Branch: = https://github.com/tarantool/tarantool/tree/servoin/gh-4574-schema-version= >> Issue: https://github.com/tarantool/tarantool/issues/4574 >>=20 >> 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 >>=20 >> 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 =3D nil >>=20 >> box_is_configured =3D true >> + >> + -- Check if schema version matches Tarantool version >> + -- and print warning if it's not (in case user forgot to call = box.schema.upgrade()) >=20 > 1. The code starting from here=20 >> + local version =3D box.space._schema:get{'version'} >> + if version ~=3D nil then >> + local major =3D version[2] >> + local minor =3D version[3] >> + local patch =3D version[4] or 0 >> + local schema_version =3D string.format("%s.%s.%s", major, = minor, patch) >> + local tarantool_version =3D box.info.version >=20 > to here doesn=E2=80=99t needed, as you do exactly the same inside the = schema_needs_upgrade() >=20 >> + if private.schema_needs_upgrade() then >> + -- Print the warning >> + local msg =3D string.format( >=20 > 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 =3D locked(load_cfg) >>=20 >> 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 >>=20 >> = --------------------------------------------------------------------------= ------ >>=20 >> +local handlers =3D { >> + {version =3D mkversion(1, 7, 6), func =3D upgrade_to_1_7_6, auto = =3D true}, >> + {version =3D mkversion(1, 7, 7), func =3D upgrade_to_1_7_7, auto = =3D true}, >> + {version =3D mkversion(1, 10, 0), func =3D upgrade_to_1_10_0, = auto =3D true}, >> + {version =3D mkversion(1, 10, 2), func =3D upgrade_to_1_10_2, = auto =3D true}, >> + {version =3D mkversion(2, 1, 0), func =3D upgrade_to_2_1_0, auto = =3D true}, >> + {version =3D mkversion(2, 1, 1), func =3D upgrade_to_2_1_1, auto = =3D true}, >> + {version =3D mkversion(2, 1, 2), func =3D upgrade_to_2_1_2, auto = =3D true}, >> + {version =3D mkversion(2, 1, 3), func =3D upgrade_to_2_1_3, auto = =3D true}, >> + {version =3D mkversion(2, 2, 1), func =3D upgrade_to_2_2_1, auto = =3D true}, >> + {version =3D mkversion(2, 3, 0), func =3D upgrade_to_2_3_0, auto = =3D true}, >> + {version =3D mkversion(2, 3, 1), func =3D upgrade_to_2_3_1, auto = =3D true}, >> +} >> + >> +-- Schema version of the snapshot. >> local function get_version() >> local version =3D box.space._schema:get{'version'} >> if version =3D=3D nil then >> @@ -985,6 +1000,12 @@ local function get_version() >> return mkversion(major, minor, patch) >> end >>=20 >> +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() >=20 > 1.2 Update the return value here. >=20 >> +end >> + >> local function upgrade(options) >> options =3D options or {} >> setmetatable(options, {__index =3D {auto =3D false}}) >> @@ -995,20 +1016,6 @@ local function upgrade(options) >> return >> end >>=20 >> - local handlers =3D { >> - {version =3D mkversion(1, 7, 6), func =3D upgrade_to_1_7_6, = auto =3D true}, >> - {version =3D mkversion(1, 7, 7), func =3D upgrade_to_1_7_7, = auto =3D true}, >> - {version =3D mkversion(1, 10, 0), func =3D = upgrade_to_1_10_0, auto =3D true}, >> - {version =3D mkversion(1, 10, 2), func =3D = upgrade_to_1_10_2, auto =3D true}, >> - {version =3D mkversion(2, 1, 0), func =3D upgrade_to_2_1_0, = auto =3D true}, >> - {version =3D mkversion(2, 1, 1), func =3D upgrade_to_2_1_1, = auto =3D true}, >> - {version =3D mkversion(2, 1, 2), func =3D upgrade_to_2_1_2, = auto =3D true}, >> - {version =3D mkversion(2, 1, 3), func =3D upgrade_to_2_1_3, = auto =3D true}, >> - {version =3D mkversion(2, 2, 1), func =3D upgrade_to_2_2_1, = auto =3D true}, >> - {version =3D mkversion(2, 3, 0), func =3D upgrade_to_2_3_0, = auto =3D true}, >> - {version =3D mkversion(2, 3, 1), func =3D upgrade_to_2_3_1, = auto =3D true}, >> - } >> - >> for _, handler in ipairs(handlers) do >> if version >=3D handler.version then >> goto continue >> @@ -1047,3 +1054,4 @@ end >>=20 >> box.schema.upgrade =3D upgrade; >> box.internal.bootstrap =3D bootstrap; >> +box.internal.schema_needs_upgrade =3D 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 =3D = "box/lua/cfg_test8.lua", workdir=3D"sql/upgrade/2.1.0/"') >> + | --- >> + | - true >> + | ... >> +test_run:cmd("start server cfg_tester8") >> + | --- >> + | - true >> + | ... >> +--- Check that the warning is printed. >> +version_warning =3D "Please, consider using box.schema.upgrade()." >> + | --- >> + | ... >> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~=3D 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 =3D = "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) =3D=3D 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 =3D = "box/lua/cfg_test8.lua", workdir=3D"sql/upgrade/2.1.0/"') >=20 >> +test_run:cmd("start server cfg_tester8") >> +--- Check that the warning is printed. >> +version_warning =3D "Please, consider using box.schema.upgrade()." >> +test_run:grep_log('cfg_tester8', version_warning, 1000) ~=3D nil >> +test_run:cmd("stop server cfg_tester8") >> +test_run:cmd("cleanup server cfg_tester8") >> + >> +test_run:cmd('create server cfg_tester9 with script =3D = "box/lua/cfg_test9.lua=E2=80=9D=E2=80=99) >=20 > 2. You can reuse the cfg_test1.lua for the writable node, no need in = extra file. >=20 >> +test_run:cmd("start server cfg_tester9") >> +--- Check that the warning isn't printed. >> +test_run:grep_log('cfg_tester9', version_warning, 1000) =3D=3D 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 =3D require('os') >> + >> +box.cfg{ >> + listen =3D os.getenv("LISTEN"), >> + read_only =3D 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 =3D require('os') >> + >> +box.cfg{ >> + listen =3D os.getenv("LISTEN"), >> + read_only =3D 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 >> --- >> --=20 >> 2.24.3 (Apple Git-128)