From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id D604A24A6C for ; Wed, 24 Jul 2019 17:28:01 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ZpgUdrHoOG5L for ; Wed, 24 Jul 2019 17:28:01 -0400 (EDT) Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 967EA249E0 for ; Wed, 24 Jul 2019 17:28:01 -0400 (EDT) Date: Thu, 25 Jul 2019 00:27:43 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation Message-ID: <20190724212743.jwrti2iuclleogjq@tkn_work_nb> References: <20190708121338.76309-1-roman.habibov@tarantool.org> <20190712095549.ac4hhaxhxzgaik5i@tarantool.org> <20190720190714.5pejxoy2ae43nssp@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20190720190714.5pejxoy2ae43nssp@tkn_work_nb> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Roman Khabibov Cc: tarantool-patches@freelists.org, Konstantin Osipov , Vladislav Shpilevoy , Kirill Yukhin On Sat, Jul 20, 2019 at 10:07:14PM +0300, Alexander Turenko wrote: > > > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua > > > index 2892bb3d3..723e63045 100644 > > > --- a/src/box/lua/net_box.lua > > > +++ b/src/box/lua/net_box.lua > > > @@ -761,6 +761,11 @@ local function create_transport(host, port, user, password, callback, > > > if status ~= 0 then > > > local body > > > body, body_end = decode(body_rpos) > > > + local err_msg = 'Space \'277\' does not exist' > > > > Space 277? Seriously? This code handles not only collations request. > > Should be more generic, IMHO. > > Of course, it is better to use VCOLLATION_ID constant that already > defined above. > > Kirill, I didn't got your comment. This error message is only about one > specific case: there is no such space. I think this approach is better > then version check (that is proposed in the next version of the patch), > because of readability: the version check approach leads to many small > if's, while here we add only one check. > > BTW, Kostja also has a comment: > https://github.com/tarantool/tarantool/commit/47227cfa4c9bac76a882464a2d0440e5137f158e#commitcomment-34342515 > > WBR, Alexander Turenko. We discussed two approaches with Kostja: * Ignore _vcollation absense on a peer during schema (re)load: https://github.com/tarantool/tarantool/commit/47227cfa4c9bac76a882464a2d0440e5137f158e * Don't fire a request according to a peer tarantool version: https://github.com/tarantool/tarantool/commit/abc88ef8d38e21a6c7faf925a19105ce65d45467 Summary of the discussion: * The former lean on the fact that the only reason of the error re absense of _vcollation is an old version of a server. There is no guarantee that this assumption will retain in the future. * The former approach checks an error message against a pattern. Changing of the error message will break backward compatibility. * The latter approach looks bigger then the former one, because of passthrough of remote.peer_version_id value and more code with branching. We should try to find better way to implement this approach. * Kostja also mentioned possibility to return 'collation' field always, even for old peers: just use a table with default collation_id - collation (string) mapping. It will not work with non-standard collations, but maybe it worth to consider this approach anyway. To be honest I don't know net.box code enough to propose better way to implement 2nd approach. Vlad, it would be great if you will look into this and give some pointers to Roman, if time allows. CCed Kostja and Vlad. WBR, Alexander Turenko.