From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (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 247E345C304 for ; Tue, 1 Dec 2020 12:58:22 +0300 (MSK) 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> From: Serge Petrenko Message-ID: <68080f1b-a50d-3e7b-cb3a-7b8e8d6e437b@tarantool.org> Date: Tue, 1 Dec 2020 12:58:21 +0300 MIME-Version: 1.0 In-Reply-To: <271345E1-6C02-40D2-ADC3-50C96F4BCA90@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB 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 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 > 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. > + 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? > +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. > +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