fix: prevent reconnect after pool close
This commit is contained in:
@@ -2,6 +2,7 @@ package db
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"log"
|
"log"
|
||||||
"net"
|
"net"
|
||||||
@@ -22,8 +23,11 @@ type Pool struct {
|
|||||||
mu sync.RWMutex
|
mu sync.RWMutex
|
||||||
conn clickhouse.Conn
|
conn clickhouse.Conn
|
||||||
healthy bool
|
healthy bool
|
||||||
|
closed bool
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var errPoolClosed = errors.New("db pool is closed")
|
||||||
|
|
||||||
// NewPool 创建 Pool。即使首次连接失败也返回非 nil 实例,后台会持续重试。
|
// NewPool 创建 Pool。即使首次连接失败也返回非 nil 实例,后台会持续重试。
|
||||||
func NewPool(ctx context.Context, dsn string, reconnectInterval time.Duration) *Pool {
|
func NewPool(ctx context.Context, dsn string, reconnectInterval time.Duration) *Pool {
|
||||||
p := &Pool{dsn: dsn, reconnectInterval: reconnectInterval}
|
p := &Pool{dsn: dsn, reconnectInterval: reconnectInterval}
|
||||||
@@ -35,6 +39,13 @@ func NewPool(ctx context.Context, dsn string, reconnectInterval time.Duration) *
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (p *Pool) connect(ctx context.Context) error {
|
func (p *Pool) connect(ctx context.Context) error {
|
||||||
|
p.mu.RLock()
|
||||||
|
closed := p.closed
|
||||||
|
p.mu.RUnlock()
|
||||||
|
if closed {
|
||||||
|
return errPoolClosed
|
||||||
|
}
|
||||||
|
|
||||||
cctx, cancel := context.WithTimeout(ctx, 5*time.Second)
|
cctx, cancel := context.WithTimeout(ctx, 5*time.Second)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
opts, err := ClickHouseOptions(p.dsn)
|
opts, err := ClickHouseOptions(p.dsn)
|
||||||
@@ -62,6 +73,11 @@ func (p *Pool) connect(ctx context.Context) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
p.mu.Lock()
|
p.mu.Lock()
|
||||||
|
if p.closed {
|
||||||
|
p.mu.Unlock()
|
||||||
|
_ = conn.Close()
|
||||||
|
return errPoolClosed
|
||||||
|
}
|
||||||
if p.conn != nil {
|
if p.conn != nil {
|
||||||
_ = p.conn.Close()
|
_ = p.conn.Close()
|
||||||
}
|
}
|
||||||
@@ -217,6 +233,7 @@ func (p *Pool) Get() clickhouse.Conn {
|
|||||||
// Close starts closing the underlying connection and waits within ctx.
|
// Close starts closing the underlying connection and waits within ctx.
|
||||||
func (p *Pool) Close(ctx context.Context) error {
|
func (p *Pool) Close(ctx context.Context) error {
|
||||||
p.mu.Lock()
|
p.mu.Lock()
|
||||||
|
p.closed = true
|
||||||
conn := p.conn
|
conn := p.conn
|
||||||
p.conn = nil
|
p.conn = nil
|
||||||
p.healthy = false
|
p.healthy = false
|
||||||
|
|||||||
+53
-9
@@ -183,6 +183,37 @@ func TestInitialConnectWithIncompatibleSchemaStaysUnhealthy(t *testing.T) {
|
|||||||
assertNonDestructiveMigration(t, candidate.statements)
|
assertNonDestructiveMigration(t, candidate.statements)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestClosePreventsConcurrentConnectFromPublishing(t *testing.T) {
|
||||||
|
old := newFakeClickHouseConn()
|
||||||
|
pingStarted := make(chan struct{})
|
||||||
|
pingRelease := make(chan struct{})
|
||||||
|
candidate := newFakeClickHouseConn().withBlockedPing(pingStarted, pingRelease)
|
||||||
|
pool := &Pool{dsn: "localhost:9000", conn: old, healthy: true}
|
||||||
|
pool.open = func(*clickhouse.Options) (clickhouse.Conn, error) { return candidate, nil }
|
||||||
|
|
||||||
|
connectDone := make(chan error, 1)
|
||||||
|
go func() { connectDone <- pool.connect(context.Background()) }()
|
||||||
|
<-pingStarted
|
||||||
|
|
||||||
|
if err := pool.Close(context.Background()); err != nil {
|
||||||
|
t.Fatalf("Close: %v", err)
|
||||||
|
}
|
||||||
|
close(pingRelease)
|
||||||
|
if err := <-connectDone; err == nil {
|
||||||
|
t.Fatal("connect succeeded after pool was closed")
|
||||||
|
}
|
||||||
|
|
||||||
|
if pool.Get() != nil || pool.Healthy() {
|
||||||
|
t.Fatalf("closed pool connection = %T, healthy = %v; want nil, false", pool.Get(), pool.Healthy())
|
||||||
|
}
|
||||||
|
if old.closeCalls != 1 {
|
||||||
|
t.Fatalf("old connection close calls = %d, want 1", old.closeCalls)
|
||||||
|
}
|
||||||
|
if candidate.closeCalls != 1 {
|
||||||
|
t.Fatalf("candidate close calls = %d, want 1", candidate.closeCalls)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func assertNonDestructiveMigration(t *testing.T, statements []string) {
|
func assertNonDestructiveMigration(t *testing.T, statements []string) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
if len(statements) == 0 || !strings.Contains(strings.ToUpper(statements[0]), "CREATE TABLE IF NOT EXISTS") {
|
if len(statements) == 0 || !strings.Contains(strings.ToUpper(statements[0]), "CREATE TABLE IF NOT EXISTS") {
|
||||||
@@ -197,12 +228,14 @@ func assertNonDestructiveMigration(t *testing.T, statements []string) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
type fakeClickHouseConn struct {
|
type fakeClickHouseConn struct {
|
||||||
rows *fakeRows
|
rows *fakeRows
|
||||||
row driver.Row
|
row driver.Row
|
||||||
execErr error
|
execErr error
|
||||||
queryErr error
|
queryErr error
|
||||||
statements []string
|
statements []string
|
||||||
closeCalls int
|
closeCalls int
|
||||||
|
pingStarted chan struct{}
|
||||||
|
pingRelease chan struct{}
|
||||||
}
|
}
|
||||||
|
|
||||||
func newFakeClickHouseConn() *fakeClickHouseConn {
|
func newFakeClickHouseConn() *fakeClickHouseConn {
|
||||||
@@ -229,6 +262,11 @@ func (c *fakeClickHouseConn) withColumnType(index int, typ string) *fakeClickHou
|
|||||||
c.rows.values[index][1] = typ
|
c.rows.values[index][1] = typ
|
||||||
return c
|
return c
|
||||||
}
|
}
|
||||||
|
func (c *fakeClickHouseConn) withBlockedPing(started, release chan struct{}) *fakeClickHouseConn {
|
||||||
|
c.pingStarted = started
|
||||||
|
c.pingRelease = release
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
func (c *fakeClickHouseConn) Contributors() []string { return nil }
|
func (c *fakeClickHouseConn) Contributors() []string { return nil }
|
||||||
func (c *fakeClickHouseConn) ServerVersion() (*driver.ServerVersion, error) { return nil, nil }
|
func (c *fakeClickHouseConn) ServerVersion() (*driver.ServerVersion, error) { return nil, nil }
|
||||||
@@ -249,9 +287,15 @@ func (c *fakeClickHouseConn) Exec(_ context.Context, query string, _ ...any) err
|
|||||||
return c.execErr
|
return c.execErr
|
||||||
}
|
}
|
||||||
func (c *fakeClickHouseConn) AsyncInsert(context.Context, string, bool, ...any) error { return nil }
|
func (c *fakeClickHouseConn) AsyncInsert(context.Context, string, bool, ...any) error { return nil }
|
||||||
func (c *fakeClickHouseConn) Ping(context.Context) error { return nil }
|
func (c *fakeClickHouseConn) Ping(context.Context) error {
|
||||||
func (c *fakeClickHouseConn) Stats() driver.Stats { return driver.Stats{} }
|
if c.pingStarted != nil {
|
||||||
func (c *fakeClickHouseConn) Close() error { c.closeCalls++; return nil }
|
close(c.pingStarted)
|
||||||
|
<-c.pingRelease
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
func (c *fakeClickHouseConn) Stats() driver.Stats { return driver.Stats{} }
|
||||||
|
func (c *fakeClickHouseConn) Close() error { c.closeCalls++; return nil }
|
||||||
|
|
||||||
type fakeRows struct {
|
type fakeRows struct {
|
||||||
values [][]string
|
values [][]string
|
||||||
|
|||||||
Reference in New Issue
Block a user