GODRIVER-2366 Restrict minPoolSize to not exceeding maxPoolSize (#933)

Author: qingyang-hu

mongo/client.go

@@ -32,7 +32,10 @@ import (
 	"go.mongodb.org/mongo-driver/x/mongo/driver/uuid"
 )
 
-const defaultLocalThreshold = 15 * time.Millisecond
+const (
+	defaultLocalThreshold        = 15 * time.Millisecond
+	defaultMaxPoolSize    uint64 = 100
+)
 
 var (
 	// keyVaultCollOpts specifies options used to communicate with the key vault collection
@@ -348,6 +351,12 @@ func (c *Client) endSessions(ctx context.Context) {
 }
 
 func (c *Client) configure(opts *options.ClientOptions) error {
+	var defaultOptions int
+	// Set default options
+	if opts.MaxPoolSize == nil {
+		defaultOptions++
+		opts.SetMaxPoolSize(defaultMaxPoolSize)
+	}
 	if err := opts.Validate(); err != nil {
 		return err
 	}
@@ -688,8 +697,8 @@ func (c *Client) configure(opts *options.ClientOptions) error {
 	// Deployment
 	if opts.Deployment != nil {
 		// topology options: WithSeedlist, WithURI, WithSRVServiceName and WithSRVMaxHosts
-		// server options: WithClock and WithConnectionOptions
-		if len(serverOpts) > 2 || len(topologyOpts) > 4 {
+		// server options: WithClock and WithConnectionOptions + default maxPoolSize
+		if len(serverOpts) > 2+defaultOptions || len(topologyOpts) > 4 {
 			return errors.New("cannot specify topology or server options with a deployment")
 		}
 		c.deployment = opts.Deployment

mongo/client_test.go

@@ -179,6 +179,94 @@ func TestClient(t *testing.T) {
 		client := setupClient(options.Client().SetReadConcern(rc))
 		assert.Equal(t, rc, client.readConcern, "expected read concern %v, got %v", rc, client.readConcern)
 	})
+	t.Run("min pool size from Set*PoolSize()", func(t *testing.T) {
+		testCases := []struct {
+			name string
+			opts *options.ClientOptions
+			err  error
+		}{
+			{
+				name: "minPoolSize < default maxPoolSize",
+				opts: options.Client().SetMinPoolSize(64),
+				err:  nil,
+			},
+			{
+				name: "minPoolSize > default maxPoolSize",
+				opts: options.Client().SetMinPoolSize(128),
+				err:  errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=128 maxPoolSize=100"),
+			},
+			{
+				name: "minPoolSize < maxPoolSize",
+				opts: options.Client().SetMinPoolSize(128).SetMaxPoolSize(256),
+				err:  nil,
+			},
+			{
+				name: "minPoolSize == maxPoolSize",
+				opts: options.Client().SetMinPoolSize(128).SetMaxPoolSize(128),
+				err:  nil,
+			},
+			{
+				name: "minPoolSize > maxPoolSize",
+				opts: options.Client().SetMinPoolSize(64).SetMaxPoolSize(32),
+				err:  errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=64 maxPoolSize=32"),
+			},
+			{
+				name: "maxPoolSize == 0",
+				opts: options.Client().SetMinPoolSize(128).SetMaxPoolSize(0),
+				err:  nil,
+			},
+		}
+		for _, tc := range testCases {
+			t.Run(tc.name, func(t *testing.T) {
+				_, err := NewClient(tc.opts)
+				assert.Equal(t, tc.err, err, "expected error %v, got %v", tc.err, err)
+			})
+		}
+	})
+	t.Run("min pool size from ApplyURI()", func(t *testing.T) {
+		testCases := []struct {
+			name string
+			opts *options.ClientOptions
+			err  error
+		}{
+			{
+				name: "minPoolSize < default maxPoolSize",
+				opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=64"),
+				err:  nil,
+			},
+			{
+				name: "minPoolSize > default maxPoolSize",
+				opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=128"),
+				err:  errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=128 maxPoolSize=100"),
+			},
+			{
+				name: "minPoolSize < maxPoolSize",
+				opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=128&maxPoolSize=256"),
+				err:  nil,
+			},
+			{
+				name: "minPoolSize == maxPoolSize",
+				opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=128&maxPoolSize=128"),
+				err:  nil,
+			},
+			{
+				name: "minPoolSize > maxPoolSize",
+				opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=64&maxPoolSize=32"),
+				err:  errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=64 maxPoolSize=32"),
+			},
+			{
+				name: "maxPoolSize == 0",
+				opts: options.Client().ApplyURI("mongodb://localhost:27017/?minPoolSize=128&maxPoolSize=0"),
+				err:  nil,
+			},
+		}
+		for _, tc := range testCases {
+			t.Run(tc.name, func(t *testing.T) {
+				_, err := NewClient(tc.opts)
+				assert.Equal(t, tc.err, err, "expected error %v, got %v", tc.err, err)
+			})
+		}
+	})
 	t.Run("retry writes", func(t *testing.T) {
 		retryWritesURI := "mongodb://localhost:27017/?retryWrites=false"
 		retryWritesErrorURI := "mongodb://localhost:27017/?retryWrites=foobar"

mongo/options/clientoptions.go

@@ -181,6 +181,11 @@ func (c *ClientOptions) validateAndSetError() {
 		}
 	}
 
+	if c.MaxPoolSize != nil && c.MinPoolSize != nil && *c.MaxPoolSize != 0 && *c.MinPoolSize > *c.MaxPoolSize {
+		c.err = fmt.Errorf("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=%d maxPoolSize=%d", *c.MinPoolSize, *c.MaxPoolSize)
+		return
+	}
+
 	// verify server API version if ServerAPIOptions are passed in.
 	if c.ServerAPIOptions != nil {
 		c.err = c.ServerAPIOptions.ServerAPIVersion.Validate()

mongo/options/clientoptions_test.go

@@ -631,6 +631,40 @@ func TestClientOptions(t *testing.T) {
 			})
 		}
 	})
+	t.Run("minPoolSize validation", func(t *testing.T) {
+		testCases := []struct {
+			name string
+			opts *ClientOptions
+			err  error
+		}{
+			{
+				"minPoolSize < maxPoolSize",
+				Client().SetMinPoolSize(128).SetMaxPoolSize(256),
+				nil,
+			},
+			{
+				"minPoolSize == maxPoolSize",
+				Client().SetMinPoolSize(128).SetMaxPoolSize(128),
+				nil,
+			},
+			{
+				"minPoolSize > maxPoolSize",
+				Client().SetMinPoolSize(64).SetMaxPoolSize(32),
+				errors.New("minPoolSize must be less than or equal to maxPoolSize, got minPoolSize=64 maxPoolSize=32"),
+			},
+			{
+				"maxPoolSize == 0",
+				Client().SetMinPoolSize(128).SetMaxPoolSize(0),
+				nil,
+			},
+		}
+		for _, tc := range testCases {
+			t.Run(tc.name, func(t *testing.T) {
+				err := tc.opts.Validate()
+				assert.Equal(t, tc.err, err, "expected error %v, got %v", tc.err, err)
+			})
+		}
+	})
 	t.Run("srvMaxHosts validation", func(t *testing.T) {
 		testCases := []struct {
 			name string

x/mongo/driver/topology/pool.go

@@ -176,6 +176,10 @@ func newPool(config poolConfig, connOpts ...ConnectionOption) *pool {
 		conns:                 make(map[uint64]*connection, config.MaxPoolSize),
 		idleConns:             make([]*connection, 0, config.MaxPoolSize),
 	}
+	// minSize must not exceed maxSize if maxSize is not 0
+	if pool.maxSize != 0 && pool.minSize > pool.maxSize {
+		pool.minSize = pool.maxSize
+	}
 	pool.connOpts = append(pool.connOpts, withGenerationNumberFn(func(_ generationNumberFn) generationNumberFn { return pool.getGenerationForNewConnection }))
 
 	pool.generation.connect()

x/mongo/driver/topology/pool_test.go

@@ -17,6 +17,22 @@ func TestPool(t *testing.T) {
 	t.Run("newPool", func(t *testing.T) {
 		t.Parallel()
 
+		t.Run("minPoolSize should not exceed maxPoolSize", func(t *testing.T) {
+			t.Parallel()
+
+			p := newPool(poolConfig{MinPoolSize: 100, MaxPoolSize: 10})
+			assert.Equalf(t, uint64(10), p.minSize, "expected minSize of a pool not to be greater than maxSize")
+
+			p.close(context.Background())
+		})
+		t.Run("minPoolSize may exceed maxPoolSize of 0", func(t *testing.T) {
+			t.Parallel()
+
+			p := newPool(poolConfig{MinPoolSize: 10, MaxPoolSize: 0})
+			assert.Equalf(t, uint64(10), p.minSize, "expected minSize of a pool to be greater than maxSize of 0")
+
+			p.close(context.Background())
+		})
 		t.Run("should be paused", func(t *testing.T) {
 			t.Parallel()
 

x/mongo/driver/topology/server_options.go

@@ -44,7 +44,6 @@ func newServerConfig(opts ...ServerOption) *serverConfig {
 	cfg := &serverConfig{
 		heartbeatInterval: 10 * time.Second,
 		heartbeatTimeout:  10 * time.Second,
-		maxConns:          100,
 		registry:          defaultRegistry,
 	}