fix(deploy): wait for teal-db instead of failing on a transient state - #64
Conversation
The deploy of ce6c7a0 (#61) aborted 19s in, after a good backup and a successful image pull, leaving production on a6c05be for two weeks. The migration it was guarding is purely additive — create table, add foreign key, add unique constraint — and the guard passes on demand today. The likely trigger: a cancelled CI run took its own backup at 08:33:46 and was killed mid-restart; the next run's migration guard ran 90s later while the stack was still recreating, could not reach the database, and correctly refused to go live. Two deploys colliding, reported as an unverifiable migration. Pre-flight now waits up to DB_WAIT_TIMEOUT (60s) for teal-db rather than checking once, and the check is repeated immediately before the migration scan — minutes pass pulling the image, and that guard's failure mode is to abort the deploy, so a transient connection problem must not masquerade as an unsafe migration. A healthy database still costs zero extra time.
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Health polling and migration readiness gate docker/deploy-prod.sh |
Adds DB_WAIT_TIMEOUT with a 60-second default, polls teal-db health until healthy or timed out, and repeats the wait before destructive-migration scanning. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Sequence Diagram(s)
sequenceDiagram
participant deploy-prod.sh
participant wait_for_db
participant Docker
participant teal-db
deploy-prod.sh->>wait_for_db: Wait for database readiness
wait_for_db->>Docker: Inspect teal-db health
Docker->>teal-db: Read health status
teal-db-->>Docker: Return health status
Docker-->>wait_for_db: Return health result
wait_for_db-->>deploy-prod.sh: Continue or abort on timeout
deploy-prod.sh->>wait_for_db: Re-check before migration scan
Poem
I’m a rabbit with a watch,
Waiting by the database hatch.
Poll, poll, healthy green—
Then migrations may be seen!
Timeout keeps the hop on track.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly describes the main change: waiting for teal-db during deploys instead of aborting on transient states. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/deploy-guard-db-wait
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docker/deploy-prod.sh (1)
65-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCapture the timeout status directly instead of discarding and re-querying.
Currently,
wait_for_dbechoes the failing status on timeout, but>/dev/nulldiscards it, forcing thefatalmessage to re-querydb_health. This could log an inaccurate status (e.g.,healthy) if the database recovers immediately after the timeout. Capturing the output avoids this race condition. Additionally, consider adding a brief log statement before these operations to indicate that the script is waiting.
docker/deploy-prod.sh#L65-L65: change tofinal_status=$(wait_for_db) || fatal "... (status: ${final_status}) ..."and optionally add alogstatement beforehand.docker/deploy-prod.sh#L108-L108: change tofinal_status=$(wait_for_db) || fatal "... (status: ${final_status}) ..."and optionally add alogstatement beforehand.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/deploy-prod.sh` at line 65, Capture the output of wait_for_db in final_status instead of discarding it and re-querying db_health, then use that captured status in the fatal message at docker/deploy-prod.sh lines 65-65 and 108-108. Optionally add a brief log before each wait indicating that the script is waiting for the database.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docker/deploy-prod.sh`:
- Line 65: Capture the output of wait_for_db in final_status instead of
discarding it and re-querying db_health, then use that captured status in the
fatal message at docker/deploy-prod.sh lines 65-65 and 108-108. Optionally add a
brief log before each wait indicating that the script is waiting for the
database.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3ca995d-0a94-45a3-95ce-1a619346abef
📒 Files selected for processing (1)
docker/deploy-prod.sh
1 participant
Why
Production has been running
a6c05be(Tailwind 4) since 2026-07-07, missing #57 (Vite 8) and #61 (per-user encrypted API keys). The deploy ofce6c7a0failed 19s in and nobody noticed —mainhas been red since.Diagnosis (from backup timestamps, host image list and container state on gerty):
quality,tests,rectorall passed; onlyDeploy to gertyfailed.teal-20260707-083549.sql.gz, valid).The migration it was guarding is purely additive:
Likely trigger: a cancelled CI run took its own backup at 08:33:46 and was killed mid-restart. The next run's guard ran ~90s later while the stack was still recreating, could not reach the database, and correctly refused to go live. Two deploys colliding, surfaced as an unverifiable migration.
What changed
concurrency: cancel-in-progress: falseprevents two deploys running at once, but nothing stops a cancelled deploy from leaving the box mid-apply and poisoning the next one.DB_WAIT_TIMEOUT(60s, overridable) forteal-dbinstead of checking once.A healthy database costs zero extra time.
Not changed
The destructive-migration regex, no-backup-no-deploy, and the rollback path all behaved correctly and are untouched. The guard halting on something it could not verify was right; it just had a bad reason to be unable to verify.
Verification
bash -nclean.Summary by CodeRabbit