importer: bound verification queries server-side, and add validation-query for the ad-hoc ones #209

Merged

art merged 2 commits from importer-validation-timeouts into main

2026-08-22 21:22:52 +00:00

art-bot commented

2026-08-21 18:32:38 +00:00

Collaborator

Two commits. The first closes a hole in machinery already relied on; the second
adds the missing half.

1. The acceptance timeout was client-side only

A test's timeout: was a context.WithTimeout and nothing else. pgx cancels a
query when the context expires, but a cancel is a message the client sends,
so the bound was only ever as good as the client's liveness and its error
handling — not the guarantee the mechanism was built to give.

runAcceptanceTests now takes its connection through acquireBounded, which
sets statement_timeout from the test's own timeout: and lock_timeout to 5 s,
and resets both before the connection returns to the pool (a leaked 10 s
statement_timeout on a reused connection would turn the safety net into a
source of spurious failures).

lock_timeout is the other half: a verification query has no business queueing
behind DDL. An import's swap takes AccessExclusiveLock on the partition, and a
check that waits for it both stalls itself and — holding AccessShareLock on
what it has already read — can stall the swap.

Measured: a test declaring timeout: 10 against SELECT pg_sleep(300) fails
at 10.002 s with SQLSTATE 57014 instead of running to completion.

It caught a real mis-declaration. conf/pl's buildings R120 —
sum(ST_NumGeometries(geom)) over ~9.5M multipolygons — has been reported as an
intermittent 10 s timeout in #101, #107 and #108. It is not intermittent and it
is not the machine: it takes 19.3 s, and only looked flaky because the
client-side cancel raced it. Moved to the 100 rung, where its sibling urząd check
already sits.

Full sweep under --acceptance all: is 42/42, se 55/55, pl 52/52, fi 57/57,
gr 60/60, world 34/34.

Not claimed: that this rescues a query whose client was SIGKILLed. On this box
a killed importer's backend went away within seconds even with
client_connection_check_interval at its 0 default — not isolated, and not a
property to lean on. The point is that the bound no longer depends on finding out.

2. `importer validation-query <config> <file.sql>`

The motivating incident: a verification query written during an investigation ran
for 9h40m, held AccessShareLock on a partition an import then needed to
swap, and blocked that import for 7h36m. The import looked slow; it was
queued. Ad-hoc verification had no bound because a single-shot query has no
business being committed to a config file just to inherit one.

Same acquireBounded connection, so the same statement_timeout and
lock_timeout; --timeout is on the same ladder (1, 10, 100, 1000) and defaults
to 1000.

Deliberately not a general SQL runner:

BEGIN READ ONLY — a file that tries to write is refused by the server
(SQLSTATE 25006), structural rather than a promise
a file argument, not an inline string — complex SQL through shell quoting is
how queries get mangled, and the file survives the run
the elapsed time is printed on purpose: a query worth keeping should
graduate into the owning layer's or contract's acceptance_tests: at the
rung its measured runtime earns. validation-query is the scratchpad, the
config is the durable record — the relationship the deep tier already has with
the fast tier.

Verified live: a normal query returns rows + runtime; pg_sleep(300) at
--timeout 10 fails at 10.001 s; CREATE TABLE is refused and the table does not
exist afterwards; --timeout 25 is rejected as off-ladder. Offline CLI tests
cover argument/ladder validation and the help path. docs/importer-cli.md
documents both halves.

Two commits. The first closes a hole in machinery already relied on; the second adds the missing half. ## 1. The acceptance timeout was client-side only A test's `timeout:` was a `context.WithTimeout` and nothing else. pgx cancels a query when the context expires, but a cancel is a message the **client** sends, so the bound was only ever as good as the client's liveness and its error handling — not the guarantee the mechanism was built to give. `runAcceptanceTests` now takes its connection through `acquireBounded`, which sets `statement_timeout` from the test's own `timeout:` and `lock_timeout` to 5 s, and resets both before the connection returns to the pool (a leaked 10 s `statement_timeout` on a reused connection would turn the safety net into a source of spurious failures). `lock_timeout` is the other half: a verification query has no business queueing behind DDL. An import's swap takes `AccessExclusiveLock` on the partition, and a check that waits for it both stalls itself and — holding `AccessShareLock` on what it has already read — can stall the swap. **Measured:** a test declaring `timeout: 10` against `SELECT pg_sleep(300)` fails at 10.002 s with SQLSTATE 57014 instead of running to completion. **It caught a real mis-declaration.** `conf/pl`'s buildings R120 — `sum(ST_NumGeometries(geom))` over ~9.5M multipolygons — has been reported as an intermittent 10 s timeout in #101, #107 and #108. It is not intermittent and it is not the machine: it takes **19.3 s**, and only looked flaky because the client-side cancel raced it. Moved to the 100 rung, where its sibling urząd check already sits. Full sweep under `--acceptance all`: is 42/42, se 55/55, pl 52/52, fi 57/57, gr 60/60, world 34/34. *Not claimed:* that this rescues a query whose client was SIGKILLed. On this box a killed importer's backend went away within seconds even with `client_connection_check_interval` at its 0 default — not isolated, and not a property to lean on. The point is that the bound no longer depends on finding out. ## 2. `importer validation-query <config> <file.sql>` The motivating incident: a verification query written during an investigation ran for **9h40m**, held `AccessShareLock` on a partition an import then needed to swap, and blocked that import for **7h36m**. The import looked slow; it was queued. Ad-hoc verification had no bound because a single-shot query has no business being committed to a config file just to inherit one. Same `acquireBounded` connection, so the same `statement_timeout` and `lock_timeout`; `--timeout` is on the same ladder (1, 10, 100, 1000) and defaults to 1000. Deliberately not a general SQL runner: - `BEGIN READ ONLY` — a file that tries to write is refused by the server (SQLSTATE 25006), structural rather than a promise - a file argument, not an inline string — complex SQL through shell quoting is how queries get mangled, and the file survives the run - the elapsed time is printed on purpose: a query worth keeping should **graduate** into the owning layer's or contract's `acceptance_tests:` at the rung its measured runtime earns. `validation-query` is the scratchpad, the config is the durable record — the relationship the deep tier already has with the fast tier. Verified live: a normal query returns rows + runtime; `pg_sleep(300)` at `--timeout 10` fails at 10.001 s; `CREATE TABLE` is refused and the table does not exist afterwards; `--timeout 25` is rejected as off-ladder. Offline CLI tests cover argument/ladder validation and the help path. `docs/importer-cli.md` documents both halves.

