From ebb4c089005cb3cdb1b7d917233c9a3386302b65 Mon Sep 17 00:00:00 2001
From: Oleg Jukovec <oleg.jukovec@tarantool.org>
Date: Thu, 29 Feb 2024 10:34:54 +0300
Subject: [PATCH] pool: fix notify on remove a connection

ConnectionPool.Remove() does not notify a ConnectionHandler on
an instance removing from the pool.
---
 CHANGELOG.md                 |  3 ++
 pool/connection_pool.go      |  1 +
 pool/connection_pool_test.go | 71 ++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 30aa750f..aecdf37e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,9 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.
 
 ### Fixed
 
+- `ConnectionPool.Remove()` does not notify a `ConnectionHandler` after
+  an instance is already removed from the pool (#385)
+
 ## [2.0.0] - 2024-02-12
 
 There are a lot of changes in the new major version. The main ones:
diff --git a/pool/connection_pool.go b/pool/connection_pool.go
index 9a79f911..b4062c38 100644
--- a/pool/connection_pool.go
+++ b/pool/connection_pool.go
@@ -1374,6 +1374,7 @@ func (p *ConnectionPool) controller(ctx context.Context, e *endpoint) {
 					// we need to start an another one for the shutdown.
 					go func() {
 						e.closeErr = e.conn.CloseGraceful()
+						p.handlerDeactivated(e.name, e.conn, e.role)
 						close(e.closed)
 					}()
 				} else {
diff --git a/pool/connection_pool_test.go b/pool/connection_pool_test.go
index cadd8356..2592c711 100644
--- a/pool/connection_pool_test.go
+++ b/pool/connection_pool_test.go
@@ -1166,6 +1166,77 @@ func TestConnectionHandlerUpdateError(t *testing.T) {
 	require.Nilf(t, err, "failed to get ConnectedNow()")
 }
 
+type testDeactivatedErrorHandler struct {
+	mut         sync.Mutex
+	deactivated []string
+}
+
+func (h *testDeactivatedErrorHandler) Discovered(name string, conn *tarantool.Connection,
+	role pool.Role) error {
+	return nil
+}
+
+func (h *testDeactivatedErrorHandler) Deactivated(name string, conn *tarantool.Connection,
+	role pool.Role) error {
+	h.mut.Lock()
+	defer h.mut.Unlock()
+
+	h.deactivated = append(h.deactivated, name)
+	return nil
+}
+
+func TestConnectionHandlerDeactivated_on_remove(t *testing.T) {
+	poolServers := []string{servers[0], servers[1]}
+	poolInstances := makeInstances(poolServers, connOpts)
+	roles := []bool{false, false}
+
+	err := test_helpers.SetClusterRO(makeDialers(poolServers), connOpts, roles)
+	require.Nilf(t, err, "fail to set roles for cluster")
+
+	h := &testDeactivatedErrorHandler{}
+	poolOpts := pool.Opts{
+		CheckTimeout:      100 * time.Microsecond,
+		ConnectionHandler: h,
+	}
+	ctx, cancel := test_helpers.GetPoolConnectContext()
+	defer cancel()
+	connPool, err := pool.ConnectWithOpts(ctx, poolInstances, poolOpts)
+	require.Nilf(t, err, "failed to connect")
+	require.NotNilf(t, connPool, "conn is nil after Connect")
+	defer connPool.Close()
+
+	args := test_helpers.CheckStatusesArgs{
+		ConnPool:           connPool,
+		Mode:               pool.ANY,
+		Servers:            servers,
+		ExpectedPoolStatus: true,
+		ExpectedStatuses: map[string]bool{
+			servers[0]: true,
+			servers[1]: true,
+		},
+	}
+	err = test_helpers.CheckPoolStatuses(args)
+	require.Nil(t, err)
+
+	for _, server := range poolServers {
+		connPool.Remove(server)
+		connPool.Remove(server)
+	}
+
+	args = test_helpers.CheckStatusesArgs{
+		ConnPool:           connPool,
+		Mode:               pool.ANY,
+		Servers:            servers,
+		ExpectedPoolStatus: false,
+	}
+	err = test_helpers.CheckPoolStatuses(args)
+	require.Nil(t, err)
+
+	h.mut.Lock()
+	defer h.mut.Unlock()
+	require.ElementsMatch(t, poolServers, h.deactivated)
+}
+
 func TestRequestOnClosed(t *testing.T) {
 	server1 := servers[0]
 	server2 := servers[1]