From: Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH] net.box: drop conn:timeout() Date: Mon, 26 Jul 2021 13:58:28 +0300 [thread overview] Message-ID: <e6bf1f3347bce8ebc07a66279e2eb44fc8b11edc.1627297005.git.vdavydov@tarantool.org> (raw) conn:timeout(timeout) writes (now + timeout) to conn._deadlines table keyed by fiber.self(). The deadline will then be used by all conn methods unless specified explicitly in the options argument. Usage of fiber.self() degrades net.box performance by about 15% in case timeout is not specified, because it is a C call and so interferes with JIT. See the ticket for the test details. Since the feature is unusable and was deprecated more than four years ago (in 1.7.4) in favor of specifying timeout in the options argument (see commit 58da206c33197), it should be fine to drop it. Closes #6242 --- .../gh-6242-net-box-drop-conn-timeout.md | 9 ++++ src/box/lua/net_box.lua | 26 ---------- .../net.box_count_inconsistent_gh-3262.result | 49 ------------------- ...et.box_count_inconsistent_gh-3262.test.lua | 20 -------- 4 files changed, 9 insertions(+), 95 deletions(-) create mode 100644 changelogs/unreleased/gh-6242-net-box-drop-conn-timeout.md diff --git a/changelogs/unreleased/gh-6242-net-box-drop-conn-timeout.md b/changelogs/unreleased/gh-6242-net-box-drop-conn-timeout.md new file mode 100644 index 000000000000..69d12fb858e5 --- /dev/null +++ b/changelogs/unreleased/gh-6242-net-box-drop-conn-timeout.md @@ -0,0 +1,9 @@ +## feature/core + +* **[Breaking change]** timeout() method of net.box connection, which was + marked deprecated more than four years ago (in 1.7.4) was dropped, because + it negatively affected performance of hot net.box methods, like call() and + select(), in case those are called without specifying a timeout. + +--- +Breaking change: timeout() method of net.box connection was dropped. diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua index 3878abf21914..15e6c093cc15 100644 --- a/src/box/lua/net_box.lua +++ b/src/box/lua/net_box.lua @@ -1047,9 +1047,6 @@ local function new_sm(host, port, opts, connection, greeting) setmetatable(remote, console_mt) else setmetatable(remote, remote_mt) - -- @deprecated since 1.7.4 - remote._deadlines = setmetatable({}, {__mode = 'k'}) - remote._space_mt = space_metatable(remote) remote._index_mt = index_metatable(remote) if opts.call_16 then @@ -1176,17 +1173,11 @@ function remote_methods:_request(method, opts, request_ctx, ...) return res end if opts.timeout then - -- conn.space:request(, { timeout = timeout }) deadline = fiber_clock() + opts.timeout - else - -- conn:timeout(timeout).space:request() - -- @deprecated since 1.7.4 - deadline = self._deadlines[fiber_self()] end on_push = opts.on_push or on_push_sync_default on_push_ctx = opts.on_push_ctx else - deadline = self._deadlines[fiber_self()] on_push = on_push_sync_default end -- Execute synchronous request. @@ -1289,28 +1280,11 @@ end function remote_methods:wait_state(state, timeout) check_remote_arg(self, 'wait_state') - if timeout == nil then - local deadline = self._deadlines[fiber_self()] - timeout = deadline and max(0, deadline - fiber_clock()) - end return self._transport.wait_state(state, timeout) end local compat_warning_said = false --- @deprecated since 1.7.4 -function remote_methods:timeout(timeout) - check_remote_arg(self, 'timeout') - if not compat_warning_said then - compat_warning_said = true - log.warn("netbox:timeout(timeout) is deprecated since 1.7.4, ".. - "please use space:<request>(..., {timeout = t}) instead.") - end - -- Sic: this is broken by design - self._deadlines[fiber_self()] = (timeout and fiber_clock() + timeout) - return self -end - function remote_methods:_install_schema(schema_version, spaces, indices, collations) local sl, space_mt, index_mt = {}, self._space_mt, self._index_mt diff --git a/test/box/net.box_count_inconsistent_gh-3262.result b/test/box/net.box_count_inconsistent_gh-3262.result index c77677625d97..286f9ded8aea 100644 --- a/test/box/net.box_count_inconsistent_gh-3262.result +++ b/test/box/net.box_count_inconsistent_gh-3262.result @@ -427,55 +427,6 @@ cn:eval('return ret_after(...)', {1.00}, { timeout = 1e-9 }) --- - error: Timeout exceeded ... --- --- :timeout() --- @deprecated since 1.7.4 --- -cn:timeout(1).space.net_box_test_space.index.primary:select{234} ---- -- - [234, 1, 2, 3] -... -cn:call('ret_after', {.01}) ---- -- [[0.01]] -... -cn:timeout(1):call('ret_after', {.01}) ---- -- [[0.01]] -... -cn:timeout(.01):call('ret_after', {1}) ---- -- error: Timeout exceeded -... -cn = remote:timeout(0.0000000001):connect(LISTEN.host, LISTEN.service, { user = 'netbox', password = '123' }) ---- -... -cn:close() ---- -... -cn = remote:timeout(1):connect(LISTEN.host, LISTEN.service, { user = 'netbox', password = '123' }) ---- -... -remote.self:ping() ---- -- true -... -remote.self.space.net_box_test_space:select{234} ---- -- - [234, 1, 2, 3] -... -remote.self:timeout(123).space.net_box_test_space:select{234} ---- -- - [234, 1, 2, 3] -... -remote.self:is_connected() ---- -- true -... -remote.self:wait_connected() ---- -- true -... cn:close() --- ... diff --git a/test/box/net.box_count_inconsistent_gh-3262.test.lua b/test/box/net.box_count_inconsistent_gh-3262.test.lua index e84e85cf3829..a6d87cdcc830 100644 --- a/test/box/net.box_count_inconsistent_gh-3262.test.lua +++ b/test/box/net.box_count_inconsistent_gh-3262.test.lua @@ -155,26 +155,6 @@ cn:call('ret_after', {1.00}, { timeout = 1e-9 }) cn:eval('return ret_after(...)', {0.01}, { timeout = 1.00 }) cn:eval('return ret_after(...)', {1.00}, { timeout = 1e-9 }) --- --- :timeout() --- @deprecated since 1.7.4 --- - -cn:timeout(1).space.net_box_test_space.index.primary:select{234} -cn:call('ret_after', {.01}) -cn:timeout(1):call('ret_after', {.01}) -cn:timeout(.01):call('ret_after', {1}) - -cn = remote:timeout(0.0000000001):connect(LISTEN.host, LISTEN.service, { user = 'netbox', password = '123' }) -cn:close() -cn = remote:timeout(1):connect(LISTEN.host, LISTEN.service, { user = 'netbox', password = '123' }) - -remote.self:ping() -remote.self.space.net_box_test_space:select{234} -remote.self:timeout(123).space.net_box_test_space:select{234} -remote.self:is_connected() -remote.self:wait_connected() - cn:close() -- cleanup database after tests space:drop() -- 2.25.1
next reply other threads:[~2021-07-26 10:58 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-26 10:58 Vladimir Davydov via Tarantool-patches [this message] 2021-07-26 22:37 ` Vladislav Shpilevoy via Tarantool-patches 2021-07-27 10:06 ` Vladimir Davydov via Tarantool-patches 2021-07-27 21:27 ` Vladislav Shpilevoy via Tarantool-patches 2021-07-29 8:05 ` Vladimir Davydov via Tarantool-patches
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=e6bf1f3347bce8ebc07a66279e2eb44fc8b11edc.1627297005.git.vdavydov@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=vdavydov@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] net.box: drop conn:timeout()' \ /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