art-bot added 2 commits

2026-08-21 18:32:38 +00:00

importer: enforce every acceptance test's timeout server-side, not just in the client 334ffa63d4

The declared `timeout:` was a context deadline and nothing else. pgx cancels a
query when the context expires, but a cancel is a message the CLIENT sends, so the
bound was only ever as good as the client's liveness and its error handling —
which is not the guarantee the mechanism was built to give. runAcceptanceTests now
takes its connection through acquireBounded, which sets statement_timeout from the
test's own `timeout:` and lock_timeout to 5 s, and resets both before the
connection goes back to the pool (a leaked 10 s statement_timeout on a reused
connection would turn this into a source of spurious failures).

lock_timeout is the other half. A verification query has no business queueing
behind DDL: an import's swap takes AccessExclusiveLock on the partition, and a
check that waits for it both stalls itself and — holding AccessShareLock on what
it has already read — can stall the swap. Failing fast says more than a run that
appears to hang.

Measured: a test declaring `timeout: 10` against `SELECT pg_sleep(300)` fails at
10.002 s with SQLSTATE 57014 instead of running to completion.

**This caught a real mis-declaration.** conf/pl's buildings R120 —
`sum(ST_NumGeometries(geom))` over ~9.5M multipolygons, a full scan calling into
GEOS per row — has been reported as an intermittent 10 s timeout in #101, #107 and
#108. It is not intermittent and it is not the machine: it takes 19.3 s, and it
only looked flaky because the client-side cancel raced it. Moved to the 100 rung,
where its sibling urząd check already sits, with the measurement recorded next to
it. That was the one surprise across the whole sweep, which is the outcome the
caveat predicted: is 42/42, se 55/55, pl 52/52, fi 57/57, gr 60/60, world 34/34,
all under -acceptance all.

Not claimed: that this rescues a query whose client was SIGKILLed. On this box a
killed importer's backend went away within seconds even with
client_connection_check_interval at its 0 default, so something noticed the dead
socket — not isolated, and not a property to lean on. The point is that the bound
no longer depends on finding out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A2QYwuohkM9vWJs6uzsK58

importer: add validation-query, the ad-hoc half of the acceptance bound f7492dae0e

Verification queries written while investigating something are the ones that get
run by hand, through psql or whatever is nearest, with no bound on how long they
may take. That is how this database ended up with a query nobody was watching:
one written during an investigation ran for 9h40m, held AccessShareLock on a
partition an import then needed to swap, and blocked that import for 7h36m. The
import looked slow; it was queued. The acceptance mechanism exists precisely so
verification has an upper limit, but a single-shot query has no business being
committed to a config file just to inherit one.

  importer validation-query <config> <file.sql> [--timeout 1|10|100|1000]

The config supplies the destination connection and nothing else — no fetch, no
import, no validation beyond loading it. The query runs on a connection from
acquireBounded, the same helper the acceptance runner now uses, so it gets the
same statement_timeout and lock_timeout; --timeout is on the same ladder and
defaults to 1000.

Deliberately not a general SQL runner. The transaction is BEGIN READ ONLY, so a
file that tries to write is refused by the server (SQLSTATE 25006) rather than
caught by review — structural, not a promise. A file argument rather than an
inline string because complex SQL through shell quoting is how queries get
mangled, and because the file is an artifact that survives the run.

It prints the elapsed time on purpose. A query worth keeping should graduate into
the owning layer's or contract's acceptance_tests: at the rung its measured
runtime earns — the same relationship the deep tier already has with the fast
tier. validation-query is the scratchpad; the config is the durable record.

Verified against the live database: a normal query returns its rows and its
runtime; pg_sleep(300) at --timeout 10 fails at 10.001 s with SQLSTATE 57014;
CREATE TABLE is refused with SQLSTATE 25006 and the table does not exist
afterwards; --timeout 25 is rejected as off-ladder. Offline CLI tests cover the
argument and ladder validation and the help path, in the existing table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A2QYwuohkM9vWJs6uzsK58