Postgres: statement_timeout is applied to one pooled connection, so ~9 of 10 connections run uncapped

[mauripunzueta]

Summary

PostgresConfig::statement_timeout_ms is applied to exactly one pooled connection and then that connection is returned to the pool. SET is session-scoped, so the other max_connections - 1 connections (default: 9 of 10) never receive it and run with the server's default — usually no timeout at all.

Found while reviewing #212 / #275, which fixed a sibling defect in the same struct (connect_timeout_secs was declared, defaulted, serialized and never read).

The code

crates/persistence/src/backends/postgres/backend.rs, in PostgresBackend::new:

let pool = Self::create_pool(&config)?;

// Verify connectivity
let client = pool.get().await…?;

// Set statement timeout
client
    .execute(&format!("SET statement_timeout = {}", config.statement_timeout_ms), &[])
    .await…?;

drop(client);   // ← back to the pool, still the ONLY connection carrying the SET

The connection checked out to verify connectivity is the one that gets the SET. Every connection deadpool creates afterwards, on demand up to max_connections, starts a fresh session with no statement_timeout.

So the timeout is enforced nondeterministically, on whichever fraction of requests happens to land on that one connection.

Why this is more than a config nit

The benchmark data in #224 and #227 may be partly an artifact of this bug.

#227 reports for PG EX02: 6 rps, ~13% err, p95 = 30s (statement_timeout). #224 reports a composite search "that hits the 30s statement_timeout and returns 500".

With default_max_connections() == 10 and exactly one connection carrying the SET, roughly 1 request in 10 should be killed at 30s while the other nine run unbounded. A measured error rate of ~13% sits right on top of that prediction, and 30s is exactly default_statement_timeout_ms(). There is no server-side statement_timeout configured anywhere in .github/ or the Dockerfiles, so the 30s kills observed in those runs can only be coming from this one connection.

If that reading is right, those suites are not measuring "the query takes >30s" uniformly — they are measuring a coin flip over which connection served the request. Worth confirming before either issue's numbers are trusted as a baseline.

Why the fix needs care (and its own PR)

The obvious fix — a deadpool post_create hook that issues the SET on every new connection — would newly enforce a 30s cap on queries that have never been capped. Per #224 and #279, real composite searches at volume run for 12s median and one shape reaches ~500s. Making the timeout real would convert those from slow-but-successful into hard failures.

That may well be the correct outcome (a 500s query is not serving anyone), but it is a behaviour change that needs its own load validation and its own decision about the default — which is why #275 deliberately left it alone rather than smuggling it in.

Related dead config in the same struct

PostgresConfig::schema_name (backend.rs, "Optional schema name for schema-per-tenant isolation") is likewise never read by any query path — only written as None in Default and asserted in one test. The actual schema-per-tenant machinery lives in crates/persistence/src/strategy/schema_per_tenant.rs and has its own separate config (shared_schema), so nothing is broken today; there is also no env wiring that could set it. It should either be wired to search_path or deleted, so the struct stops advertising isolation it does not provide.

That makes three fields in PostgresConfig declared-but-not-honoured, one of which (connect_timeout_secs) is fixed in #275. A sweep for the pattern is probably worth more than three separate fixes.

Acceptance

• statement_timeout is applied to every pooled connection, not just the first.
• The change is load-validated against the shapes in Postgres FHIR search is slow at full data volume (med ~7s, p99 ~51s, one composite query 500s) #224 / Postgres composite search is I/O-bound at volume — needs a denormalized one-row-per-group storage layout #279 before merge, with an explicit decision on the default.
• Postgres FHIR search is slow at full data volume (med ~7s, p99 ~51s, one composite query 500s) #224's and HTS Terminology Server perf regression on main since v0.2.0 (EX02/EX08 expansion collapse + ~30% uniform drop) #227's error rates are re-measured afterwards, since they may move once the timeout applies uniformly.
• schema_name is either honoured or removed.