Merge pull request #264 from go-redis/fix/used-at

Update conn.UsedAt on Read/Write. Fixes #263.
This commit is contained in:
Vladimir Mihailenco 2016-03-02 14:46:27 +02:00
commit 7116858f67
4 changed files with 33 additions and 19 deletions

14
conn.go
View File

@ -9,7 +9,7 @@ import (
const defaultBufSize = 4096 const defaultBufSize = 4096
var ( var (
zeroTime = time.Time{} noTimeout = time.Time{}
) )
type conn struct { type conn struct {
@ -17,7 +17,7 @@ type conn struct {
rd *bufio.Reader rd *bufio.Reader
buf []byte buf []byte
usedAt time.Time UsedAt time.Time
ReadTimeout time.Duration ReadTimeout time.Duration
WriteTimeout time.Duration WriteTimeout time.Duration
} }
@ -76,19 +76,21 @@ func (cn *conn) writeCmds(cmds ...Cmder) error {
} }
func (cn *conn) Read(b []byte) (int, error) { func (cn *conn) Read(b []byte) (int, error) {
cn.UsedAt = time.Now()
if cn.ReadTimeout != 0 { if cn.ReadTimeout != 0 {
cn.netcn.SetReadDeadline(time.Now().Add(cn.ReadTimeout)) cn.netcn.SetReadDeadline(cn.UsedAt.Add(cn.ReadTimeout))
} else { } else {
cn.netcn.SetReadDeadline(zeroTime) cn.netcn.SetReadDeadline(noTimeout)
} }
return cn.netcn.Read(b) return cn.netcn.Read(b)
} }
func (cn *conn) Write(b []byte) (int, error) { func (cn *conn) Write(b []byte) (int, error) {
cn.UsedAt = time.Now()
if cn.WriteTimeout != 0 { if cn.WriteTimeout != 0 {
cn.netcn.SetWriteDeadline(time.Now().Add(cn.WriteTimeout)) cn.netcn.SetWriteDeadline(cn.UsedAt.Add(cn.WriteTimeout))
} else { } else {
cn.netcn.SetWriteDeadline(zeroTime) cn.netcn.SetWriteDeadline(noTimeout)
} }
return cn.netcn.Write(b) return cn.netcn.Write(b)
} }

View File

@ -164,7 +164,7 @@ func (p *connPool) closed() bool {
} }
func (p *connPool) isIdle(cn *conn) bool { func (p *connPool) isIdle(cn *conn) bool {
return p.opt.getIdleTimeout() > 0 && time.Since(cn.usedAt) > p.opt.getIdleTimeout() return p.opt.getIdleTimeout() > 0 && time.Since(cn.UsedAt) > p.opt.getIdleTimeout()
} }
// First returns first non-idle connection from the pool or nil if // First returns first non-idle connection from the pool or nil if
@ -275,9 +275,6 @@ func (p *connPool) Put(cn *conn) error {
Logger.Print(err) Logger.Print(err)
return p.Remove(cn, err) return p.Remove(cn, err)
} }
if p.opt.getIdleTimeout() > 0 {
cn.usedAt = time.Now()
}
p.freeConns <- cn p.freeConns <- cn
return nil return nil
} }

View File

@ -27,14 +27,14 @@ func (c *baseClient) putConn(cn *conn, err error) bool {
if isBadConn(err) { if isBadConn(err) {
err = c.connPool.Remove(cn, err) err = c.connPool.Remove(cn, err)
if err != nil { if err != nil {
log.Printf("pool.Remove failed: %s", err) Logger.Printf("pool.Remove failed: %s", err)
} }
return false return false
} }
err = c.connPool.Put(cn) err = c.connPool.Put(cn)
if err != nil { if err != nil {
log.Printf("pool.Put failed: %s", err) Logger.Printf("pool.Put failed: %s", err)
} }
return true return true
} }

View File

@ -55,21 +55,19 @@ var _ = Describe("Client", func() {
It("should close", func() { It("should close", func() {
Expect(client.Close()).NotTo(HaveOccurred()) Expect(client.Close()).NotTo(HaveOccurred())
err := client.Ping().Err() err := client.Ping().Err()
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError("redis: client is closed")) Expect(err).To(MatchError("redis: client is closed"))
}) })
It("should close pubsub without closing the connection", func() { It("should close pubsub without closing the client", func() {
pubsub := client.PubSub() pubsub := client.PubSub()
Expect(pubsub.Close()).NotTo(HaveOccurred()) Expect(pubsub.Close()).NotTo(HaveOccurred())
_, err := pubsub.Receive() _, err := pubsub.Receive()
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError("redis: client is closed")) Expect(err).To(MatchError("redis: client is closed"))
Expect(client.Ping().Err()).NotTo(HaveOccurred()) Expect(client.Ping().Err()).NotTo(HaveOccurred())
}) })
It("should close multi without closing the connection", func() { It("should close multi without closing the client", func() {
multi := client.Multi() multi := client.Multi()
Expect(multi.Close()).NotTo(HaveOccurred()) Expect(multi.Close()).NotTo(HaveOccurred())
@ -77,19 +75,19 @@ var _ = Describe("Client", func() {
multi.Ping() multi.Ping()
return nil return nil
}) })
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError("redis: client is closed")) Expect(err).To(MatchError("redis: client is closed"))
Expect(client.Ping().Err()).NotTo(HaveOccurred()) Expect(client.Ping().Err()).NotTo(HaveOccurred())
}) })
It("should close pipeline without closing the connection", func() { It("should close pipeline without closing the client", func() {
pipeline := client.Pipeline() pipeline := client.Pipeline()
Expect(pipeline.Close()).NotTo(HaveOccurred()) Expect(pipeline.Close()).NotTo(HaveOccurred())
pipeline.Ping() pipeline.Ping()
_, err := pipeline.Exec() _, err := pipeline.Exec()
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError("redis: client is closed")) Expect(err).To(MatchError("redis: client is closed"))
Expect(client.Ping().Err()).NotTo(HaveOccurred()) Expect(client.Ping().Err()).NotTo(HaveOccurred())
}) })
@ -171,6 +169,23 @@ var _ = Describe("Client", func() {
err = client.Ping().Err() err = client.Ping().Err()
Expect(err).NotTo(HaveOccurred()) Expect(err).NotTo(HaveOccurred())
}) })
It("should maintain conn.UsedAt", func() {
cn, _, err := client.Pool().Get()
Expect(err).NotTo(HaveOccurred())
Expect(cn.UsedAt).To(BeZero())
err = client.Pool().Put(cn)
Expect(err).NotTo(HaveOccurred())
Expect(cn.UsedAt).To(BeZero())
err = client.Ping().Err()
Expect(err).NotTo(HaveOccurred())
cn = client.Pool().First()
Expect(cn).NotTo(BeNil())
Expect(cn.UsedAt).To(BeTemporally("~", time.Now()))
})
}) })
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------