From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@dev.tarantool.org, kyukhin@tarantool.org Subject: [Tarantool-patches] [PATCH 1/1] session: remove box.session.push() 'sync' Date: Mon, 4 May 2020 23:42:55 +0200 [thread overview] Message-ID: <d759df1e9bd61e1df88a8302a4bb47aac7c65361.1588628534.git.v.shpilevoy@tarantool.org> (raw) Closes #4689 @TarantoolBot document Title: box.session.push() 'sync' is deprecated box.session.push() had two parameters - data to push and 'sync'. The sync was a request ID with which the out of bound data should be pushed into a socket. This was introduced as a workaround for #3450, and is useless since its resolution. A user anyway can't push to different sessions, where that parameter could be useful. And pushing into requests of the same session, on the contrary, is something not really needed anywhere, not portable to non-binary session types (console, background), and is just dangerous since it is easy to add a bug here. The patch removes the parameter. Now there will be thrown a 'Usage' error at attempt to use 'sync' parameter. In version 2.4 it is deprecated, prints warnings into logs, but still works. In 2.5 it is removed completely. --- Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4689-push-sync-remove-2.5 Issue: https://github.com/tarantool/tarantool/issues/4689 The commit is for master branch only. src/box/box.cc | 2 +- src/box/iproto.cc | 6 +++--- src/box/lua/console.c | 4 +--- src/box/lua/session.c | 20 +++----------------- src/box/session.cc | 6 ++---- src/box/session.h | 10 ++++------ test/app-tap/console.test.lua | 10 +--------- test/box/push.result | 18 ++++++++---------- test/box/push.test.lua | 14 ++++++-------- 9 files changed, 29 insertions(+), 61 deletions(-) diff --git a/src/box/box.cc b/src/box/box.cc index ae0907e0f..8b58e6859 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -1470,7 +1470,7 @@ box_session_push(const char *data, const char *data_end) struct port_msgpack port; struct port *base = (struct port *)&port; port_msgpack_create(base, data, data_end - data); - int rc = session_push(session, session_sync(session), base); + int rc = session_push(session, base); port_msgpack_destroy(base); return rc; } diff --git a/src/box/iproto.cc b/src/box/iproto.cc index 9dad43b0b..51fe7d93b 100644 --- a/src/box/iproto.cc +++ b/src/box/iproto.cc @@ -2153,7 +2153,6 @@ tx_end_push(struct cmsg *m) /** * Push a message from @a port to a remote client. * @param session iproto session. - * @param sync Request sync in scope of which to send the push. * @param port Port with data to send. * * @retval -1 Memory error. @@ -2162,7 +2161,7 @@ tx_end_push(struct cmsg *m) * client: the output buffer is flushed asynchronously. */ static int -iproto_session_push(struct session *session, uint64_t sync, struct port *port) +iproto_session_push(struct session *session, struct port *port) { struct iproto_connection *con = (struct iproto_connection *) session->meta.connection; @@ -2173,7 +2172,8 @@ iproto_session_push(struct session *session, uint64_t sync, struct port *port) obuf_rollback_to_svp(con->tx.p_obuf, &svp); return -1; } - iproto_reply_chunk(con->tx.p_obuf, &svp, sync, ::schema_version); + iproto_reply_chunk(con->tx.p_obuf, &svp, iproto_session_sync(session), + ::schema_version); if (! con->tx.is_push_sent) tx_begin_push(con); else diff --git a/src/box/lua/console.c b/src/box/lua/console.c index b941f50c6..666151084 100644 --- a/src/box/lua/console.c +++ b/src/box/lua/console.c @@ -510,16 +510,14 @@ port_msgpack_dump_plain(struct port *base, uint32_t *size) * Push a tagged YAML document or a Lua string into a console * socket. * @param session Console session. - * @param sync Unused request sync. * @param port Port with the data to push. * * @retval 0 Success. * @retval -1 Error. */ static int -console_session_push(struct session *session, uint64_t sync, struct port *port) +console_session_push(struct session *session, struct port *port) { - (void) sync; assert(session_vtab_registry[session->type].push == console_session_push); uint32_t text_len; diff --git a/src/box/lua/session.c b/src/box/lua/session.c index 64453463a..0a20aaad1 100644 --- a/src/box/lua/session.c +++ b/src/box/lua/session.c @@ -378,25 +378,11 @@ static int lbox_session_push(struct lua_State *L) { struct session *session = current_session(); - uint64_t sync; - switch(lua_gettop(L)) { - case 1: - sync = session_sync(session); - break; - case 2: - sync = luaL_touint64(L, 2); - if (sync != 0 || (lua_isnumber(L, 2) && - lua_tonumber(L, 2) == 0)) { - lua_pop(L, 1); - break; - } - FALLTHROUGH; - default: - return luaL_error(L, "Usage: box.session.push(data, sync)"); - } + if (lua_gettop(L) != 1) + return luaL_error(L, "Usage: box.session.push(data)"); struct port port; port_lua_create(&port, L); - if (session_push(session, sync, &port) != 0) + if (session_push(session, &port) != 0) return luaT_push_nil_and_error(L); lua_pushboolean(L, true); return 1; diff --git a/src/box/session.cc b/src/box/session.cc index 08a10924a..7085cc393 100644 --- a/src/box/session.cc +++ b/src/box/session.cc @@ -96,10 +96,9 @@ session_on_stop(struct trigger *trigger, void * /* event */) } static int -closed_session_push(struct session *session, uint64_t sync, struct port *port) +closed_session_push(struct session *session, struct port *port) { (void) session; - (void) sync; (void) port; diag_set(ClientError, ER_SESSION_CLOSED); return -1; @@ -354,10 +353,9 @@ access_check_universe(user_access_t access) } int -generic_session_push(struct session *session, uint64_t sync, struct port *port) +generic_session_push(struct session *session, struct port *port) { (void) port; - (void) sync; const char *name = tt_sprintf("Session '%s'", session_type_strs[session->type]); diag_set(ClientError, ER_UNSUPPORTED, name, "push()"); diff --git a/src/box/session.h b/src/box/session.h index 500a88b22..a15434f96 100644 --- a/src/box/session.h +++ b/src/box/session.h @@ -130,15 +130,13 @@ struct session_vtab { * Push a port data into a session data channel - socket, * console or something. * @param session Session to push into. - * @param sync Request sync in scope of which to send the - * push. * @param port Port with data to push. * * @retval 0 Success. * @retval -1 Error. */ int - (*push)(struct session *session, uint64_t sync, struct port *port); + (*push)(struct session *session, struct port *port); /** * Get session file descriptor if exists. * @param session Session to get descriptor from. @@ -345,9 +343,9 @@ access_check_universe_object(user_access_t access, const char *object_name); static inline int -session_push(struct session *session, uint64_t sync, struct port *port) +session_push(struct session *session, struct port *port) { - return session->vtab->push(session, sync, port); + return session->vtab->push(session, port); } static inline int @@ -367,7 +365,7 @@ session_sync(struct session *session) * function always returns -1 and sets ER_UNSUPPORTED error. */ int -generic_session_push(struct session *session, uint64_t sync, struct port *port); +generic_session_push(struct session *session, struct port *port); /** Return -1 from any session. */ int diff --git a/test/app-tap/console.test.lua b/test/app-tap/console.test.lua index 4feadfa5e..d5452aa17 100755 --- a/test/app-tap/console.test.lua +++ b/test/app-tap/console.test.lua @@ -21,7 +21,7 @@ local EOL = "\n...\n" test = tap.test("console") -test:plan(80) +test:plan(78) -- Start console and connect to it local server = console.listen(CONSOLE_SOCKET) @@ -75,14 +75,6 @@ test:is(client:read(';'), '"res";', 'returned result') client:write('\\set output yaml\n') client:read(EOL) --- --- gh-3790: box.session.push support uint64_t sync. --- -client:write('box.session.push(1, 9223372036854775808ULL)\n') -test:is(client:read(EOL), '%TAG !push! tag:tarantool.io/push,2018\n--- 1\n...\n', - "pushed message") -test:is(client:read(EOL), '---\n- true\n...\n', "pushed message") - -- Execute some command client:write("1\n") test:is(yaml.decode(client:read(EOL))[1], 1, "eval") diff --git a/test/box/push.result b/test/box/push.result index 7888e4d92..8a734e261 100644 --- a/test/box/push.result +++ b/test/box/push.result @@ -6,14 +6,15 @@ test_run = require('test_run').new() -- -- -- Usage. +-- gh-4689: sync is deprecated, accepts only 'data' parameter. -- box.session.push() --- -- error: 'Usage: box.session.push(data, sync)' +- error: 'Usage: box.session.push(data)' ... box.session.push(1, 'a') --- -- error: 'Usage: box.session.push(data, sync)' +- error: 'Usage: box.session.push(data)' ... fiber = require('fiber') --- @@ -95,20 +96,18 @@ test_run:cmd("setopt delimiter ';'") - true ... function dml_push_and_dml(key) - local sync = box.session.sync() - box.session.push('started dml', sync) + box.session.push('started dml') s:replace{key} - box.session.push('continued dml', sync) + box.session.push('continued dml') s:replace{-key} - box.session.push('finished dml', sync) + box.session.push('finished dml') return key end; --- ... function do_pushes(val) - local sync = box.session.sync() for i = 1, 5 do - box.session.push(i, sync) + box.session.push(i) fiber.yield() end return val @@ -374,9 +373,8 @@ test_run:cmd("setopt delimiter ';'") - true ... function do_pushes() - local sync = box.session.sync() for i = 1, 5 do - box.session.push(i + 100, sync) + box.session.push(i + 100) cond:wait() end return true diff --git a/test/box/push.test.lua b/test/box/push.test.lua index f4518466e..a66d855a0 100644 --- a/test/box/push.test.lua +++ b/test/box/push.test.lua @@ -5,6 +5,7 @@ test_run = require('test_run').new() -- -- Usage. +-- gh-4689: sync is deprecated, accepts only 'data' parameter. -- box.session.push() box.session.push(1, 'a') @@ -44,18 +45,16 @@ pk = s:create_index('pk') c:reload_schema() test_run:cmd("setopt delimiter ';'") function dml_push_and_dml(key) - local sync = box.session.sync() - box.session.push('started dml', sync) + box.session.push('started dml') s:replace{key} - box.session.push('continued dml', sync) + box.session.push('continued dml') s:replace{-key} - box.session.push('finished dml', sync) + box.session.push('finished dml') return key end; function do_pushes(val) - local sync = box.session.sync() for i = 1, 5 do - box.session.push(i, sync) + box.session.push(i) fiber.yield() end return val @@ -191,9 +190,8 @@ c = netbox.connect(box.cfg.listen) cond = fiber.cond() test_run:cmd("setopt delimiter ';'") function do_pushes() - local sync = box.session.sync() for i = 1, 5 do - box.session.push(i + 100, sync) + box.session.push(i + 100) cond:wait() end return true -- 2.21.1 (Apple Git-122.3)
next reply other threads:[~2020-05-04 21:42 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-04 21:42 Vladislav Shpilevoy [this message] 2020-05-18 21:38 ` Vladislav Shpilevoy 2020-05-19 10:00 ` Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=d759df1e9bd61e1df88a8302a4bb47aac7c65361.1588628534.git.v.shpilevoy@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=kyukhin@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/1] session: remove box.session.push() '\''sync'\''' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox