diff options
| -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. + }) +} |
