database: Postgres/AlloyDB network reliability — failover-safe pool + transactional retry with commit verification

[elovelan]

Problem

Postgres/AlloyDB connections fail during brief network outages — most notably AlloyDB's monthly maintenance switchovers (< 1s downtime), but also any transient network blip. Today:

1. Dead connections linger in the pool. database/sql with the pgx driver has no background connection health checks, so after a switchover, dead connections sit in the pool and get handed to the application.
2. No pool limits by default. ConnectionsConfig defaults to all zero values, so database/sql holds connections forever.
3. In-flight queries have no retry path. A query that fails mid-flight (connection severed) just returns an error to the caller — no retry, no indication whether a write landed.

The result: services return 500s during maintenance windows, and callers have to guess whether a write committed after a network error.

What this does

Three layers, each independently useful:

1. Failover-safe connection pool (postgres.go, model_config.go, database.go)

• Switch from sql.Open("pgx", …) to pgxpool.NewWithConfig() + stdlib.OpenDBFromPool(), keeping the *sql.DB return type so no downstream changes are needed.
• HealthCheckPeriod = 1s — a background goroutine evicts connections that exceeded MaxConnLifetime or MaxConnIdleTime every second. (It does not ping — liveness pinging happens at acquire time via stdlib's ResetSession, default "ping if idle > 1s"; database/sql then retries on a fresh conn. HealthCheckPeriod just keeps expired/idle connections from accumulating.)
• DefaultPostgresConnectionsConfig() + ApplyPostgresConnectionsConfig() — failover-safe defaults (MaxLifetime=5m, MaxIdleTime=30s, MaxOpen=25, MaxIdle=5) applied automatically when services don't explicitly configure them.

2. Retry helpers (retry.go, postgres.go)

Two functions so the common case (selects, idempotent DML) doesn't carry "unsafe" wording or transaction overhead:

• RetryIdempotent(ctx, opts, fn func() error) — retries fn (no transaction wrapper) on any error by default except context.Canceled/context.DeadlineExceeded. The caller vouches that fn is idempotent. Lightweight; for selects and idempotent DML.
• RetryPostgresNonIdempotent(ctx, db, opts, fn func(*sql.Tx) error) — runs fn in a Postgres transaction and retries the whole transaction on transient errors. fn receives a fresh *sql.Tx each attempt. For non-idempotent operations (the transaction makes retry safe via rollback, plus commit verification below). Shape matches pgx.BeginTxFunc / pgxtransactions.Transaction.

Both use a non-configurable maxRetryAttempts = 3 (matching database/sql's maxBadConnRetries+1), full-jitter backoff (up to 100ms), and respect the caller's context deadline.

RetryMySQLNonIdempotent / RetrySpannerNonIdempotent are placeholders (return "not yet implemented") so the cross-backend API surface is visible.

3. Commit verification (postgres.go, retry.go)

The hard part of retrying transactions: a commit error is ambiguous — the commit may have succeeded on the server before the error was returned to the client (e.g., the connection was severed after COMMIT was sent but before the response). Blindly retrying could duplicate committed work.

RetryPostgresNonIdempotent resolves this by asking the server, per the PostgreSQL docs (which explicitly endorse pg_xact_status for "determine whether their transaction committed or aborted after the application and database server become disconnected while a COMMIT is in progress"):

1. Before each Commit, capture the transaction id: SELECT pg_current_xact_id_if_assigned()::text (NULL → read-only — no writes to duplicate, safe to retry without verifying).
2. On a commit error, query SELECT pg_xact_status($1::xid8) on a fresh connection (the original may be dead; xid8 is globally unique so this works across connections/reconnects, modulo a brief failover replay-lag window):
   • 'aborted' → retry (the commit didn't land).
   • 'committed' → ErrCommitted (don't retry — would duplicate; the caller reconciles/alerts).
   • NULL / 'in progress' / query error → ErrCommitPhase (inconclusive; caller decides).

Commit errors are always verified (not classified by a client-side guess) — the server is the authority on whether a commit landed. Class-40 commit errors (40001 serialization, 40P01 deadlock — server rolled back) → pg_xact_status says 'aborted' → retry. A retryable commit error (verified-aborted or read-only) on exhaustion returns the original error (not ErrCommitPhase — it's known non-commit, just out of retries). ErrCommitPhase is only for inconclusive commits.

Retry design summary

• Pre-commit errors (BeginTx/fn): opt-OUT classifier — pre-commit retry is always safe (the transaction rolls back), so it retries everything except context.Canceled/context.DeadlineExceeded and the permanent SQLSTATE classes (22xxx data exceptions, 23xxx constraint violations, 42xxx syntax/schema/permission — via pgerrcode). Unknown codes and non-PgError errors are retried (a false positive wastes ~200ms; a false negative causes a spurious user-facing failure — the former is safer).
• Commit errors: always verify via pg_xact_status (above).
• opts.IsRetryable is additive (OR) on the pre-commit classifier (commit errors are verified, not classified — safer; a caller shouldn't override commit verification).

Sentinels

if errors.Is(err, database.ErrCommitted) {
    // Verified committed: do NOT retry (would duplicate). Reconcile/alert.
}
if errors.Is(err, database.ErrCommitPhase) {
    // Inconclusive: the commit may or may not have landed. Decide / re-check.
}

Both preserve the underlying commit error for errors.Is/errors.As.

Other

• pgerrcode: adopted github.com/jackc/pgerrcode (canonical Postgres error-code constants, by the pgx author) to eliminate string literals and inline human-friendly-name comments.
• Naming: RetryIdempotent (no tx) / RetryPostgresNonIdempotent (tx + verification) — the Tx suffix was dropped (the fn func(*sql.Tx) error parameter makes the transaction obvious). The existing RetryPostgres is removed (this is unmerged work).
• database/sql interaction: *sql.DB methods already retry driver.ErrBadConn internally (immediate, up to 3 attempts); this outer retry layers on top with jitter for longer outages. *sql.Tx methods have no internal retry, so this is the only retry for tx.Exec/tx.Commit.

Test plan

• TestRetryIdempotent — success, retries on any error by default (incl. 23505), custom IsRetryable short-circuits, no retry on context.Canceled, ctx cancellation, attempt exhaustion.
• TestRetryPostgresNonIdempotent — success; fn-error-then-succeed (rollback between attempts); non-retryable fn error short-circuits (23505); driver.ErrBadConn from fn retried; SafeToRetry Begin failure retried; SafeToRetry Commit failure → verified-aborted → retry; verify-aborted → retry; verify-committed → ErrCommitted; verify-unknown (NULL) → ErrCommitPhase; verify-error → ErrCommitPhase; read-only (no xid) → retry without verifying; class-40 exhaustion → verify-aborted each attempt → original error (not ErrCommitPhase); 57P01+inconclusive → ErrCommitPhase; additive opts.IsRetryable (pre-commit); ctx cancellation; attempt exhaustion. Uses a database/sql/driver mock (with QueryerContext to script the verification queries).
• TestIsRetryablePostgresPreCommitError — pre-commit classifier leg coverage.
• TestErrCommitPhase / TestErrCommitted — sentinel detection; underlying error preserved; distinct from each other.
• TestRetryMySQLNonIdempotentNotImplemented / TestRetrySpannerNonIdempotentNotImplemented — placeholders return "not yet implemented".
• Existing IsRetryablePostgresError / PostgresUniqueViolation / PostgresDeadlockFound / pool-defaults tests still pass.
• go build ./..., go vet ./..., go test ./database/... -short all pass.

Origin

This combines the failover-safe pool work (upstream moov-io/base PR #490) with the transaction-retry pattern suggested in its review (r3470551946, r3487236671, r3482973525, r3483137216).