* [tarantool-patches] [PATCH] net.box: ignore absence of _vcollation @ 2019-07-08 12:13 Roman Khabibov 2019-07-12 9:55 ` [tarantool-patches] " Kirill Yukhin 0 siblings, 1 reply; 10+ messages in thread From: Roman Khabibov @ 2019-07-08 12:13 UTC (permalink / raw) To: tarantool-patches; +Cc: alexander.turenko Ignore error about absence of _vcollation, when a client connects to a server of old Tarantool versions, where this space does not exist. Closes #4307 --- Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4307-vcoll Issue: https://github.com/tarantool/tarantool/issues/4307 src/box/lua/net_box.lua | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) 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' + if id == select3_id and string.find(body[IPROTO_ERROR_KEY], + err_msg) ~= nil then + break + end return error_sm(E_NO_CONNECTION, body[IPROTO_ERROR_KEY]) end if schema_version == nil then @@ -776,7 +781,7 @@ local function create_transport(host, port, user, password, callback, until response[select1_id] and response[select2_id] and response[select3_id] callback('did_fetch_schema', schema_version, response[select1_id], - response[select2_id],response[select3_id]) + response[select2_id], response[select3_id]) set_state('active') return iproto_sm(schema_version) end @@ -1270,16 +1275,28 @@ function remote_methods:_install_schema(schema_version, spaces, indices, local pktype = index[PARTS][k][2] or index[PARTS][k].type local pkfield = index[PARTS][k][1] or index[PARTS][k].field local pkcollation = nil - if pkcollationid ~= nil then + if pkcollationid ~= nil and collations ~= nil then pkcollation = collations[pkcollationid + 1][2] end - local pk = { - type = pktype, - fieldno = pkfield + 1, - collation = pkcollation, - is_nullable = pknullable - } + local pk + if collations ~= nil then + pk = { + type = pktype, + fieldno = pkfield + 1, + collation = pkcollation, + is_nullable = pknullable + } + else + pk = { + type = pktype, + fieldno = pkfield + 1, + collation_id = pkcollationid, + is_nullable = pknullable + } + end + + idx.parts[k] = pk end idx.unique = not not index[OPTS].unique -- 2.20.1 (Apple Git-117) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-08 12:13 [tarantool-patches] [PATCH] net.box: ignore absence of _vcollation Roman Khabibov @ 2019-07-12 9:55 ` Kirill Yukhin 2019-07-19 12:16 ` Roman Khabibov 2019-07-20 19:07 ` Alexander Turenko 0 siblings, 2 replies; 10+ messages in thread From: Kirill Yukhin @ 2019-07-12 9:55 UTC (permalink / raw) To: tarantool-patches; +Cc: alexander.turenko Hello, On 08 Jul 15:13, Roman Khabibov wrote: > Ignore error about absence of _vcollation, when a client connects > to a server of old Tarantool versions, where this space does not > exist. > > Closes #4307 > --- > Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4307-vcoll > Issue: https://github.com/tarantool/tarantool/issues/4307 Add a regression test please. > > src/box/lua/net_box.lua | 33 +++++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 8 deletions(-) > > 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. > @@ -1270,16 +1275,28 @@ function remote_methods:_install_schema(schema_version, spaces, indices, > local pktype = index[PARTS][k][2] or index[PARTS][k].type > local pkfield = index[PARTS][k][1] or index[PARTS][k].field > local pkcollation = nil > - if pkcollationid ~= nil then > + if pkcollationid ~= nil and collations ~= nil then > pkcollation = collations[pkcollationid + 1][2] > end > > - local pk = { > - type = pktype, > - fieldno = pkfield + 1, > - collation = pkcollation, > - is_nullable = pknullable > - } > + local pk > + if collations ~= nil then > + pk = { > + type = pktype, > + fieldno = pkfield + 1, > + collation = pkcollation, > + is_nullable = pknullable > + } > + else > + pk = { > + type = pktype, > + fieldno = pkfield + 1, > + collation_id = pkcollationid, > + is_nullable = pknullable > + } > + end IMHO, it'd be better to not duplicate code and put actual difference under if-then-else hammock. -- Regards, Kirill Yukhin ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-12 9:55 ` [tarantool-patches] " Kirill Yukhin @ 2019-07-19 12:16 ` Roman Khabibov 2019-07-20 19:07 ` Alexander Turenko 1 sibling, 0 replies; 10+ messages in thread From: Roman Khabibov @ 2019-07-19 12:16 UTC (permalink / raw) To: tarantool-patches; +Cc: Kirill Yukhin, Alexander Turenko > On Jul 12, 2019, at 12:55, Kirill Yukhin <kyukhin@tarantool.org> wrote: > > Hello, > > On 08 Jul 15:13, Roman Khabibov wrote: >> Ignore error about absence of _vcollation, when a client connects >> to a server of old Tarantool versions, where this space does not >> exist. >> >> Closes #4307 >> --- >> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4307-vcoll >> Issue: https://github.com/tarantool/tarantool/issues/4307 >> >> src/box/lua/net_box.lua | 33 +++++++++++++++++++++++++-------- >> 1 file changed, 25 insertions(+), 8 deletions(-) >> >> 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. @@ -685,7 +685,7 @@ local function create_transport(host, port, user, password, callback, set_state('active') return console_sm(rid) elseif greeting.protocol == 'Binary' then - return iproto_auth_sm(greeting.salt) + return iproto_auth_sm(greeting.salt, greeting.version_id) else return error_sm(E_NO_CONNECTION, 'Unknown protocol: '..greeting.protocol) @@ -710,11 +710,11 @@ local function create_transport(host, port, user, password, callback, end end - iproto_auth_sm = function(salt) + iproto_auth_sm = function(salt, version_id) set_state('auth') if not user or not password then set_state('fetch_schema') - return iproto_schema_sm() + return iproto_schema_sm(nil, version_id) end encode_auth(send_buf, new_request_id(), user, password, salt) local err, hdr, body_rpos, body_end = send_and_recv_iproto() @@ -727,34 +727,52 @@ local function create_transport(host, port, user, password, callback, return error_sm(E_NO_CONNECTION, body[IPROTO_ERROR_KEY]) end set_state('fetch_schema') - return iproto_schema_sm(hdr[IPROTO_SCHEMA_VERSION_KEY]) + return iproto_schema_sm(hdr[IPROTO_SCHEMA_VERSION_KEY], version_id) end - iproto_schema_sm = function(schema_version) + iproto_schema_sm = function(schema_version, version_id) if not callback('will_fetch_schema') then set_state('active') - return iproto_sm(schema_version) + return iproto_sm(schema_version, version_id) end + local need_vcoll + -- 131584 is tarantool_version_id() for 2.2.0. + if version_id ~= nil and version_id >= 131584 then + need_vcoll = true + else + need_vcoll = false + end + local select1_id = new_request_id() local select2_id = new_request_id() - local select3_id = new_request_id() + local select3_id = nil local response = {} -- fetch everything from space _vspace, 2 = ITER_ALL encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0, 0xFFFFFFFF, nil) -- fetch everything from space _vindex, 2 = ITER_ALL encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0, 0xFFFFFFFF, nil) - -- fetch everything from space _vcollation, 2 = ITER_ALL - encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, 0xFFFFFFFF, - nil) + if need_vcoll == true then + select3_id = new_request_id() + -- fetch everything from space _vcollation, 2 = ITER_ALL + encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, 0xFFFFFFFF, + nil) + end schema_version = nil -- any schema_version will do provided that -- it is consistent across responses + + local function vcoll_response(need_vcoll) + if need_vcoll == true then + return response[select3_id] + end + return true + end repeat local err, hdr, body_rpos, body_end = send_and_recv_iproto() if err then return error_sm(err, hdr) end dispatch_response_iproto(hdr, body_rpos, body_end) local id = hdr[IPROTO_SYNC_KEY] - if id == select1_id or id == select2_id or id == select3_id then + if id == select1_id or id == select2_id or id == select3_id and need_vcoll then -- response to a schema query we've submitted local status = hdr[IPROTO_STATUS_KEY] local response_schema_version = hdr[IPROTO_SCHEMA_VERSION_KEY] @@ -767,21 +785,21 @@ local function create_transport(host, port, user, password, callback, schema_version = response_schema_version elseif schema_version ~= response_schema_version then -- schema changed while fetching schema; restart loader - return iproto_schema_sm() + return iproto_schema_sm(nil, version_id) end local body body, body_end = decode(body_rpos) response[id] = body[IPROTO_DATA_KEY] end until response[select1_id] and response[select2_id] and - response[select3_id] + vcoll_response(need_vcoll) callback('did_fetch_schema', schema_version, response[select1_id], - response[select2_id],response[select3_id]) + response[select2_id], need_vcoll and response[select3_id] or nil) set_state('active') - return iproto_sm(schema_version) + return iproto_sm(schema_version, version_id) end - iproto_sm = function(schema_version) + iproto_sm = function(schema_version, version_id) local err, hdr, body_rpos, body_end = send_and_recv_iproto() if err then return error_sm(err, hdr) end dispatch_response_iproto(hdr, body_rpos, body_end) @@ -794,9 +812,9 @@ local function create_transport(host, port, user, password, callback, local body body, body_end = decode(body_rpos) set_state('fetch_schema') - return iproto_schema_sm(schema_version) + return iproto_schema_sm(schema_version, version_id) end - return iproto_sm(schema_version) + return iproto_sm(schema_version, version_id) end error_sm = function(err, msg) >> @@ -1270,16 +1275,28 @@ function remote_methods:_install_schema(schema_version, spaces, indices, >> local pktype = index[PARTS][k][2] or index[PARTS][k].type >> local pkfield = index[PARTS][k][1] or index[PARTS][k].field >> local pkcollation = nil >> - if pkcollationid ~= nil then >> + if pkcollationid ~= nil and collations ~= nil then >> pkcollation = collations[pkcollationid + 1][2] >> end >> >> - local pk = { >> - type = pktype, >> - fieldno = pkfield + 1, >> - collation = pkcollation, >> - is_nullable = pknullable >> - } >> + local pk >> + if collations ~= nil then >> + pk = { >> + type = pktype, >> + fieldno = pkfield + 1, >> + collation = pkcollation, >> + is_nullable = pknullable >> + } >> + else >> + pk = { >> + type = pktype, >> + fieldno = pkfield + 1, >> + collation_id = pkcollationid, >> + is_nullable = pknullable >> + } >> + end > > IMHO, it'd be better to not duplicate code and put actual difference > under if-then-else hammock. @@ -1270,16 +1288,21 @@ function remote_methods:_install_schema(schema_version, spaces, indices, local pktype = index[PARTS][k][2] or index[PARTS][k].type local pkfield = index[PARTS][k][1] or index[PARTS][k].field local pkcollation = nil - if pkcollationid ~= nil then + if pkcollationid ~= nil and collations ~= nil then pkcollation = collations[pkcollationid + 1][2] end local pk = { type = pktype, fieldno = pkfield + 1, - collation = pkcollation, is_nullable = pknullable } + if collations ~= nil then + pk.collation = pkcollation + else + pk.collation_id = pkcollationid + end + commit abc88ef8d38e21a6c7faf925a19105ce65d45467 Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Mon Jul 8 14:52:48 2019 +0300 net.box: take into account _vcollation presence The error occured, when a client connects to a server of old Tarantool versions, where _vcollation space does not exist. After this patch, _vcollation is fetched depending on a server version. Closes #4307 diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua index 2892bb3d3..9fe792195 100644 --- a/src/box/lua/net_box.lua +++ b/src/box/lua/net_box.lua @@ -685,7 +685,7 @@ local function create_transport(host, port, user, password, callback, set_state('active') return console_sm(rid) elseif greeting.protocol == 'Binary' then - return iproto_auth_sm(greeting.salt) + return iproto_auth_sm(greeting.salt, greeting.version_id) else return error_sm(E_NO_CONNECTION, 'Unknown protocol: '..greeting.protocol) @@ -710,11 +710,11 @@ local function create_transport(host, port, user, password, callback, end end - iproto_auth_sm = function(salt) + iproto_auth_sm = function(salt, version_id) set_state('auth') if not user or not password then set_state('fetch_schema') - return iproto_schema_sm() + return iproto_schema_sm(nil, version_id) end encode_auth(send_buf, new_request_id(), user, password, salt) local err, hdr, body_rpos, body_end = send_and_recv_iproto() @@ -727,34 +727,52 @@ local function create_transport(host, port, user, password, callback, return error_sm(E_NO_CONNECTION, body[IPROTO_ERROR_KEY]) end set_state('fetch_schema') - return iproto_schema_sm(hdr[IPROTO_SCHEMA_VERSION_KEY]) + return iproto_schema_sm(hdr[IPROTO_SCHEMA_VERSION_KEY], version_id) end - iproto_schema_sm = function(schema_version) + iproto_schema_sm = function(schema_version, version_id) if not callback('will_fetch_schema') then set_state('active') - return iproto_sm(schema_version) + return iproto_sm(schema_version, version_id) end + local need_vcoll + -- 131584 is tarantool_version_id() for 2.2.0. + if version_id ~= nil and version_id >= 131584 then + need_vcoll = true + else + need_vcoll = false + end + local select1_id = new_request_id() local select2_id = new_request_id() - local select3_id = new_request_id() + local select3_id = nil local response = {} -- fetch everything from space _vspace, 2 = ITER_ALL encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0, 0xFFFFFFFF, nil) -- fetch everything from space _vindex, 2 = ITER_ALL encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0, 0xFFFFFFFF, nil) - -- fetch everything from space _vcollation, 2 = ITER_ALL - encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, 0xFFFFFFFF, - nil) + if need_vcoll == true then + select3_id = new_request_id() + -- fetch everything from space _vcollation, 2 = ITER_ALL + encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, 0xFFFFFFFF, + nil) + end schema_version = nil -- any schema_version will do provided that -- it is consistent across responses + + local function vcoll_response(need_vcoll) + if need_vcoll == true then + return response[select3_id] + end + return true + end repeat local err, hdr, body_rpos, body_end = send_and_recv_iproto() if err then return error_sm(err, hdr) end dispatch_response_iproto(hdr, body_rpos, body_end) local id = hdr[IPROTO_SYNC_KEY] - if id == select1_id or id == select2_id or id == select3_id then + if id == select1_id or id == select2_id or id == select3_id and need_vcoll then -- response to a schema query we've submitted local status = hdr[IPROTO_STATUS_KEY] local response_schema_version = hdr[IPROTO_SCHEMA_VERSION_KEY] @@ -767,21 +785,21 @@ local function create_transport(host, port, user, password, callback, schema_version = response_schema_version elseif schema_version ~= response_schema_version then -- schema changed while fetching schema; restart loader - return iproto_schema_sm() + return iproto_schema_sm(nil, version_id) end local body body, body_end = decode(body_rpos) response[id] = body[IPROTO_DATA_KEY] end until response[select1_id] and response[select2_id] and - response[select3_id] + vcoll_response(need_vcoll) callback('did_fetch_schema', schema_version, response[select1_id], - response[select2_id],response[select3_id]) + response[select2_id], need_vcoll and response[select3_id] or nil) set_state('active') - return iproto_sm(schema_version) + return iproto_sm(schema_version, version_id) end - iproto_sm = function(schema_version) + iproto_sm = function(schema_version, version_id) local err, hdr, body_rpos, body_end = send_and_recv_iproto() if err then return error_sm(err, hdr) end dispatch_response_iproto(hdr, body_rpos, body_end) @@ -794,9 +812,9 @@ local function create_transport(host, port, user, password, callback, local body body, body_end = decode(body_rpos) set_state('fetch_schema') - return iproto_schema_sm(schema_version) + return iproto_schema_sm(schema_version, version_id) end - return iproto_sm(schema_version) + return iproto_sm(schema_version, version_id) end error_sm = function(err, msg) @@ -1270,16 +1288,21 @@ function remote_methods:_install_schema(schema_version, spaces, indices, local pktype = index[PARTS][k][2] or index[PARTS][k].type local pkfield = index[PARTS][k][1] or index[PARTS][k].field local pkcollation = nil - if pkcollationid ~= nil then + if pkcollationid ~= nil and collations ~= nil then pkcollation = collations[pkcollationid + 1][2] end local pk = { type = pktype, fieldno = pkfield + 1, - collation = pkcollation, is_nullable = pknullable } + if collations ~= nil then + pk.collation = pkcollation + else + pk.collation_id = pkcollationid + end + idx.parts[k] = pk end idx.unique = not not index[OPTS].unique ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-12 9:55 ` [tarantool-patches] " Kirill Yukhin 2019-07-19 12:16 ` Roman Khabibov @ 2019-07-20 19:07 ` Alexander Turenko 2019-07-24 21:27 ` Alexander Turenko 1 sibling, 1 reply; 10+ messages in thread From: Alexander Turenko @ 2019-07-20 19:07 UTC (permalink / raw) To: Kirill Yukhin; +Cc: tarantool-patches, Roman Khabibov, Konstantin Osipov > > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-20 19:07 ` Alexander Turenko @ 2019-07-24 21:27 ` Alexander Turenko 2019-07-24 22:43 ` Vladislav Shpilevoy 0 siblings, 1 reply; 10+ messages in thread From: Alexander Turenko @ 2019-07-24 21:27 UTC (permalink / raw) To: Roman Khabibov Cc: tarantool-patches, 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-24 21:27 ` Alexander Turenko @ 2019-07-24 22:43 ` Vladislav Shpilevoy 2019-07-30 10:58 ` Alexander Turenko 0 siblings, 1 reply; 10+ messages in thread From: Vladislav Shpilevoy @ 2019-07-24 22:43 UTC (permalink / raw) To: Alexander Turenko, Roman Khabibov Cc: tarantool-patches, Konstantin Osipov, Kirill Yukhin Hi! Yeah, I remember that I should explain my vision of how that task should be solved. See below a long-read. Here are my explanations on how netbox works, and on how should this issue be fixed (perhaps). Netbox for each connection creates a worker fiber and a metadata object keeping connect timeout, reconnect timeout, status, etc. The metadata object is known as a netbox connection, and is the object with which a user works. It is a result of 'netbox.connect()'. Worker fiber does real work: establishes a tcp connection, sends/receives data. But there appears a problem of cyclic references. The thing is that the worker fiber needs some metadata from the netbox object. At least, 'reconnect_timeout'. On the other hand, the netbox object needs some results from the worker fiber - responses on the requests, connection status. In other words, both the worker fiber and the netbox object need something from each other. If the worker fiber and the netbox object would keep references to each other, they would never be deleted - Lua GC is not that smart. So there is a channel of communication between them, which references them both weakly. https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L834-L852 This function is called when a netbox object is created. It takes some mandatory meta like host, port, and a so called 'callback'. It is a closure defined here: https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L910-L954 The closure has access to the netbox object https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L909 and returns certain parameters of it. The worker fiber uses a weak reference to this closure as a proxy to read metadata of the netbox object: https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L837-L840. For example, when the worker fiber needs to obtain reconnect_timeout, it calls the callback https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L443. The callback here is a weak reference to the closure https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L948-L953. And vice versa - when the worker fiber wants to return something back to the netbox object, it calls the same callback, but with a different argument: https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L399. How does that callback help to GC the connection? Lets assume, that there are no references to a netbox object in user code. It is not referenced in the worker fiber either, because the only reference is callback, and it is weak. So it is deleted. Together with https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L844-L846. It is a standard way of setting your own callbacks on GC of a table - you create a dummy cdata object with a custom GC callback. On deletion of that gc-hook a function https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L473-L481 is called. It cancels the worker fiber. Now both the netbox object and the worker fiber are deleted. Talking of this issue: your situation is simpler, as I see now, when I dived a bit into the problem. You have nothing to do about the netbox object. Your target is the worker fiber only. And in such a case I don't see any magic solution that would shrink this patch https://github.com/tarantool/tarantool/commit/abc88ef8d38e21a6c7faf925a19105ce65d45467 to a couple of lines. The only obvious thing you can do here is to avoid passing version id to iproto_auth_sm and iproto_schema_sm functions, and Alexander proposed. Just make it a global value here https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L232 together with the others, if you still will need. Your case is different from reconnect_timeout and other callback-stuff, because these parameters are set by a user out of the worker fiber, and can be changed by him. Callback allows the worker fiber to read actual values always. Version id originates from the worker fiber, and so can be maintained and kept here. Talking of how to remove branching - it has nothing to do with netbox specific code, or GC, or anything application dependent. It is a pure technical matter. You have a classic problem already solved in box/lua/upgrade.lua. The problem is that you have a pipeline of versions, each adding, or removing, or changing something. And it is logical to have a pipeline of functions building that schema step by step. IMO, you should have several functions to fetch and to install schema. Foe example: function fetch_schema_2_2_1() ... end function fetch_schema_2_2_2() fetch_schema_2_2_1() -- + some v_collation things. end function fetch_schema_2_2_3() fetch_schema_2_2_3() --- + ... end And you have a global table where key is version id, value is a schema fetch function. Here https://github.com/tarantool/tarantool/commit/abc88ef8d38e21a6c7faf925a19105ce65d45467#diff-a9dd9be71f8af749f288abb6304a5185R733 you take a fetcher from the table like 'fetcher = fetchers[version_id]', and just call it. All these encode selects https://github.com/tarantool/tarantool/commit/abc88ef8d38e21a6c7faf925a19105ce65d45467#diff-a9dd9be71f8af749f288abb6304a5185R754 should move to these schema fetchers. Of course, it wouldn't be that simple. At least, because schema fetch requests are sent in parallel, as you can see here: https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L743-L748 And still the idea is the same. Perhaps you will have not only fetcher for each version, but two functions: start_fetch/end_fetch, or begin_fetch/end_fetch. A first function sends a request (encode_select), a second one waits for a response. Or let these fetchers return an asynchronous id, by which you will wait for responses in one place. Don't know what to chose. If I were you, and I had time, I would try to implement these step-wise fetchers. They would be quite easy to support and to extend for other future schema changes. Perhaps, it will remove the branching. Up to you what to do, to Alexander, and to other managers. My only wish is to look at the final patch when it will be ready. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-24 22:43 ` Vladislav Shpilevoy @ 2019-07-30 10:58 ` Alexander Turenko 2019-07-30 12:08 ` Konstantin Osipov 2019-07-30 20:18 ` Vladislav Shpilevoy 0 siblings, 2 replies; 10+ messages in thread From: Alexander Turenko @ 2019-07-30 10:58 UTC (permalink / raw) To: Vladislav Shpilevoy Cc: Roman Khabibov, tarantool-patches, Konstantin Osipov, Kirill Yukhin Thank you for the detailed overview and the ways to solve the problem! > Talking of this issue: your situation is simpler, as I see now, when I dived a > bit into the problem. You have nothing to do about the netbox object. Your target > is the worker fiber only. And in such a case I don't see any magic solution that > would shrink this patch https://github.com/tarantool/tarantool/commit/abc88ef8d38e21a6c7faf925a19105ce65d45467 > to a couple of lines. The only obvious thing you can do here is to avoid passing > version id to iproto_auth_sm and iproto_schema_sm functions, and Alexander proposed. > Just make it a global value here > https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L232 > together with the others, if you still will need. I found it is even simpler: we already have greeting in this context and can use it directly. > Talking of how to remove branching - it has nothing to do with netbox specific code, > or GC, or anything application dependent. It is a pure technical matter. You have a > classic problem already solved in box/lua/upgrade.lua. The problem is that you have a > pipeline of versions, each adding, or removing, or changing something. And it is logical > to have a pipeline of functions building that schema step by step. IMO, you should have > several functions to fetch and to install schema. > > Foe example: > > function fetch_schema_2_2_1() > ... > end > > function fetch_schema_2_2_2() > fetch_schema_2_2_1() > -- + some v_collation things. > end > > function fetch_schema_2_2_3() > fetch_schema_2_2_3() > --- + ... > end > > And you have a global table where key is version id, value is a schema fetch > function. Here > https://github.com/tarantool/tarantool/commit/abc88ef8d38e21a6c7faf925a19105ce65d45467#diff-a9dd9be71f8af749f288abb6304a5185R733 > you take a fetcher from the table like 'fetcher = fetchers[version_id]', and just > call it. We'll either need to add a fetcher for each release or choose a last that is not newer then a peer's version_id. > All these encode selects > https://github.com/tarantool/tarantool/commit/abc88ef8d38e21a6c7faf925a19105ce65d45467#diff-a9dd9be71f8af749f288abb6304a5185R754 > should move to these schema fetchers. Of course, it wouldn't be that simple. At least, > because schema fetch requests are sent in parallel, as you can see here: > https://github.com/tarantool/tarantool/blob/0529e2f0ef7dd4c19283b9e8e6b0e7e43bf400a3/src/box/lua/net_box.lua#L743-L748 > And still the idea is the same. Perhaps you will have not only fetcher for each > version, but two functions: start_fetch/end_fetch, or begin_fetch/end_fetch. A > first function sends a request (encode_select), a second one waits for a response. > Or let these fetchers return an asynchronous id, by which you will wait for > responses in one place. Don't know what to chose. If I were you, and I had time, I > would try to implement these step-wise fetchers. They would be quite easy to > support and to extend for other future schema changes. The idea is interesting. This however would take some effort (several days for me I guess) to experimenting and find a good variant. Sadly, it seems I can not pay enough time to that. > Perhaps, it will remove the branching. Up to you what to do, to Alexander, and to > other managers. My only wish is to look at the final patch when it will be ready. Please, look at the patch below or on Totktonada/gh-4307-net-box-fix-schema-fetch branch if the time allows. It is more or less the same as Roman's one, but I was charged by your great email and tried to minimize the patch as possible. ---- commit fff809e6e3d253245a1c596f3c2d51ab983f3efe Author: Alexander Turenko <alexander.turenko@tarantool.org> Date: Tue Jul 30 03:38:22 2019 +0300 net.box: fix schema fetching from 1.10/2.1 servers After 2.2.0-390-ga7c855e5b ("net.box: fetch '_vcollation' sysview into the module") net.box fetches _vcollation view unconditionally, while the view was added in 2.2.0-389-g3e3ef182f and, say, tarantool-1.10 and tarantool-2.1 do not have it. This leads to a runtime error "Space '277' does not exist" on a newer client that connects to an older server. Now the view is fetched conditionally depending of a version of a server: if it is above 2.2.1, then net.box will fetch it. Note: at the time there are no release with a number above 2.2.1. When _vcollation view is available, a collation in an index part will be shown by its name (with 'collation' field), otherwise it will be shown by its ID (in 'collation_id' field). For example: Connect to tarantool 1.10: | tarantool> connection = require('net.box').connect('localhost:3301') | --- | ... | | tarantool> connection.space.s.index.sk.parts | --- | - - type: string | is_nullable: false | collation_id: 2 | fieldno: 2 | ... Connect to tarantool 2.2.1 (when it will be released): | tarantool> connection = require('net.box').connect('localhost:3301') | --- | ... | | tarantool> connection.space.s.index.sk.parts | --- | - - type: string | is_nullable: false | collation: unicode_ci | fieldno: 2 | ... Fixes #4307. diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua index 2892bb3d3..31a8c16b7 100644 --- a/src/box/lua/net_box.lua +++ b/src/box/lua/net_box.lua @@ -84,6 +84,14 @@ local function decode_push(raw_data) return response[IPROTO_DATA_KEY][1], raw_end end +local function version_id(major, minor, patch) + return bit.bor(bit.lshift(major, 16), bit.lshift(minor, 8), patch) +end + +local function version_at_least(peer_version_id, major, minor, patch) + return peer_version_id >= version_id(major, minor, patch) +end + local method_encoder = { ping = internal.encode_ping, call_16 = internal.encode_call_16, @@ -735,17 +743,23 @@ local function create_transport(host, port, user, password, callback, set_state('active') return iproto_sm(schema_version) end + -- _vcollation view was added in 2.2.0-389-g3e3ef182f + local peer_has_vcollation = version_at_least(greeting.version_id, + 2, 2, 1) local select1_id = new_request_id() local select2_id = new_request_id() - local select3_id = new_request_id() + local select3_id local response = {} -- fetch everything from space _vspace, 2 = ITER_ALL encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0, 0xFFFFFFFF, nil) -- fetch everything from space _vindex, 2 = ITER_ALL encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0, 0xFFFFFFFF, nil) -- fetch everything from space _vcollation, 2 = ITER_ALL - encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, 0xFFFFFFFF, - nil) + if peer_has_vcollation then + select3_id = new_request_id() + encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, + 0xFFFFFFFF, nil) + end schema_version = nil -- any schema_version will do provided that -- it is consistent across responses @@ -754,6 +768,8 @@ local function create_transport(host, port, user, password, callback, if err then return error_sm(err, hdr) end dispatch_response_iproto(hdr, body_rpos, body_end) local id = hdr[IPROTO_SYNC_KEY] + -- trick: omit check for peer_has_vcollation: id is + -- not nil if id == select1_id or id == select2_id or id == select3_id then -- response to a schema query we've submitted local status = hdr[IPROTO_STATUS_KEY] @@ -774,9 +790,10 @@ local function create_transport(host, port, user, password, callback, response[id] = body[IPROTO_DATA_KEY] end until response[select1_id] and response[select2_id] and - response[select3_id] + (not peer_has_vcollation or response[select3_id]) + -- trick: response[select3_id] is nil when the key is nil callback('did_fetch_schema', schema_version, response[select1_id], - response[select2_id],response[select3_id]) + response[select2_id], response[select3_id]) set_state('active') return iproto_sm(schema_version) end @@ -1269,17 +1286,23 @@ function remote_methods:_install_schema(schema_version, spaces, indices, local pkcollationid = index[PARTS][k].collation local pktype = index[PARTS][k][2] or index[PARTS][k].type local pkfield = index[PARTS][k][1] or index[PARTS][k].field + -- resolve a collation name if a peer has + -- _vcollation view local pkcollation = nil - if pkcollationid ~= nil then + if pkcollationid ~= nil and collations ~= nil then pkcollation = collations[pkcollationid + 1][2] end local pk = { type = pktype, fieldno = pkfield + 1, - collation = pkcollation, is_nullable = pknullable } + if collations == nil then + pk.collation_id = pkcollationid + else + pk.collation = pkcollation + end idx.parts[k] = pk end idx.unique = not not index[OPTS].unique diff --git a/test/box/net.box.result b/test/box/net.box.result index 1d9f43400..e3dabf7d9 100644 --- a/test/box/net.box.result +++ b/test/box/net.box.result @@ -2905,18 +2905,41 @@ c = net:connect(box.cfg.listen) box.internal.collation.create('test', 'ICU', 'ru-RU') --- ... +collation_id = box.internal.collation.id_by_name('test') +--- +... _ = space:create_index('sk', { type = 'tree', parts = {{1, 'str', collation = 'test'}}, unique = true }) --- ... c:reload_schema() --- ... -c.space.test.index.sk.parts +parts = c.space.test.index.sk.parts --- -- - type: string - is_nullable: false - collation: test - fieldno: 1 +... +#parts == 1 +--- +- true +... +parts[1].fieldno == 1 +--- +- true +... +parts[1].type == 'string' +--- +- true +... +parts[1].is_nullable == false +--- +- true +... +if _TARANTOOL >= '2.2.1' then \ + return parts[1].collation == 'test' \ +else \ + return parts[1].collation_id == collation_id \ +end +--- +- true ... c:close() --- diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua index de629ab59..8e65ff470 100644 --- a/test/box/net.box.test.lua +++ b/test/box/net.box.test.lua @@ -1195,9 +1195,19 @@ c:close() box.schema.user.grant('guest', 'read', 'space', 'test') c = net:connect(box.cfg.listen) box.internal.collation.create('test', 'ICU', 'ru-RU') +collation_id = box.internal.collation.id_by_name('test') _ = space:create_index('sk', { type = 'tree', parts = {{1, 'str', collation = 'test'}}, unique = true }) c:reload_schema() -c.space.test.index.sk.parts +parts = c.space.test.index.sk.parts +#parts == 1 +parts[1].fieldno == 1 +parts[1].type == 'string' +parts[1].is_nullable == false +if _TARANTOOL >= '2.2.1' then \ + return parts[1].collation == 'test' \ +else \ + return parts[1].collation_id == collation_id \ +end c:close() box.internal.collation.drop('test') space:drop() diff --git a/test/box/stat_net.result b/test/box/stat_net.result index 9eeada7ec..2cf567d57 100644 --- a/test/box/stat_net.result +++ b/test/box/stat_net.result @@ -83,9 +83,9 @@ box.stat.net.CONNECTIONS.total --- - 4 ... -box.stat.net.REQUESTS.total +box.stat.net.REQUESTS.total > 0 --- -- 17 +- true ... box.stat.net.CONNECTIONS.current --- @@ -115,6 +115,9 @@ test_run:wait_cond(function() return box.stat.net.CONNECTIONS.current == 1 end, --- - true ... +requests_total_saved = box.stat.net.REQUESTS.total +--- +... future1 = cn:call('tweedledee', {}, {is_async = true}) --- ... @@ -149,9 +152,9 @@ test_run:wait_cond(function() return box.stat.net.REQUESTS.current == 0 end, WAI --- - true ... -box.stat.net.REQUESTS.total +box.stat.net.REQUESTS.total - requests_total_saved == 2 --- -- 19 +- true ... -- reset box.stat.reset() diff --git a/test/box/stat_net.test.lua b/test/box/stat_net.test.lua index 830deb36b..a1dd324bd 100644 --- a/test/box/stat_net.test.lua +++ b/test/box/stat_net.test.lua @@ -29,7 +29,7 @@ cn.space.tweedledum:select() --small request box.stat.net.SENT.total > 0 box.stat.net.RECEIVED.total > 0 box.stat.net.CONNECTIONS.total -box.stat.net.REQUESTS.total +box.stat.net.REQUESTS.total > 0 box.stat.net.CONNECTIONS.current box.stat.net.REQUESTS.current @@ -41,6 +41,7 @@ test_run:wait_cond(function() return box.stat.net.CONNECTIONS.current == 2 end, cn3:close() test_run:wait_cond(function() return box.stat.net.CONNECTIONS.current == 1 end, WAIT_COND_TIMEOUT) +requests_total_saved = box.stat.net.REQUESTS.total future1 = cn:call('tweedledee', {}, {is_async = true}) test_run:wait_cond(function() return box.stat.net.REQUESTS.current == 1 end, WAIT_COND_TIMEOUT) future2 = cn:call('tweedledee', {}, {is_async = true}) @@ -50,7 +51,7 @@ ch:put(true) future1:wait_result() future2:wait_result() test_run:wait_cond(function() return box.stat.net.REQUESTS.current == 0 end, WAIT_COND_TIMEOUT) -box.stat.net.REQUESTS.total +box.stat.net.REQUESTS.total - requests_total_saved == 2 -- reset box.stat.reset() ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-30 10:58 ` Alexander Turenko @ 2019-07-30 12:08 ` Konstantin Osipov 2019-07-30 20:18 ` Vladislav Shpilevoy 1 sibling, 0 replies; 10+ messages in thread From: Konstantin Osipov @ 2019-07-30 12:08 UTC (permalink / raw) To: Alexander Turenko Cc: Vladislav Shpilevoy, Roman Khabibov, tarantool-patches, Kirill Yukhin * Alexander Turenko <alexander.turenko@tarantool.org> [19/07/30 15:04]: LGTM, Vlad please take another look. -- Konstantin Osipov, Moscow, Russia ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-30 10:58 ` Alexander Turenko 2019-07-30 12:08 ` Konstantin Osipov @ 2019-07-30 20:18 ` Vladislav Shpilevoy 2019-07-30 22:35 ` Alexander Turenko 1 sibling, 1 reply; 10+ messages in thread From: Vladislav Shpilevoy @ 2019-07-30 20:18 UTC (permalink / raw) To: Alexander Turenko Cc: Roman Khabibov, tarantool-patches, Konstantin Osipov, Kirill Yukhin Hi! Thanks for the patch! > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua > index 2892bb3d3..31a8c16b7 100644 > --- a/src/box/lua/net_box.lua > +++ b/src/box/lua/net_box.lua > @@ -754,6 +768,8 @@ local function create_transport(host, port, user, password, callback, > if err then return error_sm(err, hdr) end > dispatch_response_iproto(hdr, body_rpos, body_end) > local id = hdr[IPROTO_SYNC_KEY] > + -- trick: omit check for peer_has_vcollation: id is > + -- not nil Nit: usually we start comments with a capital letter and finish with a dot. Here and in other hunks. Other than that the patch LGTM. > if id == select1_id or id == select2_id or id == select3_id then > -- response to a schema query we've submitted > local status = hdr[IPROTO_STATUS_KEY] > diff --git a/test/box/net.box.result b/test/box/net.box.result > index 1d9f43400..e3dabf7d9 100644 > --- a/test/box/net.box.result > +++ b/test/box/net.box.result > @@ -2905,18 +2905,41 @@ c = net:connect(box.cfg.listen) > +#parts == 1 > +--- > +- true > +... > +parts[1].fieldno == 1 > +--- > +- true > +... > +parts[1].type == 'string' > +--- > +- true > +... > +parts[1].is_nullable == false > +--- > +- true > +... > +if _TARANTOOL >= '2.2.1' then \ > + return parts[1].collation == 'test' \ > +else \ > + return parts[1].collation_id == collation_id \ I like this new test-run feature with '\', looks way better than setopt delimited, and looks like C. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH] net.box: ignore absence of _vcollation 2019-07-30 20:18 ` Vladislav Shpilevoy @ 2019-07-30 22:35 ` Alexander Turenko 0 siblings, 0 replies; 10+ messages in thread From: Alexander Turenko @ 2019-07-30 22:35 UTC (permalink / raw) To: Kirill Yukhin Cc: Roman Khabibov, tarantool-patches, Konstantin Osipov, Vladislav Shpilevoy Kirill, I have acquired LGTMs from Kostja and Vlad (with the tiny comment below, which I had fixed). Can you proceed with the change? https://github.com/tarantool/tarantool/issues/4307 https://github.com/tarantool/tarantool/tree/Totktonada/gh-4307-net-box-fix-schema-fetch WBR, Alexander Turenko. > > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua > > index 2892bb3d3..31a8c16b7 100644 > > --- a/src/box/lua/net_box.lua > > +++ b/src/box/lua/net_box.lua > > @@ -754,6 +768,8 @@ local function create_transport(host, port, user, password, callback, > > if err then return error_sm(err, hdr) end > > dispatch_response_iproto(hdr, body_rpos, body_end) > > local id = hdr[IPROTO_SYNC_KEY] > > + -- trick: omit check for peer_has_vcollation: id is > > + -- not nil > > Nit: usually we start comments with a capital letter and finish > with a dot. Here and in other hunks. Other than that the patch > LGTM. I'm aware of that. I gave glance to comments around and they are formatted from a small letter and w/o a period, so I formatted the new code in the same way to make reading more comfortable. Now hovewer I see that there is no consistency in this point across net_box.lua file. So I agree with you: it worth to write new comments in our standard way. Applied an incremental diff at end of the email. I have applied the diff, rebased the branch upward current master and force-pushed the branch. > > +if _TARANTOOL >= '2.2.1' then \ > > + return parts[1].collation == 'test' \ > > +else \ > > + return parts[1].collation_id == collation_id \ > > I like this new test-run feature with '\', looks way better > than setopt delimited, and looks like C. Thanks! ---- diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua index 31a8c16b7..ea169637d 100644 --- a/src/box/lua/net_box.lua +++ b/src/box/lua/net_box.lua @@ -743,7 +743,7 @@ local function create_transport(host, port, user, password, callback, set_state('active') return iproto_sm(schema_version) end - -- _vcollation view was added in 2.2.0-389-g3e3ef182f + -- _vcollation view was added in 2.2.0-389-g3e3ef182f. local peer_has_vcollation = version_at_least(greeting.version_id, 2, 2, 1) local select1_id = new_request_id() @@ -768,8 +768,8 @@ local function create_transport(host, port, user, password, callback, if err then return error_sm(err, hdr) end dispatch_response_iproto(hdr, body_rpos, body_end) local id = hdr[IPROTO_SYNC_KEY] - -- trick: omit check for peer_has_vcollation: id is - -- not nil + -- Trick: omit check for peer_has_vcollation: id is + -- not nil. if id == select1_id or id == select2_id or id == select3_id then -- response to a schema query we've submitted local status = hdr[IPROTO_STATUS_KEY] @@ -791,7 +791,7 @@ local function create_transport(host, port, user, password, callback, end until response[select1_id] and response[select2_id] and (not peer_has_vcollation or response[select3_id]) - -- trick: response[select3_id] is nil when the key is nil + -- Trick: response[select3_id] is nil when the key is nil. callback('did_fetch_schema', schema_version, response[select1_id], response[select2_id], response[select3_id]) set_state('active') @@ -1286,8 +1286,8 @@ function remote_methods:_install_schema(schema_version, spaces, indices, local pkcollationid = index[PARTS][k].collation local pktype = index[PARTS][k][2] or index[PARTS][k].type local pkfield = index[PARTS][k][1] or index[PARTS][k].field - -- resolve a collation name if a peer has - -- _vcollation view + -- Resolve a collation name if a peer has + -- _vcollation view. local pkcollation = nil if pkcollationid ~= nil and collations ~= nil then pkcollation = collations[pkcollationid + 1][2] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-07-30 22:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-08 12:13 [tarantool-patches] [PATCH] net.box: ignore absence of _vcollation Roman Khabibov 2019-07-12 9:55 ` [tarantool-patches] " Kirill Yukhin 2019-07-19 12:16 ` Roman Khabibov 2019-07-20 19:07 ` Alexander Turenko 2019-07-24 21:27 ` Alexander Turenko 2019-07-24 22:43 ` Vladislav Shpilevoy 2019-07-30 10:58 ` Alexander Turenko 2019-07-30 12:08 ` Konstantin Osipov 2019-07-30 20:18 ` Vladislav Shpilevoy 2019-07-30 22:35 ` Alexander Turenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox