Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 0 additions & 34 deletions x/mongo/driver/topology/cmap_prose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 0 additions & 4 deletions x/mongo/driver/topology/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -364,7 +362,6 @@ func (c *connection) writeWireMessage(ctx context.Context, wm []byte) error {
}
}

c.bumpIdleDeadline()
return nil
}

Expand Down Expand Up @@ -429,7 +426,6 @@ func (c *connection) readWireMessage(ctx context.Context, dst []byte) ([]byte, e
}
}

c.bumpIdleDeadline()
return dst, nil
}

Expand Down
8 changes: 8 additions & 0 deletions x/mongo/driver/topology/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
69 changes: 69 additions & 0 deletions x/mongo/driver/topology/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down