Skip to content

Commit 49a42ef

Browse files
dannykoppingdannykopping
authored
feat: make database connection pool size configurable (#21403)
Closes #21360 A few considerations/notes: - I've kept the number of conns to 10 in all other places, except coderd - which uses the config value - I opted to also make idle conns configurable; the greater the delta between max open and max idle, the more connection churn - Postgres maintains a [_process_ per connection](https://www.postgresql.org/docs/current/connect-estab.html), contrary to what the comment said previously - Operators should be able to tune this, since process churn can negatively affect OS scheduling - I've set the value to `"auto"` by default so it's not another knob one _has to_ twiddle, and sets max idle = max conns / 3 --------- Signed-off-by: Danny Kopping <danny@coder.com>
1 parent 61ae5b8 commit 49a42ef

13 files changed

Lines changed: 334 additions & 17 deletions

File tree

‎cli/server.go‎

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,16 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
747747
// "bare" read on this channel.
748748
var pubsubWatchdogTimeout <-chan struct{}
749749

750-
sqlDB, dbURL, err := getAndMigratePostgresDB(ctx, logger, vals.PostgresURL.String(), codersdk.PostgresAuth(vals.PostgresAuth), sqlDriver)
750+
maxOpenConns := int(vals.PostgresConnMaxOpen.Value())
751+
maxIdleConns, err := codersdk.ComputeMaxIdleConns(maxOpenConns, vals.PostgresConnMaxIdle.Value())
752+
if err != nil {
753+
return xerrors.Errorf("compute max idle connections: %w", err)
754+
}
755+
logger.Debug(ctx, "creating database connection pool", slog.F("max_open_conns", maxOpenConns), slog.F("max_idle_conns", maxIdleConns))
756+
sqlDB, dbURL, err := getAndMigratePostgresDB(ctx, logger, vals.PostgresURL.String(), codersdk.PostgresAuth(vals.PostgresAuth), sqlDriver,
757+
WithMaxOpenConns(maxOpenConns),
758+
WithMaxIdleConns(maxIdleConns),
759+
)
751760
if err != nil {
752761
return xerrors.Errorf("connect to postgres: %w", err)
753762
}
@@ -2324,14 +2333,45 @@ func IsLocalhost(host string) bool {
23242333
return host == "localhost" || host == "127.0.0.1" || host == "::1"
23252334
}
23262335

2336+
// PostgresConnectOptions contains options for connecting to Postgres.
2337+
type PostgresConnectOptions struct {
2338+
MaxOpenConns int
2339+
MaxIdleConns int
2340+
}
2341+
2342+
// PostgresConnectOption is a functional option for ConnectToPostgres.
2343+
type PostgresConnectOption func(*PostgresConnectOptions)
2344+
2345+
// WithMaxOpenConns sets the maximum number of open connections to the database.
2346+
func WithMaxOpenConns(n int) PostgresConnectOption {
2347+
return func(o *PostgresConnectOptions) {
2348+
o.MaxOpenConns = n
2349+
}
2350+
}
2351+
2352+
// WithMaxIdleConns sets the maximum number of idle connections in the pool.
2353+
func WithMaxIdleConns(n int) PostgresConnectOption {
2354+
return func(o *PostgresConnectOptions) {
2355+
o.MaxIdleConns = n
2356+
}
2357+
}
2358+
23272359
// ConnectToPostgres takes in the migration command to run on the database once
23282360
// it connects. To avoid running migrations, pass in `nil` or a no-op function.
23292361
// Regardless of the passed in migration function, if the database is not fully
23302362
// migrated, an error will be returned. This can happen if the database is on a
23312363
// future or past migration version.
23322364
//
23332365
// If no error is returned, the database is fully migrated and up to date.
2334-
func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, dbURL string, migrate func(db *sql.DB) error) (*sql.DB, error) {
2366+
func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, dbURL string, migrate func(db *sql.DB) error, opts ...PostgresConnectOption) (*sql.DB, error) {
2367+
// Apply defaults.
2368+
options := PostgresConnectOptions{
2369+
MaxOpenConns: 10,
2370+
MaxIdleConns: 3,
2371+
}
2372+
for _, opt := range opts {
2373+
opt(&options)
2374+
}
23352375
logger.Debug(ctx, "connecting to postgresql")
23362376

23372377
var err error
@@ -2414,19 +2454,12 @@ func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, d
24142454
// cannot accept new connections, so we try to limit that here.
24152455
// Requests will wait for a new connection instead of a hard error
24162456
// if a limit is set.
2417-
sqlDB.SetMaxOpenConns(10)
2418-
// Allow a max of 3 idle connections at a time. Lower values end up
2419-
// creating a lot of connection churn. Since each connection uses about
2420-
// 10MB of memory, we're allocating 30MB to Postgres connections per
2421-
// replica, but is better than causing Postgres to spawn a thread 15-20
2422-
// times/sec. PGBouncer's transaction pooling is not the greatest so
2423-
// it's not optimal for us to deploy.
2424-
//
2425-
// This was set to 10 before we started doing HA deployments, but 3 was
2426-
// later determined to be a better middle ground as to not use up all
2427-
// of PGs default connection limit while simultaneously avoiding a lot
2428-
// of connection churn.
2429-
sqlDB.SetMaxIdleConns(3)
2457+
sqlDB.SetMaxOpenConns(options.MaxOpenConns)
2458+
// Limit idle connections to reduce connection churn while keeping some
2459+
// connections ready for reuse. When a connection is returned to the pool
2460+
// but the idle pool is full, it's closed immediately - which can cause
2461+
// connection establishment overhead when load fluctuates.
2462+
sqlDB.SetMaxIdleConns(options.MaxIdleConns)
24302463

24312464
dbNeedsClosing = false
24322465
return sqlDB, nil
@@ -2830,7 +2863,7 @@ func signalNotifyContext(ctx context.Context, inv *serpent.Invocation, sig ...os
28302863
return inv.SignalNotifyContext(ctx, sig...)
28312864
}
28322865

2833-
func getAndMigratePostgresDB(ctx context.Context, logger slog.Logger, postgresURL string, auth codersdk.PostgresAuth, sqlDriver string) (*sql.DB, string, error) {
2866+
func getAndMigratePostgresDB(ctx context.Context, logger slog.Logger, postgresURL string, auth codersdk.PostgresAuth, sqlDriver string, opts ...PostgresConnectOption) (*sql.DB, string, error) {
28342867
dbURL, err := escapePostgresURLUserInfo(postgresURL)
28352868
if err != nil {
28362869
return nil, "", xerrors.Errorf("escaping postgres URL: %w", err)
@@ -2843,7 +2876,7 @@ func getAndMigratePostgresDB(ctx context.Context, logger slog.Logger, postgresUR
28432876
}
28442877
}
28452878

2846-
sqlDB, err := ConnectToPostgres(ctx, logger, sqlDriver, dbURL, migrations.Up)
2879+
sqlDB, err := ConnectToPostgres(ctx, logger, sqlDriver, dbURL, migrations.Up, opts...)
28472880
if err != nil {
28482881
return nil, "", xerrors.Errorf("connect to postgres: %w", err)
28492882
}

‎cli/testdata/coder_server_--help.golden‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@ OPTIONS:
6565
Type of auth to use when connecting to postgres. For AWS RDS, using
6666
IAM authentication (awsiamrds) is recommended.
6767

68+
--postgres-conn-max-idle string, $CODER_PG_CONN_MAX_IDLE (default: auto)
69+
Maximum number of idle connections to the database. Set to "auto" (the
70+
default) to use max open / 3. Value must be greater or equal to 0; 0
71+
means explicitly no idle connections.
72+
73+
--postgres-conn-max-open int, $CODER_PG_CONN_MAX_OPEN (default: 10)
74+
Maximum number of open connections to the database. Defaults to 10.
75+
6876
--postgres-url string, $CODER_PG_CONNECTION_URL
6977
URL of a PostgreSQL database. If empty, PostgreSQL binaries will be
7078
downloaded from Maven (https://repo1.maven.org/maven2) and store all

‎cli/testdata/server-config.yaml.golden‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,14 @@ ephemeralDeployment: false
483483
# authentication (awsiamrds) is recommended.
484484
# (default: password, type: enum[password\|awsiamrds])
485485
pgAuth: password
486+
# Maximum number of open connections to the database. Defaults to 10.
487+
# (default: 10, type: int)
488+
pgConnMaxOpen: 10
489+
# Maximum number of idle connections to the database. Set to "auto" (the default)
490+
# to use max open / 3. Value must be greater or equal to 0; 0 means explicitly no
491+
# idle connections.
492+
# (default: auto, type: string)
493+
pgConnMaxIdle: auto
486494
# A URL to an external Terms of Service that must be accepted by users when
487495
# logging in.
488496
# (default: <unset>, type: string)

‎coderd/apidoc/docs.go‎

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json‎

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎codersdk/deployment.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,10 @@ var PostgresAuthDrivers = []string{
442442
string(PostgresAuthAWSIAMRDS),
443443
}
444444

445+
// PostgresConnMaxIdleAuto is the value for auto-computing max idle connections
446+
// based on max open connections.
447+
const PostgresConnMaxIdleAuto = "auto"
448+
445449
// DeploymentValues is the central configuration values the coder server.
446450
type DeploymentValues struct {
447451
Verbose serpent.Bool `json:"verbose,omitempty"`
@@ -462,6 +466,8 @@ type DeploymentValues struct {
462466
EphemeralDeployment serpent.Bool `json:"ephemeral_deployment,omitempty" typescript:",notnull"`
463467
PostgresURL serpent.String `json:"pg_connection_url,omitempty" typescript:",notnull"`
464468
PostgresAuth string `json:"pg_auth,omitempty" typescript:",notnull"`
469+
PostgresConnMaxOpen serpent.Int64 `json:"pg_conn_max_open,omitempty" typescript:",notnull"`
470+
PostgresConnMaxIdle serpent.String `json:"pg_conn_max_idle,omitempty" typescript:",notnull"`
465471
OAuth2 OAuth2Config `json:"oauth2,omitempty" typescript:",notnull"`
466472
OIDC OIDCConfig `json:"oidc,omitempty" typescript:",notnull"`
467473
Telemetry TelemetryConfig `json:"telemetry,omitempty" typescript:",notnull"`
@@ -2623,6 +2629,30 @@ func (c *DeploymentValues) Options() serpent.OptionSet {
26232629
Value: serpent.EnumOf(&c.PostgresAuth, PostgresAuthDrivers...),
26242630
YAML: "pgAuth",
26252631
},
2632+
{
2633+
Name: "Postgres Connection Max Open",
2634+
Description: "Maximum number of open connections to the database. Defaults to 10.",
2635+
Flag: "postgres-conn-max-open",
2636+
Env: "CODER_PG_CONN_MAX_OPEN",
2637+
Default: "10",
2638+
Value: serpent.Validate(&c.PostgresConnMaxOpen, func(value *serpent.Int64) error {
2639+
if value.Value() <= 0 {
2640+
return xerrors.New("must be greater than zero")
2641+
}
2642+
return nil
2643+
}),
2644+
YAML: "pgConnMaxOpen",
2645+
},
2646+
{
2647+
Name: "Postgres Connection Max Idle",
2648+
Description: "Maximum number of idle connections to the database. Set to \"auto\" (the default) to use max open / 3. " +
2649+
"Value must be greater or equal to 0; 0 means explicitly no idle connections.",
2650+
Flag: "postgres-conn-max-idle",
2651+
Env: "CODER_PG_CONN_MAX_IDLE",
2652+
Default: PostgresConnMaxIdleAuto,
2653+
Value: &c.PostgresConnMaxIdle,
2654+
YAML: "pgConnMaxIdle",
2655+
},
26262656
{
26272657
Name: "Secure Auth Cookie",
26282658
Description: "Controls if the 'Secure' property is set on browser session cookies.",
@@ -4128,3 +4158,28 @@ func (c CryptoKey) CanVerify(now time.Time) bool {
41284158
beforeDelete := c.DeletesAt.IsZero() || now.Before(c.DeletesAt)
41294159
return hasSecret && beforeDelete
41304160
}
4161+
4162+
// ComputeMaxIdleConns calculates the effective maxIdleConns value. If
4163+
// configuredIdle is "auto", it returns maxOpen/3 with a minimum of 1. If
4164+
// configuredIdle exceeds maxOpen, it returns an error.
4165+
func ComputeMaxIdleConns(maxOpen int, configuredIdle string) (int, error) {
4166+
configuredIdle = strings.TrimSpace(configuredIdle)
4167+
if configuredIdle == PostgresConnMaxIdleAuto {
4168+
computed := maxOpen / 3
4169+
if computed < 1 {
4170+
return 1, nil
4171+
}
4172+
return computed, nil
4173+
}
4174+
idle, err := strconv.Atoi(configuredIdle)
4175+
if err != nil {
4176+
return 0, xerrors.Errorf("invalid max idle connections %q: must be %q or >= 0", configuredIdle, PostgresConnMaxIdleAuto)
4177+
}
4178+
if idle < 0 {
4179+
return 0, xerrors.Errorf("max idle connections must be %q or >= 0", PostgresConnMaxIdleAuto)
4180+
}
4181+
if idle > maxOpen {
4182+
return 0, xerrors.Errorf("max idle connections (%d) cannot exceed max open connections (%d)", idle, maxOpen)
4183+
}
4184+
return idle, nil
4185+
}

‎codersdk/deployment_test.go‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,3 +765,120 @@ func TestRetentionConfigParsing(t *testing.T) {
765765
})
766766
}
767767
}
768+
769+
func TestComputeMaxIdleConns(t *testing.T) {
770+
t.Parallel()
771+
772+
tests := []struct {
773+
name string
774+
maxOpen int
775+
configuredIdle string
776+
expectedIdle int
777+
expectError bool
778+
errorContains string
779+
}{
780+
{
781+
name: "auto_default_10_open",
782+
maxOpen: 10,
783+
configuredIdle: "auto",
784+
expectedIdle: 3, // 10/3 = 3
785+
},
786+
{
787+
name: "auto_with_whitespace",
788+
maxOpen: 10,
789+
configuredIdle: " auto ",
790+
expectedIdle: 3, // 10/3 = 3
791+
},
792+
{
793+
name: "auto_30_open",
794+
maxOpen: 30,
795+
configuredIdle: "auto",
796+
expectedIdle: 10, // 30/3 = 10
797+
},
798+
{
799+
name: "auto_minimum_1",
800+
maxOpen: 1,
801+
configuredIdle: "auto",
802+
expectedIdle: 1, // 1/3 = 0, but minimum is 1
803+
},
804+
{
805+
name: "auto_minimum_2_open",
806+
maxOpen: 2,
807+
configuredIdle: "auto",
808+
expectedIdle: 1, // 2/3 = 0, but minimum is 1
809+
},
810+
{
811+
name: "auto_3_open",
812+
maxOpen: 3,
813+
configuredIdle: "auto",
814+
expectedIdle: 1, // 3/3 = 1
815+
},
816+
{
817+
name: "explicit_equal_to_max",
818+
maxOpen: 10,
819+
configuredIdle: "10",
820+
expectedIdle: 10,
821+
},
822+
{
823+
name: "explicit_less_than_max",
824+
maxOpen: 10,
825+
configuredIdle: "5",
826+
expectedIdle: 5,
827+
},
828+
{
829+
name: "explicit_with_whitespace",
830+
maxOpen: 10,
831+
configuredIdle: " 5 ",
832+
expectedIdle: 5,
833+
},
834+
{
835+
name: "explicit_0",
836+
maxOpen: 10,
837+
configuredIdle: "0",
838+
expectedIdle: 0,
839+
},
840+
{
841+
name: "error_exceeds_max",
842+
maxOpen: 10,
843+
configuredIdle: "15",
844+
expectError: true,
845+
errorContains: "cannot exceed",
846+
},
847+
{
848+
name: "error_exceeds_max_by_1",
849+
maxOpen: 10,
850+
configuredIdle: "11",
851+
expectError: true,
852+
errorContains: "cannot exceed",
853+
},
854+
{
855+
name: "error_invalid_string",
856+
maxOpen: 10,
857+
configuredIdle: "invalid",
858+
expectError: true,
859+
errorContains: "must be \"auto\" or >= 0",
860+
},
861+
{
862+
name: "error_negative",
863+
maxOpen: 10,
864+
configuredIdle: "-1",
865+
expectError: true,
866+
errorContains: "must be \"auto\" or >= 0",
867+
},
868+
}
869+
870+
for _, tt := range tests {
871+
t.Run(tt.name, func(t *testing.T) {
872+
t.Parallel()
873+
874+
result, err := codersdk.ComputeMaxIdleConns(tt.maxOpen, tt.configuredIdle)
875+
if tt.expectError {
876+
require.Error(t, err)
877+
require.Contains(t, err.Error(), tt.errorContains)
878+
} else {
879+
require.NoError(t, err)
880+
require.Equal(t, tt.expectedIdle, result)
881+
}
882+
})
883+
}
884+
}

‎docs/reference/api/general.md‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)