Files
multica/server/cmd/server/dbstats_test.go
LinYushen 07034f4455 feat(server): configurable pgxpool size with sane defaults (#1381)
* feat(server): configurable pgxpool size with sane defaults

pgxpool.New(ctx, url) silently sets MaxConns = max(4, NumCPU). On the
prod pods that resolved to 4, which got fully saturated by daemon
claim/heartbeat traffic (~3800 acquires/s) and showed up as ~900ms
acquire waits on every query — the actual root cause of the 3s+
/tasks/claim tail latency. The db pool stats logging from #1378
confirmed this with empty_acquire_delta == acquire_count_delta.

Switch to pgxpool.ParseConfig + NewWithConfig and apply per-pod
defaults of MaxConns=25 / MinConns=5, both overridable via env vars
(DATABASE_MAX_CONNS / DATABASE_MIN_CONNS) so the size can be tuned
in prod without a redeploy.

The defaults follow the standard 'small pool, lots of waiters' guidance
for Postgres (PG community / HikariCP formula
`(core_count * 2) + effective_spindle_count`); 25 leaves headroom for
bursts and occasional long queries while staying safely under typical
managed-Postgres max_connections ceilings when multiplied across pods.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(server): respect DATABASE_URL pool_* params; add precedence tests

Address review feedback on #1381:

- Configuration precedence is now explicit: DATABASE_MAX_CONNS env >
  pool_max_conns query param on DATABASE_URL > built-in default. Same
  for min_conns. Previously the env-empty path unconditionally
  overwrote whatever ParseConfig had read from the URL — a silent
  regression for deployments that already tuned pool size via the
  connection string.
- Add unit tests in dbstats_test.go covering each precedence branch
  (defaults, URL-only, env-over-URL, partial URL, invalid env,
  min>max clamp).
- Move pool tuning vars out of 'Required Variables' into a new
  'Database Pool Tuning (Optional)' section in SELF_HOSTING_ADVANCED.md
  so self-hosters don't think they need to set them.
- Add commented entries in .env.example.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(server): invalid pool env falls back to URL/code default, never pgx 4

Address second round of review on #1381:

Previous code passed cfg.MaxConns / cfg.MinConns as the envInt32 fallback,
which meant an invalid DATABASE_MAX_CONNS value silently fell back to
ParseConfig's value — i.e. pgx's built-in default of 4/0 when the URL had
no pool_* params. That's exactly the bad value this PR exists to remove,
and the previous test (TestPoolSizing_InvalidEnvFallsBack) accidentally
locked it in.

Compute the non-env fallback first (URL pool_* if present, else code
default 25/5) and pass that to envInt32. Misconfigured env now lands on
the same value as if the env were unset — never on the pgx default.

Replace the loose 'max > 0' assertion with two precise tests:
- invalid env + no URL param → code default (25/5)
- invalid env + URL param    → URL value

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-20 17:07:19 +08:00

110 lines
3.6 KiB
Go

package main
import (
"testing"
"github.com/jackc/pgx/v5/pgxpool"
)
// applyPoolSizing mirrors the env+URL precedence logic in newDBPool but
// without actually opening a connection, so the resolution rules can be
// asserted in unit tests.
func applyPoolSizing(t *testing.T, dbURL string, envMax, envMin string) (max, min int32) {
t.Helper()
cfg, err := pgxpool.ParseConfig(dbURL)
if err != nil {
t.Fatalf("ParseConfig: %v", err)
}
urlParams := poolParamsFromURL(dbURL)
maxFallback := defaultMaxConns
if urlParams["pool_max_conns"] {
maxFallback = cfg.MaxConns
}
if envMax != "" {
t.Setenv("DATABASE_MAX_CONNS", envMax)
}
cfg.MaxConns = envInt32("DATABASE_MAX_CONNS", maxFallback)
minFallback := defaultMinConns
if urlParams["pool_min_conns"] {
minFallback = cfg.MinConns
}
if envMin != "" {
t.Setenv("DATABASE_MIN_CONNS", envMin)
}
cfg.MinConns = envInt32("DATABASE_MIN_CONNS", minFallback)
if cfg.MinConns > cfg.MaxConns {
cfg.MinConns = cfg.MaxConns
}
return cfg.MaxConns, cfg.MinConns
}
func TestPoolSizing_DefaultsWhenNothingSet(t *testing.T) {
max, min := applyPoolSizing(t, "postgres://u:p@h/db?sslmode=disable", "", "")
if max != defaultMaxConns || min != defaultMinConns {
t.Fatalf("got max=%d min=%d, want %d/%d", max, min, defaultMaxConns, defaultMinConns)
}
}
func TestPoolSizing_URLParamsHonoredWhenEnvUnset(t *testing.T) {
url := "postgres://u:p@h/db?sslmode=disable&pool_max_conns=40&pool_min_conns=8"
max, min := applyPoolSizing(t, url, "", "")
if max != 40 || min != 8 {
t.Fatalf("URL params should win when env unset; got max=%d min=%d", max, min)
}
}
func TestPoolSizing_EnvOverridesURL(t *testing.T) {
url := "postgres://u:p@h/db?sslmode=disable&pool_max_conns=40&pool_min_conns=8"
max, min := applyPoolSizing(t, url, "100", "20")
if max != 100 || min != 20 {
t.Fatalf("env should win over URL; got max=%d min=%d", max, min)
}
}
func TestPoolSizing_PartialURLParam(t *testing.T) {
// Only pool_max_conns is set in URL — pool_min_conns should fall back to
// the code default, not pgx's built-in default (which would be 0).
url := "postgres://u:p@h/db?sslmode=disable&pool_max_conns=40"
max, min := applyPoolSizing(t, url, "", "")
if max != 40 {
t.Fatalf("URL pool_max_conns should be honored; got max=%d", max)
}
if min != defaultMinConns {
t.Fatalf("min should default; got min=%d, want %d", min, defaultMinConns)
}
}
func TestPoolSizing_InvalidEnvFallsBackToCodeDefault(t *testing.T) {
// Invalid env value with no URL pool param → code default, NOT pgx's
// built-in 4. This is the regression that was fixed; pinning it here
// so we don't silently fall back to the bad value again.
max, min := applyPoolSizing(t, "postgres://u:p@h/db?sslmode=disable", "not-a-number", "")
if max != defaultMaxConns {
t.Fatalf("invalid env should fall back to code default; got max=%d, want %d", max, defaultMaxConns)
}
if min != defaultMinConns {
t.Fatalf("got min=%d, want %d", min, defaultMinConns)
}
}
func TestPoolSizing_InvalidEnvFallsBackToURLParam(t *testing.T) {
// Invalid env value with a URL pool param → URL param wins, NOT pgx
// default. This is what makes the precedence chain end at "URL or code
// default" rather than at "pgx default" on misconfiguration.
url := "postgres://u:p@h/db?sslmode=disable&pool_max_conns=40"
max, _ := applyPoolSizing(t, url, "not-a-number", "")
if max != 40 {
t.Fatalf("invalid env should fall back to URL param; got max=%d, want 40", max)
}
}
func TestPoolSizing_MinClampedToMax(t *testing.T) {
max, min := applyPoolSizing(t, "postgres://u:p@h/db?sslmode=disable", "10", "50")
if min > max {
t.Fatalf("min should be clamped to max; got max=%d min=%d", max, min)
}
}