ci: dump database container logs when the DB wait times out - #6217
ci: dump database container logs when the DB wait times out#6217prql-bot wants to merge 2 commits into
Conversation
prql-bot
left a comment
There was a problem hiding this comment.
The gating is right — outcome is skipped on the matrix legs that never start the DBs (only default,test-dbs-external,lsp on ubuntu-24.04 sets that feature), so the step is inert there rather than needing its own contains(inputs.features, …) guard. working-directory matches the Run docker compose step above, and nothing is torn down in between, so the containers are still around to dump.
One gap in the diagnostic itself, and it's the hypothesis most likely to explain the failure that motivated this PR: disk exhaustion doesn't show up in compose ps/logs in a way you can read confidently, and the run captured no df -h either. On attempt 1 of run 32428511588, Free up disk space was skipped at 23:26:45 — it's gated on steps.cache.outputs.cache-hit == 'false', so on the cache-hit path (the common one on main) nothing is freed and its df -h never prints. That's the same path the failure took. Meanwhile the 💰 Cache step's own comment in this file says the job already runs at the edge of what GHA can hold ("our current cache size at ~1.3GB is about as much as GHA can handle"), and mssql needs several GB to initialise — it's the one service of the four that would die first under disk pressure, which matches "only mssql was missing".
So the next occurrence would still leave "was it disk?" unanswered. One line closes it, at no cost on the success path since the step only runs on failure.
Since this is a bot PR with no separate author to apply the suggestion, I'll push it to the branch after posting.
Problem
testsfailed on main in run 32428511588: thetest-rust (x86_64-unknown-linux-gnu, ubuntu-24.04, default,test-dbs-external,lsp)job errored withError: Timed out waiting for: tcp:1433— the mssql container never started listening.check-ok-to-mergethen failed as a consequence.The failure was transient, and re-running the failed jobs on that same commit has since passed, so main is green again. The evidence pointed at infrastructure rather than the commit: the triggering commit was a revert of a tend-workflow-timeout change that touches nothing in the test path, the run an hour earlier (32423339797) passed the same job on the same code, and both runs pulled identical mssql image layers (
901cdc4e17f1,dea7da2748b4,bd746ec3ac20), so it wasn't an upstream image change. The other three ports (3306, 5432, 9004) came up fine, so docker and the compose network were healthy — only mssql was missing.What can't be established from that run is why. The containers started at 23:25:29 and mssql still wasn't listening when the wait gave up at 23:28:38, over three minutes later — but the workflow captures nothing from the containers themselves, so a container that crashed on startup and one that was merely slow look identical in the log. That gap is the durable problem: the failure is rare (this
timeout: 60000has been in place since #2766 in 2023), so each occurrence is the only chance to diagnose it, and this one was spent blind.Solution
Add a step after the wait, gated on
steps.wait-for-db.outcome == 'failure', that dumpsdf -h,docker compose ps --allanddocker compose logs. Next time the wait times out, the failing run itself carries the containers' exit codes, their startup output, and the disk state, instead of the diagnosis needing a re-run that — being a flake — probably passes and destroys the evidence.df -his there because disk exhaustion is the one hypothesis the container logs alone don't settle: theFree up disk spacestep earlier in the job is skipped on a cache hit, which is exactly the path this run took, so nothing on that path reports free space — and mssql, the largest of the four images, is the container that would fail to initialise first.The step keys off the wait step's own
outcomerather than a barefailure(), so it doesn't fire on an unrelated earlier failure, and it's inert on the matrix legs that skip the DBs (outcomeisskippedthere).I deliberately did not bump
timeout: 60000or add compose healthchecks. Both are plausible remedies, but which one is right depends on whether mssql crashed or was starved or was slow — exactly the question this run couldn't answer. Better to make the next occurrence readable than to guess now.Testing
prettier --checkpasses on the workflow, and it parses underyaml.safe_loadwith both steps resolving as intended:Parsed steps
The new step only runs on the failure path, so a green CI run on this PR exercises the change's inertness rather than its output; its behaviour on failure is verified by inspection.
Automated fix for failed run