From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp30.i.mail.ru (smtp30.i.mail.ru [94.100.177.90]) (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 CFC3445C304 for ; Thu, 3 Dec 2020 15:21:48 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.1\)) From: Roman Khabibov In-Reply-To: <1817a3ee-fd68-25dd-58d2-9a75fd747f5b@tarantool.org> Date: Thu, 3 Dec 2020 15:21:46 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <49A81DE5-3B45-467B-9C9D-F44CA6C2692D@tarantool.org> References: <20201115235416.72858-1-roman.habibov@tarantool.org> <39A73765-A387-4EA2-83DA-4B80534AAAF0@tarantool.org> <2046B5A1-96D9-48D5-9F17-7AC1EAE83496@tarantool.org> <271345E1-6C02-40D2-ADC3-50C96F4BCA90@tarantool.org> <68080f1b-a50d-3e7b-cb3a-7b8e8d6e437b@tarantool.org> <5F726C10-5FE5-4494-B08A-5657AD44C894@tarantool.org> <1817a3ee-fd68-25dd-58d2-9a75fd747f5b@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: "Alexander V. Tikhonov" Cc: tarantool-patches@dev.tarantool.org Alexander, can you check it6 please? > On Dec 2, 2020, at 18:25, roman via Tarantool-patches = wrote: >=20 > Hi! Thanks for LGTM. >=20 > Kirill, you can push it. >=20 > On 02.12.2020 12:17, Serge Petrenko wrote: >>=20 >> 02.12.2020 03:16, Roman Khabibov =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>> Hi! Thanks for the review. >>>> On Dec 1, 2020, at 12:58, Serge Petrenko = wrote: >>>>=20 >>>>=20 >>>> 30.11.2020 16:43, Roman Khabibov =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>>> Thanks. >>>>>=20 >>>>> Serge, could you, please, look through the patch?=20 >>>>=20 >>>> Hi! Thanks for the patch! >>>>=20 >>>> Since you're working on this instead of Sergey now, you may add = yourself to >>>>=20 >>>> the Co-developed-by field in the commit message, like it is done = here: >>>>=20 >>>> = https://github.com/tarantool/tarantool/commit/cfccfd449c890c18615185ba4895= d9081e50c318=20 >>>>=20 >>>>=20 >>>> Please see 3 more comments below. >>>>=20 >>>>> commit 8b3265c1599772f5a85e47ed1a8232571ec23f8d >>>>> Author: Sergey Voinov >>>>> Date: Wed Dec 11 17:28:39 2019 +0300 >>>>>=20 >>>>> 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 >>>>>=20 >>>>> 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 =3D nil >>>>> 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()) >>>>> + local version =3D box.space._schema:get{'version'}=20 >>>>=20 >>>> 1. Version unused. You get schema version in = `schema_needs_upgrade()` anyway. >>>>=20 >>>> 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. >>>>=20 >>>>=20 >>>> You'll also need to update test/box/stat.result after this is = done.=20 >>> 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 =3D nil >>> 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()). >>> + local needs, schema_version_str =3D = private.schema_needs_upgrade() >>> + if needs then >>> + local msg =3D 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 =3D locked(load_cfg) >>>>> + if version ~=3D nil then >>>>> + local needs, schema_version_str =3D = private.schema_needs_upgrade() >>>>> + local tarantool_version_str =3D box.info.version >>>>> + if needs then >>>>> + -- Print the warning >>>>> + local msg =3D 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 =3D 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 =3D = "box/lua/cfg_test8.lua", workdir=3D"sql/upgrade/2.1.0/"')=20 >>>>=20 >>>> 2. Can you reuse `cfg_test1.lua` here?=20 >>> No, I need to have "read_only =3D true=E2=80=9D.=20 >>=20 >>=20 >> Oh, now I see. So that the instance doesn't auto-upgrade. >>=20 >> Thanks for the fixes! LGTM. >>=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=20= >>>>=20 >>>> 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.=20 >>> Done. >>>>=20