Skip to main content

D106 — Postgres safety valves, a pre-flight guard + kill switch, and dev-stack speed levers for the parallel suite (RESOLVED)

Architecture decision record. Status, thematic clusters, and how to record a new ADR: the decision log index. The mechanics this ADR describes live in App\Providers\ParallelTestingServiceProvider, the Makefile, docker-compose.yml, and docker/Dockerfile.dev; the operational view is on Testing & the gates and Local development.

Context​

A 20-hour investigation into make test-parallel runs that wedge indefinitely (never completing, never failing — just stuck, with no Postgres deadlock ever logged) found three independent, stacked problems:

  1. A cross-connection self-deadlock invisible to Postgres's own deadlock detector. Each worker's two connections (pgsql, pgsql_logs) are two separate sessions onto the SAME token database. Some code path leaves the pgsql_logs session idle-in-transaction; that session holds a lock the between-test TRUNCATE ... CASCADE (issued on the pgsql session) needs, so pgsql blocks. This is a real deadlock in effect — one worker permanently wedged — but Postgres's cycle-detection algorithm never sees it, because the two sessions never form a cycle IT can observe (the idle-in-transaction session isn't itself waiting on anything Postgres tracks as a lock request). Nothing times out; the run just never finishes.
  2. Interrupted non-TTY runs leave zombie pest processes. Killing a make test-parallel invocation that has no controlling terminal — the agent/CI case; an interactive terminal run already gets a TTY via the Makefile's existing TTY_FLAG and Ctrl-C propagates into the container through it — does not tear down the paratest master + worker processes inside the app container. docker compose exec simply detaches; the exec'd process keeps running, orphaned, holding database connections and CPU that the next run then contends with.
  3. The suite is DB-bound. Each worker's per-test reset is a ~69-table TRUNCATE ... CASCADE inside one transaction, plus whichever tests call migrate:fresh; N workers doing this concurrently means N concurrent large transactions racing the same physical disk for WAL fsyncs, on a dev/CI stack where durability past the run buys nothing (state is rebuilt from migrations every time regardless).

Decision​

Four independent, composable fixes — none depends on the others, each is individually revertible.

1. Postgres safety valves (ParallelTestingServiceProvider)​

ALTER DATABASE "<db>" SET idle_in_transaction_session_timeout = '120s' and ALTER DATABASE "<db>" SET lock_timeout = '120s', applied:

  • To every token database, in the setUpProcess hook, right after it is created (or confirmed to already exist from a prior run) — idempotent ALTER DATABASE … SET, safe to re-issue. This part was never racy: each worker's token database is distinct, so no two processes ever ALTER the same one.
  • To the base stables_testing database — setUpProcess only fires under --parallel (the framework guards every ParallelTesting hook in whenRunningInParallel()), so serial make test needs a separate application, and where each fires is deliberately split to avoid a cross-process race (caught by QA on the first real full-suite run, not by review — see below):
    • boot() applies it, but ONLY when ParallelTesting::token() is falsy — true for a genuine serial run (no parallel machinery at all) and, harmlessly, for the paratest master process (it carries no token of its own either, but is a single process running sequentially before any worker starts, so it cannot race itself). Every WORKER process carries its own truthy token, so this branch is never reached there.
    • setUpProcess applies it too, guarded by the same static once-applied flag, because that hook runs sequentially in the single parent process, once per token, before any worker starts — the same reasoning the pre-existing ensureRlsRoleExists() call already relied on.
    • The bug this replaced: the first version of this code applied the base-database valves unconditionally in boot(), guarded only by the static flag. That flag is a private static property — process-local memory, not shared state — and each of the N parallel workers is a SEPARATE PHP process with its own copy, defaulting to false. Every worker's boot() therefore independently decided "not yet applied" and issued ALTER DATABASE "stables_testing" SET … concurrently against the one shared base database, which Postgres serializes at the catalog level — losing races intermittently threw SQLSTATE[XX000] tuple concurrently updated, failing whichever test happened to be booting in the losing worker (reproduced 2/2 full-suite runs; a different test failed each time — CustodyCommandsRequireProviderTest in one run, ExampleTest in the other — because WHICH worker loses the race, and what it happens to be doing at that moment, is non-deterministic). The fix moves the only-ever-racy path (an unconditional per-worker boot() call) onto the two paths that are either genuinely single-process (serial) or genuinely sequential-within-one-process (the parent's setUpProcess loop) — never onto a path any worker actually takes.

Both settings are per-database defaults applied to every future session against that database — not the issuing connection's own session — so they take effect for exactly the sessions that matter (the worker's pgsql/pgsql_logs pair) without needing every call site to set them itself. A stuck idle-in-transaction session now aborts after 2 minutes instead of wedging the run indefinitely; a TRUNCATE that can't acquire its lock within 2 minutes fails loudly instead of hanging silently. 2 minutes was chosen as comfortably longer than any single test or between-test reset should ever take, so it can't misfire on a merely-slow-but-healthy run.

