Repository navigation
Move DB migrations off the request/connect path into a one-shot deploy-time runner #31
Description
Activity
Reviewed commit:
be11bdcb5. Note:origin/mainmoved to3e9660d06during the review; re-verify against currentmainbefore changing code, since a finding may have been fixed or moved.The 2026-07-28 full-repo review re-derived this and confirmed it is still present on
be11bdcb5. Adding two mechanisms this issue does not currently name, both of which widen the blast radius beyond "migrations compete with request latency".Where
internal/server/app.go:577-597(ensureDBholdsapp.dbMufor its full duration, taken at:583)internal/server/app.go:621(runMigrationsBounded)internal/server/app.go:199(runMigrationsBoundedWithbuilds its owncontext.WithTimeout(context.Background(), timeout))internal/server/app.go:140(migrationsTimeout, default 120s)- Callers:
internal/server/http.go:162,internal/server/http.go:228,internal/server/lambda.go:27
What the review adds
1.
ensureDBholdsdbMufor the entire migration, so migrations do not merely compete with request latency, they serialise every other request behind them. This issue describes the per-instance race and the fail-open behaviour, but not the mutex. Every concurrent request on that instance, including/api/scheduled/*, blocks ondbMufor up to the fullmigrationsTimeout.2. The migration runner deliberately ignores the caller's deadline.
runMigrationsBoundedWithbuilds a freshcontext.Background()bounded bymigrationsTimeout(default 120s,app.go:140), so the caller's 30s HTTP deadline has no effect on the DDL. The caller therefore cannot abandon it; it can only time out itself while the migration continues.Failure scenario
A deploy adds an index build that takes 90s.
The first request after the cold start enters
ensureDB, takesdbMu, and blocks inrunMigrationsBounded. Every concurrent request, API traffic and/api/scheduled/*(all of which callensureDBfirst), blocks ondbMu, hits its own 30s ceiling, and returns 503Service temporarily unavailable.Two consequences beyond the latency:
- On Lambda, the invocation is killed at the function timeout while the migration goroutine is still mid-DDL in a container that is then frozen. That is precisely the state that leaves
schema_migrations.dirty = true, the terminal failure mode the 120s default was raised to avoid. So the band-aid can itself produce the outcome it was meant to prevent, because the two timeouts (function and migration) are set independently and the longer one wins the DDL. - The scheduled tick that fires during this window returns 503 and is simply lost. There is no retry on the
/api/scheduled/*path, so a 90s migration window silently drops whatever collection or analytics tick landed in it, with no record that it was skipped.
Fix direction
Consistent with the one-shot deploy-time runner already proposed here. If lazy migration must stay as a transitional safety net, one cheap intermediate improvement: do the connect under
dbMuand the migration outside it, so a long DDL degrades throughput rather than blocking every request behind a mutex, and reportpending/failedthrough/healthas it already does.
Problem
Database migrations currently run lazily on DB connect, inside the
per-instance
ensureDBpath (internal/server/app.go), bounded by atimeout (
CUDLY_MIGRATION_TIMEOUT, default now 120s). This couples schemaevolution to request/cold-start traffic and has several structural failure
modes:
independently tries to run migrations. Concurrent cold starts race on the
same
schema_migrationsrow; a migration interrupted mid-run (Lambdatimeout, ENI drop) leaves
dirty = true, which is terminal — everylater boot then fail-opens and serves 500s on any query needing the
unapplied columns.
migration failure (so liveness probes pass), a broken migration is invisible
to the deploy pipeline and to the
AWS/LambdaErrorsmetric. The badbuild ships and the failure only surfaces as user-facing 500s.
buys headroom but the migration still competes with request latency budgets
and Lambda's hard 300s ceiling. A genuinely long DDL on a large table is
still at risk.
This was the root cause of the recent prod outage (migrations 070-073 never
applied;
purchase_delay_hours/revocation_window_closes_at/executed_by_user_idmissing). PR LeanerCloud/cloud-commitments-cli#1124 hardened the in-place path(opt-in dirty auto-heal, higher timeout, a CloudWatch failure alarm) but did
not change where/when migrations run.
Proposed structural fix
Move migrations off the request/connect path into a one-shot, deploy-time
runner:
Lambda/Job invoked by the deploy pipeline) runs
RunMigrationsexactly onceper deploy, before the new app version starts taking traffic.
for a large index build without competing with request budgets or the
serving Lambda's 300s ceiling.
the rollout instead of fail-opening into a half-broken serving fleet.
migrations at all (
DB_AUTO_MIGRATE=falseon the workload), so concurrentcold starts can't race or dirty the row.
Scope / out of scope
flipping the workload to
DB_AUTO_MIGRATE=false, and keeping the/healthmigrations check + the fix(migrations): opt-in dirty auto-heal, raise timeout default, failure alarm cloud-commitments-cli#1124 auto-heal/alarm as a safety net forthe transition.
Related
specs/migration-resilience.md("Not covered here" section already flagsthis as the longer-term fix)