From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106]) (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 8F2FD4696C3 for ; Mon, 6 Apr 2020 14:39:33 +0300 (MSK) Date: Mon, 6 Apr 2020 14:39:35 +0300 From: Alexander Turenko Message-ID: <20200406113935.fu3zli6s5x4dusgb@tkn_work_nb> References: <65798b71932c21183072decd2cce8b2b0f88b884.1585773275.git.alexander.turenko@tarantool.org> <78056845-c72f-f97e-de62-2f04baaed0e0@tarantool.org> <20200403233859.57lre4ef6co22g3e@tkn_work_nb> <61a72f34-7bd0-576e-e305-dfee085ca724@tarantool.org> <20200405120544.sktsaxfczlkz4ehq@tkn_work_nb.domru> <26b560a7-886b-30e9-58aa-fb4f3b26e559@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <26b560a7-886b-30e9-58aa-fb4f3b26e559@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] net.box: fix fetching of schema of an old version List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org > >> The test and the bug are still upgrade related. 'xlog/upgrade' is not > >> called 'xlog/upgrade_called'. It is called just 'upgrade'. For all cases > >> when an old snapshot is used to start a new tarantool. > > > > I disagree here. 'upgrade' is not something about working upward an old > > snapshot. > > It is. Because you won't work on the old snapshot forever. You are > going to upgrade anyway. Your bug is for when upgrade is started but > not finished. Because Tarantools are new, but upgrade() is not called > yet. It is like "sooner or later a user will use feature X after Y, so let's call Y as X". I understood your point, but still think that the name 'upgrade' is misleading. More general 'snap' or 'snapshots' looks better for me. In fact upgrade may break old connectors (say, due to unicode_ci collation of _func on 2.2) and I guess the old schema may be kept for quite long time so. > > Ouch, okay, fill.lua is called on the old tarantool version (on which > > you generate a snapshot). It is okay for my case to disable autoupgrade > > and insert the data using the new tarantool version. I guess it is okay > > too for most of cases. Your case with upgrade of a sequence can be > > tested this way too. > > No, it can't. Unless you tried and actually managed to reproduce this. > Because I just tried your way. I made checkout to 2.1, took an empty > snapshot (00000000000000000000.snap). Took 2.2 before the sequence fix > (that commit: 13de917a0e3b8affb693d041663c174d9ec7fcc9). Then I started > Tarantool in read-only mode: > > <...snipped steps to reproduce...> > > So even though in your particular case it is ok, it is very > artificial, and can't be applied to all tests. You're right. I missed that we'll unable to reproduce the problem if we'll call fixed upgrade script (from a new tarantool version). > Okey, you can omit fill.lua, but I want upgrade-related tests in > one place. Or organized in one way in maximum two places, if you are > so desperate to use tap. I'll rework the patch to keep snapshots together. Don't sure about feeding initial data in my case (from fill.lua or from a test): I'll took a time to think around. It seems logical to move snapshots out from test suites, since they will be used across the test suites. What do you think?