Skip to content

newConn race fix, fix check pool exhausted, replacing the MaxActiveConns parameter with PoolSizeStrict #2781

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+32 −27
Open

newConn race fix, fix check pool exhausted, replacing the MaxActiveConns parameter with PoolSizeStrict #2781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5aa430d
newConn race fix
nvorobev Nov 1, 2023
d822721
newConn race fix
nvorobev Nov 1, 2023
0bc34b8
replacing the MaxActiveConns parameter with PoolSizeStrict
nvorobev Nov 3, 2023
5a5d911
removed unnecessary variable
nvorobev Nov 16, 2023
76ee74e
fix check pool exhausted
nvorobev Nov 16, 2023
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
17 changes: 11 additions & 6 deletions internal/pool/pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ type Options struct {

PoolFIFO bool
PoolSize int
PoolSizeStrict bool
PoolTimeout time.Duration
MinIdleConns int
MaxIdleConns int
MaxActiveConns int
ConnMaxIdleTime time.Duration
ConnMaxLifetime time.Duration
}
Expand Down Expand Up @@ -164,13 +164,12 @@ func (p *ConnPool) NewConn(ctx context.Context) (*Conn, error) {
}

func (p *ConnPool) newConn(ctx context.Context, pooled bool) (*Conn, error) {
if p.closed() {
return nil, ErrClosed
}

if p.cfg.MaxActiveConns > 0 && p.poolSize >= p.cfg.MaxActiveConns {
p.connsMu.Lock()
if p.cfg.PoolSizeStrict && len(p.conns) >= p.cfg.PoolSize {
p.connsMu.Unlock()
return nil, ErrPoolExhausted
}
p.connsMu.Unlock()

cn, err := p.dialConn(ctx, pooled)
if err != nil {
Expand All @@ -180,6 +179,12 @@ func (p *ConnPool) newConn(ctx context.Context, pooled bool) (*Conn, error) {
p.connsMu.Lock()
defer p.connsMu.Unlock()

// It is not allowed to add new connections to the closed connection pool.
if p.closed() {
_ = cn.Close()
return nil, ErrClosed
}

Comment on lines +182 to +187
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should check if pool is closed before potential dialconn call.

p.conns = append(p.conns, cn)
if pooled {
// If pool is full remove the cn on next Put.
Expand Down
14 changes: 7 additions & 7 deletions options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,12 @@ type Options struct {
PoolFIFO bool
// Base number of socket connections.
// Default is 10 connections per every available CPU as reported by runtime.GOMAXPROCS.
// If there is not enough connections in the pool, new connections will be allocated in excess of PoolSize,
// you can limit it through MaxActiveConns
// If there are not enough connections in the pool, new connections will be allocated beyond the PoolSize,
// which can flood the server with connections under heavy load.
// To enable standard pool behavior with overflow checking, use the PoolSizeStrict parameter
PoolSize int
// Enabling classic pool mode, when it is guaranteed that no more connections will open to the server than specified in PoolSize
PoolSizeStrict bool
// Amount of time client waits for connection if all connections
// are busy before returning an error.
// Default is ReadTimeout + 1 second.
Expand All @@ -114,9 +117,6 @@ type Options struct {
// Maximum number of idle connections.
// Default is 0. the idle connections are not closed by default.
MaxIdleConns int
// Maximum number of connections allocated by the pool at a given time.
// When zero, there is no limit on the number of connections in the pool.
MaxActiveConns int
// ConnMaxIdleTime is the maximum amount of time a connection may be idle.
// Should be less than server's timeout.
//
Expand Down Expand Up @@ -458,10 +458,10 @@ func setupConnParams(u *url.URL, o *Options) (*Options, error) {
o.WriteTimeout = q.duration("write_timeout")
o.PoolFIFO = q.bool("pool_fifo")
o.PoolSize = q.int("pool_size")
o.PoolSizeStrict = q.bool("pool_size_strict")
o.PoolTimeout = q.duration("pool_timeout")
o.MinIdleConns = q.int("min_idle_conns")
o.MaxIdleConns = q.int("max_idle_conns")
o.MaxActiveConns = q.int("max_active_conns")
if q.has("conn_max_idle_time") {
o.ConnMaxIdleTime = q.duration("conn_max_idle_time")
} else {
Expand Down Expand Up @@ -505,10 +505,10 @@ func newConnPool(
},
PoolFIFO: opt.PoolFIFO,
PoolSize: opt.PoolSize,
PoolSizeStrict: opt.PoolSizeStrict,
PoolTimeout: opt.PoolTimeout,
MinIdleConns: opt.MinIdleConns,
MaxIdleConns: opt.MaxIdleConns,
MaxActiveConns: opt.MaxActiveConns,
ConnMaxIdleTime: opt.ConnMaxIdleTime,
ConnMaxLifetime: opt.ConnMaxLifetime,
})
Expand Down
8 changes: 4 additions & 4 deletions osscluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ type ClusterOptions struct {
ContextTimeoutEnabled bool

PoolFIFO bool
PoolSize int // applies per cluster node and not for the whole cluster
PoolSize int // applies per cluster node and not for the whole cluster
PoolSizeStrict bool // applies per cluster node and not for the whole cluster
PoolTimeout time.Duration
MinIdleConns int
MaxIdleConns int
MaxActiveConns int // applies per cluster node and not for the whole cluster
ConnMaxIdleTime time.Duration
ConnMaxLifetime time.Duration

Expand Down Expand Up @@ -233,9 +233,9 @@ func setupClusterQueryParams(u *url.URL, o *ClusterOptions) (*ClusterOptions, er
o.WriteTimeout = q.duration("write_timeout")
o.PoolFIFO = q.bool("pool_fifo")
o.PoolSize = q.int("pool_size")
o.PoolSizeStrict = q.bool("pool_size_strict")
o.MinIdleConns = q.int("min_idle_conns")
o.MaxIdleConns = q.int("max_idle_conns")
o.MaxActiveConns = q.int("max_active_conns")
o.PoolTimeout = q.duration("pool_timeout")
o.ConnMaxLifetime = q.duration("conn_max_lifetime")
o.ConnMaxIdleTime = q.duration("conn_max_idle_time")
Expand Down Expand Up @@ -284,10 +284,10 @@ func (opt *ClusterOptions) clientOptions() *Options {

PoolFIFO: opt.PoolFIFO,
PoolSize: opt.PoolSize,
PoolSizeStrict: opt.PoolSizeStrict,
PoolTimeout: opt.PoolTimeout,
MinIdleConns: opt.MinIdleConns,
MaxIdleConns: opt.MaxIdleConns,
MaxActiveConns: opt.MaxActiveConns,
ConnMaxIdleTime: opt.ConnMaxIdleTime,
ConnMaxLifetime: opt.ConnMaxLifetime,
DisableIndentity: opt.DisableIndentity,
Expand Down
4 changes: 2 additions & 2 deletions ring.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ type RingOptions struct {
PoolFIFO bool

PoolSize int
PoolSizeStrict bool
PoolTimeout time.Duration
MinIdleConns int
MaxIdleConns int
MaxActiveConns int
ConnMaxIdleTime time.Duration
ConnMaxLifetime time.Duration

Expand Down Expand Up @@ -155,10 +155,10 @@ func (opt *RingOptions) clientOptions() *Options {

PoolFIFO: opt.PoolFIFO,
PoolSize: opt.PoolSize,
PoolSizeStrict: opt.PoolSizeStrict,
PoolTimeout: opt.PoolTimeout,
MinIdleConns: opt.MinIdleConns,
MaxIdleConns: opt.MaxIdleConns,
MaxActiveConns: opt.MaxActiveConns,
ConnMaxIdleTime: opt.ConnMaxIdleTime,
ConnMaxLifetime: opt.ConnMaxLifetime,

Expand Down
8 changes: 4 additions & 4 deletions sentinel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ type FailoverOptions struct {
PoolFIFO bool

PoolSize int
PoolSizeStrict bool
PoolTimeout time.Duration
MinIdleConns int
MaxIdleConns int
MaxActiveConns int
ConnMaxIdleTime time.Duration
ConnMaxLifetime time.Duration

Expand Down Expand Up @@ -107,10 +107,10 @@ func (opt *FailoverOptions) clientOptions() *Options {

PoolFIFO: opt.PoolFIFO,
PoolSize: opt.PoolSize,
PoolSizeStrict: opt.PoolSizeStrict,
PoolTimeout: opt.PoolTimeout,
MinIdleConns: opt.MinIdleConns,
MaxIdleConns: opt.MaxIdleConns,
MaxActiveConns: opt.MaxActiveConns,
ConnMaxIdleTime: opt.ConnMaxIdleTime,
ConnMaxLifetime: opt.ConnMaxLifetime,

Expand Down Expand Up @@ -143,10 +143,10 @@ func (opt *FailoverOptions) sentinelOptions(addr string) *Options {

PoolFIFO: opt.PoolFIFO,
PoolSize: opt.PoolSize,
PoolSizeStrict: opt.PoolSizeStrict,
PoolTimeout: opt.PoolTimeout,
MinIdleConns: opt.MinIdleConns,
MaxIdleConns: opt.MaxIdleConns,
MaxActiveConns: opt.MaxActiveConns,
ConnMaxIdleTime: opt.ConnMaxIdleTime,
ConnMaxLifetime: opt.ConnMaxLifetime,

Expand Down Expand Up @@ -180,10 +180,10 @@ func (opt *FailoverOptions) clusterOptions() *ClusterOptions {

PoolFIFO: opt.PoolFIFO,
PoolSize: opt.PoolSize,
PoolSizeStrict: opt.PoolSizeStrict,
PoolTimeout: opt.PoolTimeout,
MinIdleConns: opt.MinIdleConns,
MaxIdleConns: opt.MaxIdleConns,
MaxActiveConns: opt.MaxActiveConns,
ConnMaxIdleTime: opt.ConnMaxIdleTime,
ConnMaxLifetime: opt.ConnMaxLifetime,

Expand Down
8 changes: 4 additions & 4 deletions universal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ type UniversalOptions struct {
PoolFIFO bool

PoolSize int
PoolSizeStrict bool
PoolTimeout time.Duration
MinIdleConns int
MaxIdleConns int
MaxActiveConns int
ConnMaxIdleTime time.Duration
ConnMaxLifetime time.Duration

Expand Down Expand Up @@ -102,10 +102,10 @@ func (o *UniversalOptions) Cluster() *ClusterOptions {
PoolFIFO: o.PoolFIFO,

PoolSize: o.PoolSize,
PoolSizeStrict: o.PoolSizeStrict,
PoolTimeout: o.PoolTimeout,
MinIdleConns: o.MinIdleConns,
MaxIdleConns: o.MaxIdleConns,
MaxActiveConns: o.MaxActiveConns,
ConnMaxIdleTime: o.ConnMaxIdleTime,
ConnMaxLifetime: o.ConnMaxLifetime,

Expand Down Expand Up @@ -147,10 +147,10 @@ func (o *UniversalOptions) Failover() *FailoverOptions {

PoolFIFO: o.PoolFIFO,
PoolSize: o.PoolSize,
PoolSizeStrict: o.PoolSizeStrict,
PoolTimeout: o.PoolTimeout,
MinIdleConns: o.MinIdleConns,
MaxIdleConns: o.MaxIdleConns,
MaxActiveConns: o.MaxActiveConns,
ConnMaxIdleTime: o.ConnMaxIdleTime,
ConnMaxLifetime: o.ConnMaxLifetime,

Expand Down Expand Up @@ -189,10 +189,10 @@ func (o *UniversalOptions) Simple() *Options {

PoolFIFO: o.PoolFIFO,
PoolSize: o.PoolSize,
PoolSizeStrict: o.PoolSizeStrict,
PoolTimeout: o.PoolTimeout,
MinIdleConns: o.MinIdleConns,
MaxIdleConns: o.MaxIdleConns,
MaxActiveConns: o.MaxActiveConns,
ConnMaxIdleTime: o.ConnMaxIdleTime,
ConnMaxLifetime: o.ConnMaxLifetime,

Expand Down