socket: Don't cleanup encrypted transport in socket_connect()

..instead cleanup only in socket_poller()

With commit d117466 socket_poller() wasn't launched from socket_connect
(for encrypted connections), if connect() failed. This was done to
prevent the socket private data from being double unreffed, from the
cleanups in both socket_poller() and socket_connect(). This allowed
future reconnects to happen successfully.

If a socket reconnects is sort of decided by the rpc notify function
registered. The above change worked with glusterd, as the glusterd rpc
notify function (glusterd_peer_rpc_notify()) continuously allowed
reconnects on failure.

mgmt_rpc_notify(), the rpc notify function in glusterfsd, behaves
differently.

For a DISCONNECT event, if more volfile servers are available or if more
addresses are available in the dns cache, it allows reconnects. If not
it terminates the program.

For a CONNECT event, it attempts to do a volfile fetch rpc request. If
sending this rpc fails, it immediately terminates the program.

One side effect of commit d117466, was that the encrypted socket was
registered with epoll, unintentionally, on a connect failure.  A weird
thing happens because of this. The epoll notifier notifies
mgmt_rpc_notify() of a CONNECT event, instead of a DISCONNECT as
expected. This causes mgmt_rpc_notify() to attempt an unsuccessful
volfile fetch rpc request, and terminate.
(I still don't know why the epoll raises the CONNECT event)

Commit 46bd29e fixed some issues with IPv6 in GlusterFS. This caused
address resolution in GlusterFS to also request of IPv6 addresses
(AF_UNSPEC) instead of just IPv4. On most systems, this causes the IPv6
addresses to be returned first.

GlusterD listens on 0.0.0.0:24007 by default. While this attaches to all
interfaces, it only listens on IPv4 addresses. GlusterFS daemons and
bricks are given 'localhost' as the volfile server. This resolves to
'::1' as the first address.

When using management encryption, the above reasons cause the daemon
processes to fail to fetch volfiles and terminate.

Solution
--------
The solution to this is simple. Instead of cleaning up the encrypted
socket in socket_connect(), launch socket_poller() and let it cleanup
the socket instead. This prevents the unintentional registration with
epoll, and socket_poller() sends the correct events to the rpc notify
functions, which allows proper reconnects to happen.

Change-Id: Idb0c0a828743cccca51cfdd1aa6458cfa0a9d100
Signed-off-by: Kaushal M <kaushal@redhat.com>
Reviewed-on: http://review.gluster.org/13926
Smoke: Gluster Build System <jenkins@build.gluster.com>
Tested-by: Jeff Darcy <jdarcy@redhat.com>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>

1 files changed, 6 insertions, 12 deletions

diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c
index f00657bde4c..ac95540def5 100644
--- a/rpc/rpc-transport/socket/src/socket.c
+++ b/rpc/rpc-transport/socket/src/socket.c
@@ -3052,17 +3052,11 @@ socket_connect (rpc_transport_t *this, int port)
 
 handler:
                 if (ret < 0) {
-                        if (priv->own_thread) {
-                                sys_close (priv->sock);
-                                priv->sock = -1;
-                        }
-                        else {
-                                /* Ignore error from connect. epoll events
-                                   should be handled in the socket handler.
-                                   shutdown(2) will result in EPOLLERR, so
-                                   cleanup is done in socket_event_handler */
-                                shutdown (priv->sock, SHUT_RDWR);
-                        }
+                        /* Ignore error from connect. epoll events
+                           should be handled in the socket handler.  shutdown(2)
+                           will result in EPOLLERR, so cleanup is done in
+                           socket_event_handler or socket_poller */
+                        shutdown (priv->sock, SHUT_RDWR);
                 }
 
                 /*
@@ -3074,7 +3068,7 @@ handler:
                 rpc_transport_ref (this);
                 refd = _gf_true;
 
-                if (!ret && priv->own_thread) {
+                if (priv->own_thread) {
                         if (pipe(priv->pipe) < 0) {
                                 gf_log(this->name,GF_LOG_ERROR,
                                 "could not create pipe");