2. Pre-flight guard + make test-kill​

make test and make test-parallel both depend on a new test-guard prerequisite target that checks the app container for an already-running suite and aborts with a pointer to make test-kill rather than starting a second run on top of a wedged one (compounding the DB contention that motivated fix #3 below). make test-kill finds and kills the pest master + worker processes inside the app container; idempotent (exits 0 whether or not anything was running).

Both use pgrep -f '[p]est/bin' — verified against this app's real process list, not assumed: php artisan test --parallel spawns a master at vendor/pestphp/pest/bin/pest and N workers at vendor/pestphp/pest/bin/worker.php (Pest's own parallel runner; this app never shells out to a literal vendor/bin/paratest binary), so a pattern of bin/pest alone — the first thing tried — matches the master but misses every worker (bin/worker.php, not bin/pest). pest/bin is the substring both processes' command lines actually share. The bracketed first character ([p]est/bin) is the classic ps | grep '[p]attern' self-exclusion idiom: pgrep -f "pest/bin" matches its OWN argv, because the string pest/bin appears verbatim in the very command line pgrep is scanning; splitting the first character into a single-character regex class keeps the match semantically identical while making the invoking command's own text ([p]est/bin, brackets included) fail to contain the literal substring the pattern requires.

Both test and test-parallel also trap INT/TERM and run the same in-container kill on interrupt, so a killed non-TTY run (fix #2 in Context) cannot leave zombies. Two things were needed to make the trap actually fire, not just look like it does — both verified empirically against dash (this Makefile's /bin/sh):

  • The long-running command runs backgrounded and waited (cmd & pid=$!; wait "$pid"), not plain foreground. POSIX shells defer a trapped signal's action until the current FOREGROUND command exits — so a trap wrapping a genuinely wedged docker compose exec would install correctly, receive the signal correctly, and still never run, because the deferred action waits for a child that (by definition, for the run this exists to fix) may never exit on its own. wait is the documented POSIX exception: a trapped signal interrupts it immediately. Confirmed by timed reproduction: a plain-foreground trap took 0 visible effect within 15s of SIGTERM against a 30-second sleep; the backgrounded-wait form fired within 0.5s of the same signal.
  • The cleanup itself is a second, short-lived docker compose exec, because the pest processes live inside the container — killing the client-side docker process alone leaves them running, orphaned, which is exactly the zombie state this exists to prevent.
  • Not $(MAKE) test-kill. GNU Make always executes any recipe line whose text actually EXPANDS the $(MAKE) variable, even under -n (so a recursive make invocation still sees the dry-run flag) — confirmed by direct reproduction: a trap calling back into $(MAKE) test-kill made make -n test-parallel actually run the full suite. The kill logic is duplicated inline instead (mentioning make test-kill in a comment, with no actual variable expansion, does not trip this — also confirmed by direct reproduction).

3. Postgres non-durability flags (docker-compose.yml)​

The postgres service's command: adds -c fsync=off -c synchronous_commit=off -c full_page_writes=off, alongside the existing max_connections=200 -c max_locks_per_transaction=256 tuning. A deliberate dev/CI-only durability tradeoff: a crash means re-seed, which is an acceptable cost on a stack that rebuilds its schema from migrations every run and never persists anything that matters past a down -v. Verified before making this change that nothing production-facing consumes this file: production and preview both build from docker/production/Dockerfile via .github/workflows/build-image.yml; docker-compose.yml is dev-only, with CI layering docker-compose.ci.yml on top of the identical base (D99) — there is no path from this file to a deployed environment.

4. OPcache for the CLI SAPI (docker/Dockerfile.dev)​

docker/opcache-cli.ini sets opcache.enable_cli=1 plus a file cache (opcache.file_cache=/tmp/opcache, directory created in the image), copied into /usr/local/etc/php/conf.d/. Investigated rather than assumed: php:8.5-cli-alpine compiles OPcache directly into the PHP binary — php -m shows Zend OPcache loaded with no matching file under the extension directory, and docker-php-ext-enable opcache itself refuses ("already compiled into PHP") — so neither docker-php-ext-enable nor -install applies; only opcache.enable_cli (off by default; opcache.enable is already on, but that setting governs the web/FPM SAPIs, not CLI) needed flipping. The file cache exists because long-lived paratest workers are not this app's only CLI-heavy pattern: every freshly spawned artisan/pest invocation — a new process each time — gets no benefit from an in-memory-only opcode cache scoped to one process's lifetime, but does benefit from a cache that persists compiled opcodes on disk across process boundaries. Deliberately not touching Xdebug: XDEBUG_MODE=off already keeps it cheap at runtime, and enabling it was out of scope for this change.

Consequences​

  • A wedged parallel run under the traced failure mode now surfaces as a loud, bounded failure (a session/lock timeout inside 2 minutes) instead of an indefinite hang with no diagnostic signal.
  • make test-kill and the pre-flight guard turn "is something already running" from a manual docker exec ... ps aux investigation into one command with a clear message.
  • An agent/CI run that gets killed non-interactively no longer leaves zombie pest workers holding database connections for the next run to contend with.
  • The dev/CI Postgres and PHP CLI runtime both trade a little safety/durability that this stack never needed for real speed on the exact workload (TRUNCATE/migrate-heavy, process-spawn-heavy) that dominates suite wall-clock time.
  • Speed was investigated but not benchmarked as part of this change. The rationale for each lever (WAL fsyncs on a TRUNCATE-heavy workload; re-parsing the framework on every process spawn) is sound on its face, but no before/after suite-timing numbers were captured here — the director schedules the actual make test-parallel timing run separately (after the image rebuild this change requires), one suite run at a time, per this repo's shared-stables_testing-DB constraint. If a re-benchmark ever shows one of these levers is not paying for itself, it can be reverted independently — none of the four fixes depends on any other.

Alternatives rejected​

  • A SELECT pg_terminate_backend(...) sweep instead of idle_in_transaction_session_timeout. Would require something to run the sweep (a cron, a pre-test hook) and still races the exact window where the deadlock forms; a per-database session default requires nothing to remember to run it and applies to every future connection unconditionally.
  • pkill -f pest without the bracket trick. Matches its own invocation's argv (the literal string appears in the very command line being scanned), so test-guard would report a phantom "already running" suite on every invocation, and test-kill would never report "nothing to kill" even when nothing was running. Rejected outright once the self-match was reproduced.
  • pgrep -f 'bin/pest' alone (no worker coverage). The first pattern tried; verified against the real process list to miss every worker process (bin/worker.php), which would silently defeat both test-guard (never detects a running suite because only the short-lived master transiently matches) and test-kill (leaves all N workers running after "killing" only the master).
  • $(MAKE) test-kill from the interrupt trap, for the obvious DRY win over duplicating the kill logic inline. Rejected once reproduced that it breaks make -n for both test and test-parallel (GNU Make's documented always-execute-under--n special case for any recipe line that expands $(MAKE)), which the verification step for this change explicitly checks.
  • A plain foreground trap 'cleanup' INT TERM; long_running_cmd (the textbook idiom, and the first thing tried here). Reproduced to not run its cleanup within 15 seconds of SIGTERM against a foreground child that ignores the signal itself — dash defers the trap until the foreground command exits, which is precisely the condition that doesn't hold for a wedged run. Rejected in favour of the backgrounded-wait form once the deferred-trap behavior was confirmed empirically.
  • Enabling Xdebug tuning as part of this change. XDEBUG_MODE=off already keeps it cheap; adding scope here would mix an unrelated lever into a change whose Context section is specifically about the traced wedge + the DB-bound workload. Left for its own change if ever warranted.
  • Tuning the default worker count. The investigation's report named this as a lever, and it is deliberately not touched here: the right count is a property of the machine running the suite (CPU cores, RAM, how much else is competing for them), not of the repo, so there is no single default that is right for everyone. It is already steerable per-run without any code change — make test-parallel ARGS="--processes=N" — and the Makefile's own comment on that target already documents the ≤16 ceiling (Valkey's logical DB count) and the "on a host with >16 cores" case. Left as a per-developer/CI-runner tuning knob, not a repo default to chase.
  • Quiescing the dev stack (docker compose stop vite/scheduler/worker) during a run. Also named in the investigation's report, and also deliberately left out: whether those services are worth stopping depends on what else is running on the SAME machine at the SAME time (a solo dev running nothing else loses little either way; a CI runner sharing the box with other jobs might gain more), which makes it a machine-local operational choice rather than something with one correct repo-wide default. It is also not free to make default: stopping vite mid-run would break anyone iterating on the frontend in a second terminal while the suite runs, and stopping scheduler/worker would silently change what a test that happens to dispatch a queued job actually observes. Left as a manual, situational step — docker compose stop vite worker scheduler needs no new Makefile target — not wired into test-parallel itself.
  • Raising the safety-valve timeouts well above 2 minutes "to be safe." A timeout long enough to never legitimately fire is a timeout that also takes that long to surface a genuine wedge — the entire point of a bounded failure over an indefinite hang. 2 minutes was chosen as comfortably longer than any healthy test or reset should take, not as a generous margin against uncertainty.

← Decision log index