diff options
| author | Hana Kim <hyangah@gmail.com> | 2026-02-23 15:38:50 -0500 |
|---|---|---|
| committer | Hyang-Ah Hana Kim <hyangah@gmail.com> | 2026-02-23 17:14:17 -0800 |
| commit | 1c7acd4bf511b868c084297235f6f38db2264b21 (patch) | |
| tree | dc8ab29b005727a6e79f8d37f56f01839b1d72fb | |
| parent | f4bdadf886f3b016bb399240f9337d35eb29e5f0 (diff) | |
| download | go-x-pkgsite-1c7acd4bf511b868c084297235f6f38db2264b21.tar.xz | |
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>
| -rw-r--r-- | cmd/internal/cmdconfig/cmdconfig.go | 1 | ||||
| -rw-r--r-- | doc/config.md | 4 | ||||
| -rw-r--r-- | internal/config/config.go | 20 | ||||
| -rw-r--r-- | internal/config/serverconfig/config.go | 20 | ||||
| -rw-r--r-- | internal/config/serverconfig/config_test.go | 52 | ||||
| -rw-r--r-- | internal/database/database.go | 17 | ||||
| -rw-r--r-- | internal/database/pool_test.go | 49 |
7 files changed, 163 insertions, 0 deletions
diff --git a/cmd/internal/cmdconfig/cmdconfig.go b/cmd/internal/cmdconfig/cmdconfig.go index 1e08cd25..8bf80e87 100644 --- a/cmd/internal/cmdconfig/cmdconfig.go +++ b/cmd/internal/cmdconfig/cmdconfig.go @@ -153,6 +153,7 @@ func OpenDB(ctx context.Context, cfg *config.Config, bypassLicenseCheck bool) (_ } 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 d45bf70b..3b98112f 100644 --- a/doc/config.md +++ b/doc/config.md @@ -8,6 +8,10 @@ Pkgsite uses these environment variables: | 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 73924c42..bd5994d2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -84,6 +84,10 @@ type Config struct { 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 @@ func (c *Config) AppVersionLabel() string { // 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 bd98e465..72715696 100644 --- a/internal/config/serverconfig/config.go +++ b/internal/config/serverconfig/config.go @@ -53,6 +53,22 @@ func GetEnvInt(ctx context.Context, key string, fallback int) int { 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 @@ func Init(ctx context.Context) (_ *config.Config, err error) { 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 a4899205..3301bd36 100644 --- a/internal/config/serverconfig/config_test.go +++ b/internal/config/serverconfig/config_test.go @@ -8,6 +8,7 @@ import ( "context" "regexp" "testing" + "time" "github.com/google/go-cmp/cmp" "golang.org/x/pkgsite/internal/config" @@ -145,3 +146,54 @@ func TestEnvAndApp(t *testing.T) { } } } +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 30571632..bcb7c09a 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -63,6 +63,23 @@ func New(db *sql.DB, instanceID string) *DB { 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 00000000..06fa048d --- /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. + }) +} |
