---
id: pg-fix-open-path
name: "Fix `max_size: 0` open hang + DSN options override (review 002 Finding 3 + options note)"
status: completed
depends_on: []
scope: single
risk: low
impact: component
level: implementation
tags: [wave-4-fixes, postgres-engine, open]
---

## Description

Two open-path fixes in `alkstore-postgres/src/store.rs`
(`open_store`), from the wave-4 general review:

**Finding 3 (MEDIUM, live-proven)**: `PgOpts { max_size: 0 }` hangs
`open` forever. Site: `store.rs:162-165` + the bootstrap `pool.get()`
at `store.rs:177` — deadpool (0.13/0.14) does not validate `max_size`
and defaults to no timeouts, so a `max_size: 0` pool can never grant a
checkout: `open` blocks forever at the bootstrap checkout (no
`BuildError`, no default pool timeouts) instead of failing typed.
Probe (removed): `open` with `max_size: 0` timed out at 5 s, never
returned.

Fix shape (the review's): validate `max_size > 0` in `open_store` —
typed `Error::Database` **before any round trip** (the engine-wide
validate-at-entry posture; the source-chain detail should say what was
rejected, e.g. "max_size must be positive"). The alternative shape
(configure deadpool timeouts so the hang surfaces as an error) is
weaker — a timeout-configured hang still burns the consumer's time;
prefer the guard. Do both only if trivial.

**Minor note (same file, same surface)**: `store.rs:152-156` —
`pool_cfg.options(...)` **replaces** any `options=` present in the
consumer's DSN (tokio-postgres's setter semantics) — a silent config
override. Preferred fix: **append** to the DSN's existing options
string (tokio-postgres `Config::get_options()` reads what the parse
carried) so the engine's `synchronous_commit` SET rides alongside
whatever the consumer configured. If appending proves impractical,
document at the site and in the crate docs that engine-configured
options win — but append is the honest default (the consumer's DSN
options were there first).

Tests (against the harness server): `max_size: 0` fails typed and
*promptly* (assert the error; the test's own structure bounds the
hang — a regression re-hangs and fails the test by timeout); a DSN
carrying pre-existing `options=` plus `open` succeeds and
`synchronous_commit` is still observable via `SHOW` on both settings
(the append path doesn't break the engine's SET); the existing
opts-flow tests stay green.

## Acceptance Criteria

- [x] `max_size: 0` fails `open` with a typed `Database` error before
      any round trip — pinned by test (prompt failure, no hang)
- [x] The consumer DSN's `options=` survive: appended, not replaced —
      pinned by test (DSN with options + engine SET coexist, `SHOW`
      observable)
- [ ] If append proved impractical instead: the engine-wins semantics
      documented at the site and in crate docs (record the decision in
      Notes) — *n/a: append landed*
- [x] Existing open/opts tests stay green
- [x] `cargo test -p alkstore-postgres` (harness server), clippy
      `-D warnings`, fmt clean; gates green server-less

## References

- docs/reviews/002-wave-4-general-review.md §Finding 3 + §Minor notes
  (the options bullet)
- docs/architecture/engine-postgres.md (Connection architecture)
- docs/architecture/decisions/008-contract-v1-pinning.md §6 (config
  split)
- tasks/pg-engine-open-opts.md (the open task of record; Notes carry
  the connect-options SET mechanics this task extends)

## Notes

Decisions of record made while implementing (the description didn't
pin them):

- **Both fix shapes' primary option landed (the guard; no deadpool
  timeout config added)**: `open_store` rejects `opts.max_size == 0`
  at entry — `database_error("max_size must be positive, got 0")`
  (the value rides the message; the source chain says what was
  rejected) — *before* the config parse and any round trip. The
  timeout-configured-hang alternative was not taken (the review ranked
  it weaker; not trivially worth doing both — deadpool 0.13/0.14 has
  no default pool timeouts to lean on and the guard is total).
- **Append is practical and landed** (the task's preferred fix; the
  n/a acceptance row): tokio-postgres 0.7.18's
  `Config::get_options() -> Option<&str>` returns the raw options
  string the parse (or a prior programmatic `options()`) carried, so
  `open_store` reads it, appends
  ` -c synchronous_commit={on|off}`, and calls the setter — the
  consumer's `options=` ride every pooled connection alongside the
  engine's SET. Postgres parses repeated `-c` flags in order, so if
  the consumer's own options set `synchronous_commit`, the engine's
  later `-c` wins *for that knob* (it is the knob `PgOpts` owns) and
  every other consumer setting lives untouched. The listener path
  (`cfg.clone()` + `application_name` only) is unchanged — the SET is
  a pool-connection concern. Documented at the site and on
  `PgOpts::max_size` (0 rejected immediately) in `opts.rs`.
- **Tests**: `open_fails_database_promptly_on_zero_max_size` pins the
  guard — typed `Database`, source chain containing "max_size" +
  "positive", bounded by an inner `tokio::time::timeout(2s)` so a
  regression re-hangs and fails here by timeout; it runs server-less
  too (unreachable-DSN fallback — the guard fires before any
  connection attempt, which is exactly the "before any round trip"
  property being pinned). `consumer_dsn_options_survive_alongside_the_engine_set`
  pins the append: DSN `options='-c statement_timeout=30000'` → open
  succeeds, `SHOW statement_timeout` = `30s` (consumer's survived)
  and `SHOW synchronous_commit` = on/off per `PgOpts` (engine's SET
  rides) — both knob values asserted for both settings, against the
  harness server.
- **The `PgOpts::max_size` doc on `opts.rs` now states the 0
  rejection** (the constructor's validation is part of the option's
  contract now that it is typed-failed at open).

## Summary

Both open-path fixes landed in `alkstore-postgres/src/store.rs`
(`open_store`). Finding 3: a `max_size > 0` entry guard fires before
the config parse and any round trip — `max_size: 0` now fails `open`
with the typed `Error::Database` ("max_size must be positive, got 0")
instead of hanging forever at the bootstrap checkout (deadpool
0.13/0.14 neither validates `max_size` nor defaults timeouts). Minor
note: the engine's connect-options SET (`-c synchronous_commit=…`) is
now *appended* to the parse-carried `options=` string (read back via
`tokio_postgres::Config::get_options()`) rather than replacing it —
the consumer's DSN options survive on every pooled connection
alongside the engine's knob; repeated `-c` flags are
order-processed by postgres so the engine still owns
`synchronous_commit` if it collides. Pinned by two new tests in
`src/store/open_tests.rs` (the zero-guard test with a bounded-hang
timeout structure, server-less-capable; the append test asserting
both `SHOW statement_timeout` and `SHOW synchronous_commit` across
both knob settings against the harness server). Verified: 118/118 pg
lib tests + contract-suite/schema suites green against the harness
(pglo-poc :15432), workspace `cargo test` green server-less (283
tests, pg skipping per convention), `cargo clippy --all-targets -- -D
warnings` and `cargo fmt --check` clean workspace-wide.