Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: vdavydov.dev@gmail.com
Subject: [PATCH 1/1] netbox: fix wait_connected ignorance
Date: Wed,  5 Dec 2018 18:06:39 +0300	[thread overview]
Message-ID: <c20a920cf9fd4294ab5ac062c4e1a95746419ba2.1544022329.git.v.shpilevoy@tarantool.org> (raw)

After this patch d2468dacaf it became possible to
wrap an existing connection into netbox API. A regular
netbox.connect function was refactored so as to reuse
connection establishment code.

But connection should be established in a worker
fiber, not in a caller's one. Otherwise it is
impossible to do not wait for connect result.

The patch just moves connection establishment into a
worker fiber, without any functional changes.

Closes #3856
---
https://github.com/tarantool/tarantool/tree/gerold103/gh-3856-netbox-ignores-wait-connected
https://github.com/tarantool/tarantool/issues/3856

 src/box/lua/net_box.lua   | 34 ++++++++++++++--------------------
 test/box/net.box.result   | 17 +++++++++++++++++
 test/box/net.box.test.lua |  7 +++++++
 3 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index fd6ebf9de..d54b3e7d9 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -419,21 +419,21 @@ local function create_transport(host, port, user, password, callback,
 
     local function start()
         if state ~= 'initial' then return not is_final_state[state] end
-        if not connection and not callback('reconnect_timeout') then
-            set_state('error', E_NO_CONNECTION)
-            return
-        end
         fiber.create(function()
             local ok, err
             worker_fiber = fiber_self()
             fiber.name(string.format('%s:%s (net.box)', host, port), {truncate=true})
-            -- It is possible, if the first connection attempt had
-            -- been failed, but reconnect timeout is set. In such
-            -- a case the worker must be run, and immediately
-            -- start reconnecting.
             if not connection then
-                set_state('error_reconnect', E_NO_CONNECTION, greeting)
-                goto do_reconnect
+                local tm = callback('fetch_connect_timeout')
+                connection, greeting = establish_connection(host, port, tm)
+                if not connection then
+                    if not callback('reconnect_timeout') then
+                        set_state('error', E_NO_CONNECTION, greeting)
+                        return
+                    end
+                    set_state('error_reconnect', E_NO_CONNECTION, greeting)
+                    goto do_reconnect
+                end
             end
     ::handle_connection::
             ok, err = pcall(protocol_sm)
@@ -472,7 +472,9 @@ local function create_transport(host, port, user, password, callback,
             set_state('closed', E_NO_CONNECTION, 'Connection closed')
         end
         if worker_fiber then
-            worker_fiber:cancel()
+            if worker_fiber:status() ~= 'dead' then
+                worker_fiber:cancel()
+            end
             worker_fiber = nil
         end
     end
@@ -990,15 +992,7 @@ end
 -- @retval Net.box object.
 --
 local function connect(...)
-    local host, port, opts = parse_connect_params(...)
-    local connection, greeting =
-        establish_connection(host, port, opts.connect_timeout)
-    if not connection then
-        local dummy_conn = new_sm(host, port, opts)
-        dummy_conn.error = greeting
-        return dummy_conn
-    end
-    return new_sm(host, port, opts, connection, greeting)
+    return new_sm(parse_connect_params(...))
 end
 
 local function check_remote_arg(remote, method)
diff --git a/test/box/net.box.result b/test/box/net.box.result
index 6e59d0bc0..41d758679 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3404,6 +3404,23 @@ ok, err
 - false
 - Connection closed
 ...
+--
+-- gh-3856: wait_connected = false is ignored.
+--
+c = net.connect('8.8.8.8:123456', {wait_connected = false})
+---
+...
+c
+---
+- opts:
+    wait_connected: false
+  host: 8.8.8.8
+  state: initial
+  port: '123456'
+...
+c:close()
+---
+...
 box.schema.func.drop('do_long')
 ---
 ...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 26773aac9..2945d4aca 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1373,6 +1373,13 @@ ready = true
 while not err do fiber.sleep(0.01) end
 ok, err
 
+--
+-- gh-3856: wait_connected = false is ignored.
+--
+c = net.connect('8.8.8.8:123456', {wait_connected = false})
+c
+c:close()
+
 box.schema.func.drop('do_long')
 box.schema.user.revoke('guest', 'write', 'space', '_schema')
 box.schema.user.revoke('guest', 'read,write', 'space', '_space')
-- 
2.17.2 (Apple Git-113)

             reply	other threads:[~2018-12-05 15:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05 15:06 Vladislav Shpilevoy [this message]
2018-12-06 17:46 ` Vladimir Davydov
2018-12-06 22:45   ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-09 11:16     ` Vladimir Davydov

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=c20a920cf9fd4294ab5ac062c4e1a95746419ba2.1544022329.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH 1/1] netbox: fix wait_connected ignorance' \
    /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