Skip to content

Revert "ConnPool check fd for bad conns" #1849

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 1 commit into from
Aug 6, 2021
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
45 changes: 0 additions & 45 deletions internal/pool/conncheck.go

This file was deleted.

9 changes: 0 additions & 9 deletions internal/pool/conncheck_dummy.go

This file was deleted.

46 changes: 0 additions & 46 deletions internal/pool/conncheck_test.go

This file was deleted.

87 changes: 1 addition & 86 deletions internal/pool/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ package pool_test

import (
"context"
"fmt"
"net"
"sync"
"syscall"
"testing"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -35,87 +32,5 @@ func perform(n int, cbs ...func(int)) {
}

func dummyDialer(context.Context) (net.Conn, error) {
// return &net.TCPConn{}, nil
return newDummyConn(), nil
}

func newDummyConn() net.Conn {
return &dummyConn{
rawConn: &dummyRawConn{},
}
}

var _ net.Conn = (*dummyConn)(nil)
var _ syscall.Conn = (*dummyConn)(nil)

type dummyConn struct {
rawConn *dummyRawConn
}

func (d *dummyConn) SyscallConn() (syscall.RawConn, error) {
return d.rawConn, nil
}

var errDummy = fmt.Errorf("dummyConn err")

func (d *dummyConn) Read(b []byte) (n int, err error) {
return 0, errDummy
}

func (d *dummyConn) Write(b []byte) (n int, err error) {
return 0, errDummy
}

func (d *dummyConn) Close() error {
d.rawConn.Close()
return nil
}

func (d *dummyConn) LocalAddr() net.Addr {
return &net.TCPAddr{}
}

func (d *dummyConn) RemoteAddr() net.Addr {
return &net.TCPAddr{}
}

func (d *dummyConn) SetDeadline(t time.Time) error {
return nil
}

func (d *dummyConn) SetReadDeadline(t time.Time) error {
return nil
}

func (d *dummyConn) SetWriteDeadline(t time.Time) error {
return nil
}

var _ syscall.RawConn = (*dummyRawConn)(nil)

type dummyRawConn struct {
closed bool
mux sync.Mutex
}

func (d *dummyRawConn) Control(f func(fd uintptr)) error {
return nil
}

func (d *dummyRawConn) Read(f func(fd uintptr) (done bool)) error {
d.mux.Lock()
defer d.mux.Unlock()
if d.closed {
return fmt.Errorf("dummyRawConn closed")
}
return nil
}

func (d *dummyRawConn) Write(f func(fd uintptr) (done bool)) error {
return nil
}
func (d *dummyRawConn) Close() {
d.mux.Lock()
d.closed = true
d.mux.Unlock()
return &net.TCPConn{}, nil
}
4 changes: 2 additions & 2 deletions internal/pool/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func (p *ConnPool) reapStaleConn() *Conn {

func (p *ConnPool) isStaleConn(cn *Conn) bool {
if p.opt.IdleTimeout == 0 && p.opt.MaxConnAge == 0 {
return connCheck(cn.netConn) != nil
return false
}

now := time.Now()
Expand All @@ -531,5 +531,5 @@ func (p *ConnPool) isStaleConn(cn *Conn) bool {
return true
}

return connCheck(cn.netConn) != nil
return false
}
7 changes: 2 additions & 5 deletions internal/pool/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import (
"testing"
"time"

"github.com/go-redis/redis/v8/internal/pool"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"github.com/go-redis/redis/v8/internal/pool"
)

var _ = Describe("ConnPool", func() {
Expand Down Expand Up @@ -285,8 +285,6 @@ var _ = Describe("conns reaper", func() {
cn.SetUsedAt(time.Now().Add(-2 * idleTimeout))
case "aged":
cn.SetCreatedAt(time.Now().Add(-2 * maxAge))
case "conncheck":
cn.Close()
}
conns = append(conns, cn)
staleConns = append(staleConns, cn)
Expand Down Expand Up @@ -373,7 +371,6 @@ var _ = Describe("conns reaper", func() {

assert("idle")
assert("aged")
assert("conncheck")
})

var _ = Describe("race", func() {
Expand Down
10 changes: 5 additions & 5 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import (
"testing"
"time"

"github.com/go-redis/redis/v8"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"github.com/go-redis/redis/v8"
)

const (
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestGinkgoSuite(t *testing.T) {
RunSpecs(t, "go-redis")
}

// ------------------------------------------------------------------------------
//------------------------------------------------------------------------------

func redisOptions() *redis.Options {
return &redis.Options{
Expand Down Expand Up @@ -364,7 +364,7 @@ func startSentinel(port, masterName, masterPort string) (*redisProcess, error) {
return p, nil
}

// ------------------------------------------------------------------------------
//------------------------------------------------------------------------------

type badConnError string

Expand Down Expand Up @@ -409,7 +409,7 @@ func (cn *badConn) Write([]byte) (int, error) {
return 0, badConnError("bad connection")
}

// ------------------------------------------------------------------------------
//------------------------------------------------------------------------------

type hook struct {
beforeProcess func(ctx context.Context, cmd redis.Cmder) (context.Context, error)
Expand Down
3 changes: 1 addition & 2 deletions pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,8 @@ var _ = Describe("pool", func() {
cn.SetNetConn(&badConn{})
client.Pool().Put(ctx, cn)

// connCheck will automatically remove damaged connections.
err = client.Ping(ctx).Err()
Expect(err).NotTo(HaveOccurred())
Expect(err).To(MatchError("bad connection"))

val, err := client.Ping(ctx).Result()
Expect(err).NotTo(HaveOccurred())
Expand Down
2 changes: 1 addition & 1 deletion sentinel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ var _ = Describe("NewFailoverClusterClient", func() {
err = master.Shutdown(ctx).Err()
Expect(err).NotTo(HaveOccurred())
Eventually(func() error {
return master.Ping(ctx).Err()
return sentinelMaster.Ping(ctx).Err()
}, "15s", "100ms").Should(HaveOccurred())

// Check that client picked up new master.
Expand Down
9 changes: 6 additions & 3 deletions tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ var _ = Describe("Tx", func() {
Expect(num).To(Equal(int64(N)))
})

It("should remove from bad connection", func() {
It("should recover from bad connection", func() {
// Put bad connection in the pool.
cn, err := client.Pool().Get(context.Background())
Expect(err).NotTo(HaveOccurred())
Expand All @@ -134,14 +134,17 @@ var _ = Describe("Tx", func() {
do := func() error {
err := client.Watch(ctx, func(tx *redis.Tx) error {
_, err := tx.TxPipelined(ctx, func(pipe redis.Pipeliner) error {
return pipe.Ping(ctx).Err()
pipe.Ping(ctx)
return nil
})
return err
})
return err
}

// connCheck will automatically remove damaged connections.
err = do()
Expect(err).To(MatchError("bad connection"))

err = do()
Expect(err).NotTo(HaveOccurred())
})
Expand Down