Skip to content

Commit 5be1db1

Browse files
qingyang-huqingyang-hu
authored
GODRIVER-2366 Restrict minPoolSize to not exceeding maxPoolSize (#933)
1 parent 7c228ce commit 5be1db1

File tree

7 files changed

+159
-4
lines changed

7 files changed

+159
-4
lines changed

‎mongo/client.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ import (
3232
"go.mongodb.org/mongo-driver/x/mongo/driver/topology"
3333
)
3434

35-
const defaultLocalThreshold = 15 * time.Millisecond
35+
const (
36+
defaultLocalThreshold = 15 * time.Millisecond
37+
defaultMaxPoolSize uint64 = 100
38+
)
3639

3740
var (
3841
// keyVaultCollOpts specifies options used to communicate with the key vault collection
@@ -348,6 +351,12 @@ func (c *Client) endSessions(ctx context.Context) {
348351
}
349352

350353
func (c *Client) configure(opts *options.ClientOptions) error {
354+
var defaultOptions int
355+
// Set default options
356+
if opts.MaxPoolSize == nil {
357+
defaultOptions++
358+
opts.SetMaxPoolSize(defaultMaxPoolSize)
359+
}
351360
if err := opts.Validate(); err != nil {
352361
return err
353362
}
@@ -688,8 +697,8 @@ func (c *Client) configure(opts *options.ClientOptions) error {
688697
// Deployment
689698
if opts.Deployment != nil {
690699
// topology options: WithSeedlist, WithURI, WithSRVServiceName and WithSRVMaxHosts
691-
// server options: WithClock and WithConnectionOptions
692-
if len(serverOpts) > 2 || len(topologyOpts) > 4 {
700+
// server options: WithClock and WithConnectionOptions + default maxPoolSize
701+
if len(serverOpts) > 2+defaultOptions || len(topologyOpts) > 4 {
693702
return errors.New("cannot specify topology or server options with a deployment")
694703
}
695704
c.deployment = opts.Deployment

‎mongo/client_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,94 @@ func TestClient(t *testing.T) {
179179
client := setupClient(options.Client().SetReadConcern(rc))
180180
assert.Equal(t, rc, client.readConcern, "expected read concern %v, got %v", rc, client.readConcern)
181181
})
182+
t.Run("min pool size from Set*PoolSize()", func(t *testing.T) {
183+
testCases := []struct {
184+
name string
185+
opts *options.ClientOptions
186+
err error
187+
}{
188+
{
189+
name: "minPoolSize < default maxPoolSize",
190+
opts: options.Client().SetMinPoolSize(64),
191+
err: nil,
192+
},
193+
{
194+
name: "minPoolSize > default maxPoolSize",
195+
opts: options.Client().SetMinPoolSize(128),
196+
err: errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=128 maxPoolSize=100"),
197+
},
198+
{
199+
name: "minPoolSize < maxPoolSize",
200+
opts: options.Client().SetMinPoolSize(128).SetMaxPoolSize(256),
201+
err: nil,
202+
},
203+
{
204+
name: "minPoolSize == maxPoolSize",
205+
opts: options.Client().SetMinPoolSize(128).SetMaxPoolSize(128),
206+
err: nil,
207+
},
208+
{
209+
name: "minPoolSize > maxPoolSize",
210+
opts: options.Client().SetMinPoolSize(64).SetMaxPoolSize(32),
211+
err: errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=64 maxPoolSize=32"),
212+
},
213+
{
214+
name: "maxPoolSize == 0",
215+
opts: options.Client().SetMinPoolSize(128).SetMaxPoolSize(0),
216+
err: nil,
217+
},
218+
}
219+
for _, tc := range testCases {
220+
t.Run(tc.name, func(t *testing.T) {
221+
_, err := NewClient(tc.opts)
222+
assert.Equal(t, tc.err, err, "expected error %v, got %v", tc.err, err)
223+
})
224+
}
225+
})
226+
t.Run("min pool size from ApplyURI()", func(t *testing.T) {
227+
testCases := []struct {
228+
name string
229+
opts *options.ClientOptions
230+
err error
231+
}{
232+
{
233+
name: "minPoolSize < default maxPoolSize",
234+
opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=64"),
235+
err: nil,
236+
},
237+
{
238+
name: "minPoolSize > default maxPoolSize",
239+
opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=128"),
240+
err: errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=128 maxPoolSize=100"),
241+
},
242+
{
243+
name: "minPoolSize < maxPoolSize",
244+
opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=128&maxPoolSize=256"),
245+
err: nil,
246+
},
247+
{
248+
name: "minPoolSize == maxPoolSize",
249+
opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=128&maxPoolSize=128"),
250+
err: nil,
251+
},
252+
{
253+
name: "minPoolSize > maxPoolSize",
254+
opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=64&maxPoolSize=32"),
255+
err: errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=64 maxPoolSize=32"),
256+
},
257+
{
258+
name: "maxPoolSize == 0",
259+
opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=128&maxPoolSize=0"),
260+
err: nil,
261+
},
262+
}
263+
for _, tc := range testCases {
264+
t.Run(tc.name, func(t *testing.T) {
265+
_, err := NewClient(tc.opts)
266+
assert.Equal(t, tc.err, err, "expected error %v, got %v", tc.err, err)
267+
})
268+
}
269+
})
182270
t.Run("retry writes", func(t *testing.T) {
183271
retryWritesURI := "mongodb://localhost:27017/?retryWrites=false"
184272
retryWritesErrorURI := "mongodb://localhost:27017/?retryWrites=foobar"

‎mongo/options/clientoptions.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,11 @@ func (c *ClientOptions) validateAndSetError() {
181181
}
182182
}
183183

184+
if c.MaxPoolSize != nil && c.MinPoolSize != nil && *c.MaxPoolSize != 0 && *c.MinPoolSize > *c.MaxPoolSize {
185+
c.err = fmt.Errorf("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=%d maxPoolSize=%d", *c.MinPoolSize, *c.MaxPoolSize)
186+
return
187+
}
188+
184189
// verify server API version if ServerAPIOptions are passed in.
185190
if c.ServerAPIOptions != nil {
186191
c.err = c.ServerAPIOptions.ServerAPIVersion.Validate()

‎mongo/options/clientoptions_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,40 @@ func TestClientOptions(t *testing.T) {
637637
})
638638
}
639639
})
640+
t.Run("minPoolSize validation", func(t *testing.T) {
641+
testCases := []struct {
642+
name string
643+
opts *ClientOptions
644+
err error
645+
}{
646+
{
647+
"minPoolSize < maxPoolSize",
648+
Client().SetMinPoolSize(128).SetMaxPoolSize(256),
649+
nil,
650+
},
651+
{
652+
"minPoolSize == maxPoolSize",
653+
Client().SetMinPoolSize(128).SetMaxPoolSize(128),
654+
nil,
655+
},
656+
{
657+
"minPoolSize > maxPoolSize",
658+
Client().SetMinPoolSize(64).SetMaxPoolSize(32),
659+
errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=64 maxPoolSize=32"),
660+
},
661+
{
662+
"maxPoolSize == 0",
663+
Client().SetMinPoolSize(128).SetMaxPoolSize(0),
664+
nil,
665+
},
666+
}
667+
for _, tc := range testCases {
668+
t.Run(tc.name, func(t *testing.T) {
669+
err := tc.opts.Validate()
670+
assert.Equal(t, tc.err, err, "expected error %v, got %v", tc.err, err)
671+
})
672+
}
673+
})
640674
t.Run("srvMaxHosts validation", func(t *testing.T) {
641675
testCases := []struct {
642676
name string

‎x/mongo/driver/topology/pool.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ func newPool(config poolConfig, connOpts ...ConnectionOption) *pool {
176176
conns: make(map[uint64]*connection, config.MaxPoolSize),
177177
idleConns: make([]*connection, 0, config.MaxPoolSize),
178178
}
179+
// minSize must not exceed maxSize if maxSize is not 0
180+
if pool.maxSize != 0 && pool.minSize > pool.maxSize {
181+
pool.minSize = pool.maxSize
182+
}
179183
pool.connOpts = append(pool.connOpts, withGenerationNumberFn(func(_ generationNumberFn) generationNumberFn { return pool.getGenerationForNewConnection }))
180184

181185
pool.generation.connect()

‎x/mongo/driver/topology/pool_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,22 @@ func TestPool(t *testing.T) {
2323
t.Run("newPool", func(t *testing.T) {
2424
t.Parallel()
2525

26+
t.Run("minPoolSize should not exceed maxPoolSize", func(t *testing.T) {
27+
t.Parallel()
28+
29+
p := newPool(poolConfig{MinPoolSize: 100, MaxPoolSize: 10})
30+
assert.Equalf(t, uint64(10), p.minSize, "expected minSize of a pool not to be greater than maxSize")
31+
32+
p.close(context.Background())
33+
})
34+
t.Run("minPoolSize may exceed maxPoolSize of 0", func(t *testing.T) {
35+
t.Parallel()
36+
37+
p := newPool(poolConfig{MinPoolSize: 10, MaxPoolSize: 0})
38+
assert.Equalf(t, uint64(10), p.minSize, "expected minSize of a pool to be greater than maxSize of 0")
39+
40+
p.close(context.Background())
41+
})
2642
t.Run("should be paused", func(t *testing.T) {
2743
t.Parallel()
2844

‎x/mongo/driver/topology/server_options.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ func newServerConfig(opts ...ServerOption) *serverConfig {
4444
cfg := &serverConfig{
4545
heartbeatInterval: 10 * time.Second,
4646
heartbeatTimeout: 10 * time.Second,
47-
maxConns: 100,
4847
registry: defaultRegistry,
4948
}
5049

0 commit comments

Comments
 (0)