internal/database: set db connection pool limits Under high-load, the frontend can issue many queries and trigger many connections. This can exceed the Cloud SQL connection limit of 100 per instance enforced by the Cloud Run environment, even if the database itself has remaining global capacity. To prevent this, we introduce limits on the database connection pool. We also include settings for idle connections and connection lifetimes to ensure better resource management and connection rotation. Introduces new env vars: GO_DISCOVERY_DATABASE_MAX_OPEN_CONNS GO_DISCOVERY_DATABASE_MAX_IDLE_CONNS GO_DISCOVERY_DATABASE_CONN_MAX_LIFETIME GO_DISCOVERY_DATABASE_CONN_MAX_IDLE_TIME Updates SetPoolSettings to validate that MaxIdleConns does not exceed MaxOpenConns, providing a warning if it does and capping it. Change-Id: I74edac05c4a23102d64e74a180c661c223e1b757 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/747620 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Nicholas Husin <nsh@golang.org> Reviewed-by: Nicholas Husin <husin@google.com> kokoro-CI: kokoro <noreply+kokoro@google.com>
diff --git a/cmd/internal/cmdconfig/cmdconfig.go b/cmd/internal/cmdconfig/cmdconfig.go index 1e08cd2..8bf80e8 100644 --- a/cmd/internal/cmdconfig/cmdconfig.go +++ b/cmd/internal/cmdconfig/cmdconfig.go
@@ -153,6 +153,7 @@ } log.Infof(ctx, "connected to secondary host %s", cfg.DBSecondaryHost) } + ddb.SetPoolSettings(cfg.DBMaxOpenConns, cfg.DBMaxIdleConns, cfg.DBConnMaxLifetime, cfg.DBConnMaxIdleTime) log.Infof(ctx, "database open finished") if bypassLicenseCheck { return postgres.NewBypassingLicenseCheck(ddb), nil
diff --git a/doc/config.md b/doc/config.md index d45bf70..3b98112 100644 --- a/doc/config.md +++ b/doc/config.md
@@ -8,6 +8,10 @@ | GO_DISCOVERY_CONFIG_BUCKET | Bucket use for dynamic configuration (gs://bucket/object) GO_DISCOVERY_CONFIG_DYNAMIC must be set if GO_DISCOVERY_CONFIG_BUCKET is set. | | GO_DISCOVERY_CONFIG_DYNAMIC | File that experiments are read from. Can be set locally using devtools/cmd/create_experiment_config/main.go. | | GO_DISCOVERY_DATABASE_HOST | Database server hostname. A primary and a secondary hosts are picked randomly. If the primary is not reachable, the system will fall back to the secondary host. | +| GO_DISCOVERY_DATABASE_MAX_OPEN_CONNS | Maximum number of open connections to the database. | +| GO_DISCOVERY_DATABASE_MAX_IDLE_CONNS | Maximum number of idle connections in the pool. | +| GO_DISCOVERY_DATABASE_CONN_MAX_LIFETIME | Maximum amount of time a connection may be reused. | +| GO_DISCOVERY_DATABASE_CONN_MAX_IDLE_TIME | Maximum amount of time a connection may be idle. | | GO_DISCOVERY_DATABASE_NAME | Name of database within the server. | | GO_DISCOVERY_DATABASE_PASSWORD | Password for database. | | | GO_DISCOVERY_DATABASE_USER | Used for frontend, worker and scripts. |
diff --git a/internal/config/config.go b/internal/config/config.go index 73924c4..bd5994d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go
@@ -84,6 +84,10 @@ DBSecret, DBUser, DBHost, DBPort, DBName, DBSSL string DBSecondaryHost string // DB host to use if first one is down DBPassword string `json:"-" yaml:"-"` + DBMaxOpenConns int + DBMaxIdleConns int + DBConnMaxLifetime time.Duration + DBConnMaxIdleTime time.Duration // Configuration for redis page cache. RedisCacheHost, RedisCachePort string @@ -146,6 +150,22 @@ // but we set this longer for the worker. const StatementTimeout = 30 * time.Minute +const ( + // DefaultDBMaxOpenConns is the default maximum number of open connections to the database. + // 80 is a safe value for Cloud Run instances, which have a hard limit of 100. + DefaultDBMaxOpenConns = 80 + + // DefaultDBMaxIdleConns is the default maximum number of idle connections in the pool. + // 10 is a conservative value for horizontal scaling. + DefaultDBMaxIdleConns = 10 + + // DefaultDBConnMaxLifetime is the default maximum amount of time a connection may be reused. + DefaultDBConnMaxLifetime = time.Hour + + // DefaultDBConnMaxIdleTime is the default maximum amount of time a connection may be idle. + DefaultDBConnMaxIdleTime = 10 * time.Minute +) + // SourceTimeout is the value of the timeout for source.Client, which is used // to fetch source code from third party URLs. const SourceTimeout = 1 * time.Minute
diff --git a/internal/config/serverconfig/config.go b/internal/config/serverconfig/config.go index bd98e46..7271569 100644 --- a/internal/config/serverconfig/config.go +++ b/internal/config/serverconfig/config.go
@@ -53,6 +53,22 @@ return fallback } +// GetEnvDuration looks up the given key from the environment and expects a duration string, +// returning the duration value if it exists, and otherwise returning the given +// fallback value. +// If the environment variable has a value but it can't be parsed as a duration, +// GetEnvDuration terminates the program. +func GetEnvDuration(ctx context.Context, key string, fallback time.Duration) time.Duration { + if s, ok := os.LookupEnv(key); ok { + v, err := time.ParseDuration(s) + if err != nil { + log.Fatalf(ctx, "bad value %q for %s: %v", s, key, err) + } + return v + } + return fallback +} + // ValidateAppVersion validates that appVersion follows the expected format // defined by AppVersionFormat. func ValidateAppVersion(appVersion string) error { @@ -153,6 +169,10 @@ DBName: GetEnv("GO_DISCOVERY_DATABASE_NAME", "discovery-db"), DBSecret: os.Getenv("GO_DISCOVERY_DATABASE_SECRET"), DBSSL: GetEnv("GO_DISCOVERY_DATABASE_SSL", "disable"), + DBMaxOpenConns: GetEnvInt(ctx, "GO_DISCOVERY_DATABASE_MAX_OPEN_CONNS", config.DefaultDBMaxOpenConns), + DBMaxIdleConns: GetEnvInt(ctx, "GO_DISCOVERY_DATABASE_MAX_IDLE_CONNS", config.DefaultDBMaxIdleConns), + DBConnMaxLifetime: GetEnvDuration(ctx, "GO_DISCOVERY_DATABASE_CONN_MAX_LIFETIME", config.DefaultDBConnMaxLifetime), + DBConnMaxIdleTime: GetEnvDuration(ctx, "GO_DISCOVERY_DATABASE_CONN_MAX_IDLE_TIME", config.DefaultDBConnMaxIdleTime), RedisCacheHost: os.Getenv("GO_DISCOVERY_REDIS_HOST"), RedisCachePort: GetEnv("GO_DISCOVERY_REDIS_PORT", "6379"), Quota: config.QuotaSettings{
diff --git a/internal/config/serverconfig/config_test.go b/internal/config/serverconfig/config_test.go index a489920..3301bd3 100644 --- a/internal/config/serverconfig/config_test.go +++ b/internal/config/serverconfig/config_test.go
@@ -8,6 +8,7 @@ "context" "regexp" "testing" + "time" "github.com/google/go-cmp/cmp" "golang.org/x/pkgsite/internal/config" @@ -145,3 +146,54 @@ } } } +func TestInitPoolSettings(t *testing.T) { + ctx := context.Background() + for _, test := range []struct { + name string + envs map[string]string + wantOpen int + wantIdle int + wantLife time.Duration + }{ + { + name: "overridden", + envs: map[string]string{ + "GO_DISCOVERY_DATABASE_HOST": "localhost", + "GO_DISCOVERY_DATABASE_MAX_OPEN_CONNS": "42", + "GO_DISCOVERY_DATABASE_MAX_IDLE_CONNS": "13", + "GO_DISCOVERY_DATABASE_CONN_MAX_LIFETIME": "10m", + }, + wantOpen: 42, + wantIdle: 13, + wantLife: 10 * time.Minute, + }, + { + name: "defaults", + envs: map[string]string{ + "GO_DISCOVERY_DATABASE_HOST": "localhost", + }, + wantOpen: config.DefaultDBMaxOpenConns, + wantIdle: config.DefaultDBMaxIdleConns, + wantLife: config.DefaultDBConnMaxLifetime, + }, + } { + t.Run(test.name, func(t *testing.T) { + for k, v := range test.envs { + t.Setenv(k, v) + } + cfg, err := Init(ctx) + if err != nil { + t.Fatal(err) + } + if cfg.DBMaxOpenConns != test.wantOpen { + t.Errorf("DBMaxOpenConns: got %d, want %d", cfg.DBMaxOpenConns, test.wantOpen) + } + if cfg.DBMaxIdleConns != test.wantIdle { + t.Errorf("DBMaxIdleConns: got %d, want %d", cfg.DBMaxIdleConns, test.wantIdle) + } + if cfg.DBConnMaxLifetime != test.wantLife { + t.Errorf("DBConnMaxLifetime: got %s, want %s", cfg.DBConnMaxLifetime, test.wantLife) + } + }) + } +}
diff --git a/internal/database/database.go b/internal/database/database.go index 3057163..bcb7c09 100644 --- a/internal/database/database.go +++ b/internal/database/database.go
@@ -63,6 +63,23 @@ return &DB{db: db, instanceID: instanceID} } +// Underlying returns the underlying *sql.DB. +func (db *DB) Underlying() *sql.DB { + return db.db +} + +// SetPoolSettings sets the connection pool settings for the database. +func (db *DB) SetPoolSettings(maxOpen, maxIdle int, maxLifetime, maxIdleTime time.Duration) { + if maxOpen > 0 && maxIdle > maxOpen { + log.Warningf(context.Background(), "SetPoolSettings: maxIdle (%d) > maxOpen (%d); capping maxIdle to maxOpen", maxIdle, maxOpen) + maxIdle = maxOpen + } + db.db.SetMaxOpenConns(maxOpen) + db.db.SetMaxIdleConns(maxIdle) + db.db.SetConnMaxLifetime(maxLifetime) + db.db.SetConnMaxIdleTime(maxIdleTime) +} + func (db *DB) Ping() error { return db.db.Ping() }
diff --git a/internal/database/pool_test.go b/internal/database/pool_test.go new file mode 100644 index 0000000..06fa048 --- /dev/null +++ b/internal/database/pool_test.go
@@ -0,0 +1,49 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package database + +import ( + "database/sql" + "testing" + "time" +) + +func TestSetPoolSettings(t *testing.T) { + // We use an empty DSN which the driver might reject, but sql.Open does not + // actually connect or ping. + sdb, err := sql.Open("postgres", "postgres://localhost/test?sslmode=disable") + if err != nil { + t.Fatal(err) + } + defer sdb.Close() + + db := New(sdb, "test") + + t.Run("valid settings", func(t *testing.T) { + maxOpen := 42 + maxIdle := 13 + maxLifetime := 10 * time.Minute + maxIdleTime := 5 * time.Minute + + db.SetPoolSettings(maxOpen, maxIdle, maxLifetime, maxIdleTime) + + stats := db.Underlying().Stats() + if stats.MaxOpenConnections != maxOpen { + t.Errorf("got %d, want %d", stats.MaxOpenConnections, maxOpen) + } + }) + + t.Run("maxIdle > maxOpen", func(t *testing.T) { + maxOpen := 10 + maxIdle := 20 + maxLifetime := 10 * time.Minute + maxIdleTime := 5 * time.Minute + + // This should log a warning and cap maxIdle to maxOpen. + db.SetPoolSettings(maxOpen, maxIdle, maxLifetime, maxIdleTime) + + // We can't easily check internal sql.DB settings, but we verify it doesn't crash. + }) +}