GODRIVER-2218 Bump connection idle deadline on pool checkIn. (#869)

Author: matthewdale

x/mongo/driver/topology/cmap_prose_test.go

@@ -186,40 +186,6 @@ func TestCMAPProse(t *testing.T) {
 				assert.Equal(t, event.ReasonError, evt.Reason, "expected reason %q, got %q",
 					event.ReasonError, evt.Reason)
 			})
-			t.Run("expired connection", func(t *testing.T) {
-				// If the connection being returned to the pool is expired, it should be removed from the pool and an
-				// event should be published.
-				clearEvents()
-
-				var dialer DialerFunc = func(context.Context, string, string) (net.Conn, error) {
-					return &testNetConn{}, nil
-				}
-
-				// We don't use the WithHandshaker option so the connection won't error during handshaking.
-				// WithIdleTimeout must be used because the connection.idleTimeoutExpired() function only checks the
-				// deadline if the idleTimeout option is greater than 0.
-				connOpts := []ConnectionOption{
-					WithDialer(func(Dialer) Dialer { return dialer }),
-					WithIdleTimeout(func(time.Duration) time.Duration { return 1 * time.Second }),
-				}
-				pool := createTestPool(t, getConfig(), connOpts...)
-				defer pool.close(context.Background())
-
-				conn, err := pool.checkOut(context.Background())
-				assert.Nil(t, err, "checkOut() error: %v", err)
-
-				// Set the idleDeadline to a time in the past to simulate expiration.
-				pastTime := time.Now().Add(-10 * time.Second)
-				conn.idleDeadline.Store(pastTime)
-
-				err = pool.checkIn(conn)
-				assert.Nil(t, err, "checkIn() error: %v", err)
-
-				assertConnectionCounts(t, pool, 1, 1)
-				evt := <-closed
-				assert.Equal(t, event.ReasonIdle, evt.Reason, "expected reason %q, got %q",
-					event.ReasonIdle, evt.Reason)
-			})
 		})
 		t.Run("close", func(t *testing.T) {
 			t.Run("connections returned gracefully", func(t *testing.T) {

x/mongo/driver/topology/connection.go

@@ -208,8 +208,6 @@ func (c *connection) connect(ctx context.Context) (err error) {
 		c.nc = tlsNc
 	}
 
-	c.bumpIdleDeadline()
-
 	// running hello and authentication is handled by a handshaker on the configuration instance.
 	handshaker := c.config.handshaker
 	if handshaker == nil {
@@ -364,7 +362,6 @@ func (c *connection) writeWireMessage(ctx context.Context, wm []byte) error {
 		}
 	}
 
-	c.bumpIdleDeadline()
 	return nil
 }
 
@@ -429,7 +426,6 @@ func (c *connection) readWireMessage(ctx context.Context, dst []byte) ([]byte, e
 		}
 	}
 
-	c.bumpIdleDeadline()
 	return dst, nil
 }
 

x/mongo/driver/topology/pool.go

@@ -596,6 +596,14 @@ func (p *pool) checkInNoEvent(conn *connection) error {
 		return ErrWrongPool
 	}
 
+	// Bump the connection idle deadline here because we're about to make the connection "available".
+	// The idle deadline is used to determine when a connection has reached its max idle time and
+	// should be closed. A connection reaches its max idle time when it has been "available" in the
+	// idle connections stack for more than the configured duration (maxIdleTimeMS). Set it before
+	// we call connectionPerished(), which checks the idle deadline, because a newly "available"
+	// connection should never be perished due to max idle time.
+	conn.bumpIdleDeadline()
+
 	if reason, perished := connectionPerished(conn); perished {
 		_ = p.removeConnection(conn, reason)
 		go func() {

x/mongo/driver/topology/pool_test.go

@@ -840,6 +840,75 @@ func TestPool(t *testing.T) {
 			p1.close(context.Background())
 			p2.close(context.Background())
 		})
+		t.Run("bumps the connection idle deadline", func(t *testing.T) {
+			t.Parallel()
+
+			cleanup := make(chan struct{})
+			defer close(cleanup)
+			addr := bootstrapConnections(t, 1, func(nc net.Conn) {
+				<-cleanup
+				_ = nc.Close()
+			})
+
+			d := newdialer(&net.Dialer{})
+			p := newPool(poolConfig{
+				Address:     address.Address(addr.String()),
+				MaxIdleTime: 100 * time.Millisecond,
+			}, WithDialer(func(Dialer) Dialer { return d }))
+			err := p.ready()
+			noerr(t, err)
+			defer p.close(context.Background())
+
+			c, err := p.checkOut(context.Background())
+			noerr(t, err)
+
+			// Sleep for 110ms, which will exceed the 100ms connection idle timeout. Then check the
+			// connection back in and expect that it is not closed because checkIn() should bump the
+			// connection idle deadline.
+			time.Sleep(110 * time.Millisecond)
+			err = p.checkIn(c)
+			noerr(t, err)
+
+			assert.Equalf(t, 0, d.lenclosed(), "should have closed 0 connections")
+			assert.Equalf(t, 1, p.availableConnectionCount(), "should have 1 idle connections in pool")
+			assert.Equalf(t, 1, p.totalConnectionCount(), "should have 1 total connection in pool")
+		})
+		t.Run("sets minPoolSize connection idle deadline", func(t *testing.T) {
+			t.Parallel()
+
+			cleanup := make(chan struct{})
+			defer close(cleanup)
+			addr := bootstrapConnections(t, 4, func(nc net.Conn) {
+				<-cleanup
+				_ = nc.Close()
+			})
+
+			d := newdialer(&net.Dialer{})
+			p := newPool(poolConfig{
+				Address:     address.Address(addr.String()),
+				MinPoolSize: 3,
+				MaxIdleTime: 10 * time.Millisecond,
+			}, WithDialer(func(Dialer) Dialer { return d }))
+			err := p.ready()
+			noerr(t, err)
+			defer p.close(context.Background())
+
+			// Wait for maintain() to open 3 connections.
+			assertConnectionsOpened(t, d, 3)
+
+			// Sleep for 100ms, which will exceed the 10ms connection idle timeout, then try to check
+			// out a connection. Expect that all minPoolSize connections checked into the pool by
+			// maintain() have passed their idle deadline, so checkOut() closes all 3 connections
+			// and tries to create a new connection.
+			time.Sleep(100 * time.Millisecond)
+			_, err = p.checkOut(context.Background())
+			noerr(t, err)
+
+			assertConnectionsClosed(t, d, 3)
+			assert.Equalf(t, 4, d.lenopened(), "should have opened 4 connections")
+			assert.Equalf(t, 0, p.availableConnectionCount(), "should have 0 idle connections in pool")
+			assert.Equalf(t, 1, p.totalConnectionCount(), "should have 1 total connection in pool")
+		})
 	})
 	t.Run("maintain", func(t *testing.T) {
 		t.Parallel()