From 78d40d5bd72d988b912c8a32b4ff2505de86c1a8 Mon Sep 17 00:00:00 2001 From: Vladimir Mihailenco Date: Wed, 2 Mar 2016 13:26:05 +0200 Subject: [PATCH 1/2] Update conn.UsedAt on Read/Write. Fixes #263. --- conn.go | 14 ++++++++------ pool.go | 5 +---- redis_test.go | 29 ++++++++++++++++++++++------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/conn.go b/conn.go index ec4abd1..40d612b 100644 --- a/conn.go +++ b/conn.go @@ -9,7 +9,7 @@ import ( const defaultBufSize = 4096 var ( - zeroTime = time.Time{} + noTimeout = time.Time{} ) type conn struct { @@ -17,7 +17,7 @@ type conn struct { rd *bufio.Reader buf []byte - usedAt time.Time + UsedAt time.Time ReadTimeout time.Duration WriteTimeout time.Duration } @@ -76,19 +76,21 @@ func (cn *conn) writeCmds(cmds ...Cmder) error { } func (cn *conn) Read(b []byte) (int, error) { + cn.UsedAt = time.Now() if cn.ReadTimeout != 0 { - cn.netcn.SetReadDeadline(time.Now().Add(cn.ReadTimeout)) + cn.netcn.SetReadDeadline(cn.UsedAt.Add(cn.ReadTimeout)) } else { - cn.netcn.SetReadDeadline(zeroTime) + cn.netcn.SetReadDeadline(noTimeout) } return cn.netcn.Read(b) } func (cn *conn) Write(b []byte) (int, error) { + cn.UsedAt = time.Now() if cn.WriteTimeout != 0 { - cn.netcn.SetWriteDeadline(time.Now().Add(cn.WriteTimeout)) + cn.netcn.SetWriteDeadline(cn.UsedAt.Add(cn.WriteTimeout)) } else { - cn.netcn.SetWriteDeadline(zeroTime) + cn.netcn.SetWriteDeadline(noTimeout) } return cn.netcn.Write(b) } diff --git a/pool.go b/pool.go index e2f9f2b..bb713bf 100644 --- a/pool.go +++ b/pool.go @@ -164,7 +164,7 @@ func (p *connPool) closed() 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 @@ -275,9 +275,6 @@ func (p *connPool) Put(cn *conn) error { Logger.Print(err) return p.Remove(cn, err) } - if p.opt.getIdleTimeout() > 0 { - cn.usedAt = time.Now() - } p.freeConns <- cn return nil } diff --git a/redis_test.go b/redis_test.go index 3822202..724d241 100644 --- a/redis_test.go +++ b/redis_test.go @@ -55,21 +55,19 @@ var _ = Describe("Client", func() { It("should close", func() { Expect(client.Close()).NotTo(HaveOccurred()) err := client.Ping().Err() - Expect(err).To(HaveOccurred()) 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() Expect(pubsub.Close()).NotTo(HaveOccurred()) _, err := pubsub.Receive() - Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("redis: client is closed")) 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() Expect(multi.Close()).NotTo(HaveOccurred()) @@ -77,19 +75,19 @@ var _ = Describe("Client", func() { multi.Ping() return nil }) - Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("redis: client is closed")) + 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() Expect(pipeline.Close()).NotTo(HaveOccurred()) pipeline.Ping() _, err := pipeline.Exec() - Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("redis: client is closed")) + Expect(client.Ping().Err()).NotTo(HaveOccurred()) }) @@ -171,6 +169,23 @@ var _ = Describe("Client", func() { err = client.Ping().Err() 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())) + }) }) //------------------------------------------------------------------------------ From 73ad84252c7afa2a2d24ba55ae51d6c972148e7c Mon Sep 17 00:00:00 2001 From: Vladimir Mihailenco Date: Wed, 2 Mar 2016 13:37:28 +0200 Subject: [PATCH 2/2] Use package logger. --- redis.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redis.go b/redis.go index f37c97b..5dbaac0 100644 --- a/redis.go +++ b/redis.go @@ -27,14 +27,14 @@ func (c *baseClient) putConn(cn *conn, err error) bool { if isBadConn(err) { err = c.connPool.Remove(cn, err) if err != nil { - log.Printf("pool.Remove failed: %s", err) + Logger.Printf("pool.Remove failed: %s", err) } return false } err = c.connPool.Put(cn) if err != nil { - log.Printf("pool.Put failed: %s", err) + Logger.Printf("pool.Put failed: %s", err) } return true }