Skip to content

Commit

Permalink
Fix prematurely closing of connections when max timeout is set
Browse files Browse the repository at this point in the history
  • Loading branch information
gillmk authored and Manvinder Gill committed Aug 10, 2018
1 parent 0835618 commit 207715d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 10 deletions.
2 changes: 2 additions & 0 deletions pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ func (c *ClientConn) Close() error {
if c.unhealthy {
wrapper.ClientConn.Close()
wrapper.ClientConn = nil
} else {
wrapper.timeInitiated = c.timeInitiated
}
select {
case c.pool.clients <- wrapper:
Expand Down
30 changes: 20 additions & 10 deletions pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,24 +141,34 @@ func TestMaxLifeDuration(t *testing.T) {
}

// Let's also make sure we don't prematurely close the connection
count := 0
p, err = New(func() (*grpc.ClientConn, error) {
count++
return grpc.Dial("example.com", grpc.WithInsecure())
}, 1, 1, 0, time.Minute)
if err != nil {
t.Errorf("The pool returned an error: %s", err.Error())
}

c, err = p.Get(context.Background())
if err != nil {
t.Errorf("Get returned an error: %s", err.Error())
}
for i := 0; i < 3; i++ {
c, err = p.Get(context.Background())
if err != nil {
t.Errorf("Get returned an error: %s", err.Error())
}

// The max life of the connection was very low (1ns), so when we close
// the connection it should get marked as unhealthy
if err := c.Close(); err != nil {
t.Errorf("Close returned an error: %s", err.Error())
// The max life of the connection is high, so when we close
// the connection it shouldn't be marked as unhealthy
if err := c.Close(); err != nil {
t.Errorf("Close returned an error: %s", err.Error())
}
if c.unhealthy {
t.Errorf("the connection shouldn't have been marked as unhealthy")
}
}
if c.unhealthy {
t.Errorf("the connection shouldn't have been marked as unhealthy")

//Count should have been 1 as dial function should only have been called once
if count > 1 {
t.Errorf("Dial function has been called multiple times")
}

}

0 comments on commit 207715d

Please sign in to comment.