pool: handle disconnection with Opts.Reconnect

The ConnectionPool didn't close a connection if it couldn't get
a role when checking the connection.

Closes #272

Author: oleg-jukovec

CHANGELOG.md

@@ -21,6 +21,8 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.
 ### Fixed
 
 - Several non-critical data race issues (#218)
+- ConnectionPool does not properly handle disconnection with Opts.Reconnect
+  set (#272)
 
 ## [1.10.0] - 2022-12-31
 

connection_pool/connection_pool.go

@@ -163,6 +163,10 @@ func ConnectWithOpts(addrs []string, connOpts tarantool.Opts, opts OptsPool) (co
 }
 
 // ConnectWithOpts creates pool for instances with addresses addrs.
+//
+// It is useless to set up tarantool.Opts.Reconnect value for a connection.
+// The connection pool has its own reconnection logic. See
+// OptsPool.CheckTimeout description.
 func Connect(addrs []string, connOpts tarantool.Opts) (connPool *ConnectionPool, err error) {
 	opts := OptsPool{
 		CheckTimeout: 1 * time.Second,
@@ -977,10 +981,18 @@ func (pool *ConnectionPool) updateConnection(s connState) connState {
 			}
 			s.role = role
 		}
-	}
+		pool.poolsMutex.Unlock()
+		return s
+	} else {
+		pool.deleteConnection(s.addr)
+		pool.poolsMutex.Unlock()
 
-	pool.poolsMutex.Unlock()
-	return s
+		s.conn.Close()
+		pool.handlerDeactivated(s.conn, s.role)
+		s.conn = nil
+		s.role = UnknownRole
+		return s
+	}
 }
 
 func (pool *ConnectionPool) tryConnect(s connState) connState {

connection_pool/connection_pool_test.go

@@ -128,7 +128,8 @@ func TestReconnect(t *testing.T) {
 		},
 	}
 
-	err = test_helpers.Retry(test_helpers.CheckPoolStatuses, args, defaultCountRetry, defaultTimeoutRetry)
+	err = test_helpers.Retry(test_helpers.CheckPoolStatuses,
+		args, defaultCountRetry, defaultTimeoutRetry)
 	require.Nil(t, err)
 
 	err = test_helpers.RestartTarantool(&instances[0])
@@ -144,6 +145,51 @@ func TestReconnect(t *testing.T) {
 		},
 	}
 
+	err = test_helpers.Retry(test_helpers.CheckPoolStatuses,
+		args, defaultCountRetry, defaultTimeoutRetry)
+	require.Nil(t, err)
+}
+
+func TestDisconnect_withReconnect(t *testing.T) {
+	const serverId = 0
+
+	opts := connOpts
+	opts.Reconnect = 10 * time.Second
+
+	connPool, err := connection_pool.Connect([]string{servers[serverId]}, opts)
+	require.Nilf(t, err, "failed to connect")
+	require.NotNilf(t, connPool, "conn is nil after Connect")
+
+	defer connPool.Close()
+
+	// Test.
+	test_helpers.StopTarantoolWithCleanup(instances[serverId])
+	args := test_helpers.CheckStatusesArgs{
+		ConnPool:           connPool,
+		Mode:               connection_pool.ANY,
+		Servers:            []string{servers[serverId]},
+		ExpectedPoolStatus: false,
+		ExpectedStatuses: map[string]bool{
+			servers[serverId]: false,
+		},
+	}
+	err = test_helpers.Retry(test_helpers.CheckPoolStatuses, args, defaultCountRetry, defaultTimeoutRetry)
+	require.Nil(t, err)
+
+	// Restart the server after success.
+	err = test_helpers.RestartTarantool(&instances[serverId])
+	require.Nilf(t, err, "failed to restart tarantool")
+
+	args = test_helpers.CheckStatusesArgs{
+		ConnPool:           connPool,
+		Mode:               connection_pool.ANY,
+		Servers:            []string{servers[serverId]},
+		ExpectedPoolStatus: true,
+		ExpectedStatuses: map[string]bool{
+			servers[serverId]: true,
+		},
+	}
+
 	err = test_helpers.Retry(test_helpers.CheckPoolStatuses, args, defaultCountRetry, defaultTimeoutRetry)
 	require.Nil(t, err)
 }