From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (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 2B5844696C3 for ; Sat, 4 Apr 2020 02:38:58 +0300 (MSK) Date: Sat, 4 Apr 2020 02:38:59 +0300 From: Alexander Turenko Message-ID: <20200403233859.57lre4ef6co22g3e@tkn_work_nb> References: <65798b71932c21183072decd2cce8b2b0f88b884.1585773275.git.alexander.turenko@tarantool.org> <78056845-c72f-f97e-de62-2f04baaed0e0@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <78056845-c72f-f97e-de62-2f04baaed0e0@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 On Sat, Apr 04, 2020 at 12:53:53AM +0200, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > On 01/04/2020 22:45, Alexander Turenko wrote: > > After 2.2.0-633-gaa0964ae1 ('net.box: fix schema fetching from 1.10/2.1 > > servers') net.box expects that _vcollation system view exists on a > > tarantool server of 2.2.1+ version. This is however not always so: a > > server may be run on a new version of tarantool, but work on a schema of > > an old version. > > > > The situation with non last schema is usual for replication cluster in > > process of upgrading: all instances run on the new version of tarantool > > first (no auto-upgrade is performed by tarantools in a cluster). Then > > box.schema.upgrade() should be called, but the instances should be > > operable even before the call. > > > > Before the commit net.box was unable to connect a server if it is run on > > a schema without _vcollation system view (say, 2.1.3), but the server > > executable is of 2.2.1 version or newer. > > > > Follows up #4307 > > Fixes #4691 > > --- > > > > https://github.com/tarantool/tarantool/issues/4691 > > https://github.com/tarantool/tarantool/tree/Totktonada/gh-4691-net-box-connect-schema-2-1-3 > > > > src/box/lua/net_box.lua | 9 ++ > > ...4691-net-box-connect-schema-2-1-3.test.lua | 93 ++++++++++++++++++ > > test/box-tap/no_auto_schema_upgrade.lua | 30 ++++++ > > .../snap/2.1.3/00000000000000000000.snap | Bin 0 -> 4466 bytes > > 4 files changed, 132 insertions(+) > > create mode 100755 test/box-tap/gh-4691-net-box-connect-schema-2-1-3.test.lua > > We have xlog/upgrade suite specifically for these tests. With > already designed processes, where and how to store old snaps, etc. > See xlog/upgrade/how_to_add_new_test.md. Not exactly. I want to test the case when an instance is NOT upgraded. So 'upgrade' directory name for snaps would be misleading. I also think that we should move from using compare-against-output tests. I'll not insist on this during review, but personally I will create new tests using pytest-like (phpunit-like, maven-failsafe-like, luatest-like) way (it is our TAP13 tests or at least near to them). I can summarize cons and pros if you want (don't sure whether I did it already somewhere). We can move 'snap' directory upward (right into test/) and use it for snapshots for all test suites. Or keep them per-suite. However see the next paragraph re fill.lua. I prefer to feed initial schema and data right from a test to keep them near to actual usage in test cases (within one file). Sometimes it eases reading of test cases, especially when you're not much familiar with a testing system. I don't much like the approach with fill.lua script (at least for data corresponds to test cases). WBR, Alexander Turenko.