diff --git a/packages/super-sync-server/env.example b/packages/super-sync-server/env.example index 91569b1704..6df3364bc3 100644 --- a/packages/super-sync-server/env.example +++ b/packages/super-sync-server/env.example @@ -118,6 +118,13 @@ SMTP_FROM="Super Productivity Sync" # transactions; raise this for very large tables. Exit code 124 = timed out. # MIGRATION_TIMEOUT=900 +# Per-migration-step timeout (seconds) enforced inside the image. deploy.sh +# derives it from MIGRATION_TIMEOUT (minus a small margin) and forwards it, so +# raising MIGRATION_TIMEOUT is enough for the deploy.sh path. Set this directly +# only for the image CMD / helm initContainer paths, which have no outer host +# timeout. Defaults to 1800 inside the image when unset. +# MIGRATE_STEP_TIMEOUT= + # Safety valve for custom/private images without an # org.opencontainers.image.revision label. Leave this unset in production; the # deploy script uses the label to prevent mixing fresh SuperSync deploy inputs diff --git a/packages/super-sync-server/prisma/migrations/README.md b/packages/super-sync-server/prisma/migrations/README.md index f867621f5f..15664df58a 100644 --- a/packages/super-sync-server/prisma/migrations/README.md +++ b/packages/super-sync-server/prisma/migrations/README.md @@ -72,3 +72,14 @@ auto-recovered — it fails loudly by gate, deterministically, which is the intended behavior. This is enforced by `tests/migration-sql.spec.ts` (the migration has no `DROP`) and `tests/migrate-deploy-script.spec.ts` (a bare CREATE is refused, never marked applied). + +**Bare vs drop-then-create for a _new_ index.** Reserve the bare fail-loud +shape for a _correctness-critical_ index, where an interrupted build should +halt the deploy for a human rather than silently retry. For a +_performance-only_ index that has a correct fallback path — e.g. +`operations_entity_ids_gin`, whose conflict lookups fall back to the scalar +`entity_id` — prefer the auto-recoverable drop-then-create shape so an +interrupted build self-heals on the next deploy instead of wedging it and +requiring manual recovery. Do **not** retro-convert an already-applied bare +CREATE: that changes its checksum and breaks `migrate deploy` on every DB that +has it. diff --git a/packages/super-sync-server/scripts/deploy.sh b/packages/super-sync-server/scripts/deploy.sh index a97320e64e..0cbded4742 100755 --- a/packages/super-sync-server/scripts/deploy.sh +++ b/packages/super-sync-server/scripts/deploy.sh @@ -95,7 +95,7 @@ load_env_value() { export "$key=$value" } -for env_key in GHCR_USER GHCR_TOKEN DATABASE_URL POSTGRES_SERVICE POSTGRES_WAIT_TIMEOUT MIGRATION_TIMEOUT DEPLOY_WAIT_TIMEOUT SUPERSYNC_SKIP_IMAGE_REVISION_CHECK; do +for env_key in GHCR_USER GHCR_TOKEN DATABASE_URL POSTGRES_SERVICE POSTGRES_WAIT_TIMEOUT MIGRATION_TIMEOUT MIGRATE_STEP_TIMEOUT DEPLOY_WAIT_TIMEOUT SUPERSYNC_SKIP_IMAGE_REVISION_CHECK; do load_env_value "$env_key" done @@ -286,7 +286,18 @@ echo "" # for arbitrarily long. Wrap the migrator with a timeout so a stuck deploy fails # loudly instead of hanging this script forever. Exit code 124 = timed out. MIGRATION_TIMEOUT="${MIGRATION_TIMEOUT:-900}" -MIGRATOR_RUN="docker compose $COMPOSE_FILES run --rm --no-deps --interactive=false -T supersync" +# The migrator image caps each migration step at MIGRATE_STEP_TIMEOUT (1800s by +# default inside the image). Left unset it silently overrides a larger +# MIGRATION_TIMEOUT and kills a slow CREATE INDEX CONCURRENTLY early, so forward +# the operator's budget into the container. Keep it just under the host timeout +# so the in-image guard — which prints the precise per-step message — fires +# first, with the host timeout as a hard backstop. +if [ "$MIGRATION_TIMEOUT" -gt 90 ]; then + MIGRATE_STEP_TIMEOUT="${MIGRATE_STEP_TIMEOUT:-$((MIGRATION_TIMEOUT - 30))}" +else + MIGRATE_STEP_TIMEOUT="${MIGRATE_STEP_TIMEOUT:-$MIGRATION_TIMEOUT}" +fi +MIGRATOR_RUN="docker compose $COMPOSE_FILES run --rm --no-deps --interactive=false -T -e MIGRATE_STEP_TIMEOUT=$MIGRATE_STEP_TIMEOUT supersync" echo "==> Verifying database connectivity from the supersync image..." set +e timeout "$POSTGRES_WAIT_TIMEOUT" \ diff --git a/packages/super-sync-server/scripts/migrate-deploy.sh b/packages/super-sync-server/scripts/migrate-deploy.sh index ae207ccf54..3b5c20eab2 100644 --- a/packages/super-sync-server/scripts/migrate-deploy.sh +++ b/packages/super-sync-server/scripts/migrate-deploy.sh @@ -48,7 +48,19 @@ MAX_ATTEMPTS="${MIGRATE_MAX_ATTEMPTS:-6}" STEP_TIMEOUT="${MIGRATE_STEP_TIMEOUT:-1800}" if command -v timeout >/dev/null 2>&1; then - with_timeout() { timeout "$STEP_TIMEOUT" "$@"; } + with_timeout() { + wt_rc=0 + timeout "$STEP_TIMEOUT" "$@" || wt_rc=$? + # GNU coreutils `timeout` exits 124 on expiry; BusyBox `timeout` (shipped by + # the node:*-alpine runtime image) instead lets the child die from the + # default SIGTERM and returns 128+15=143. Normalize so the single 124 + # timeout branch is reached on both. Under this wrapper a 143 is timeout's + # own SIGTERM, not an unrelated external kill. + if [ "$wt_rc" -eq 143 ]; then + wt_rc=124 + fi + return "$wt_rc" + } else with_timeout() { "$@"; } fi @@ -129,6 +141,17 @@ is_recoverable_concurrently_migration() { grep -Eqi 'CREATE[[:space:]]+INDEX[[:space:]]+CONCURRENTLY' "$sql" } +# The intentionally-fail-loud shape: a bare CREATE INDEX CONCURRENTLY with no +# DROP. Not auto-recovered (an interrupted build leaves an INVALID index that +# must be handled deliberately), but distinguished from a plain non-index +# migration so the loud failure can print the correct manual steps. +is_bare_create_concurrently() { + sql="$1" + [ -f "$sql" ] && + grep -Eqi 'CREATE[[:space:]]+(UNIQUE[[:space:]]+)?INDEX[[:space:]]+CONCURRENTLY' "$sql" && + ! grep -Eqi 'DROP[[:space:]]+INDEX[[:space:]]+CONCURRENTLY' "$sql" +} + # One statement per line; multi-line statements collapsed to a single line # (index DDL is whitespace-insensitive and has no line-spanning literals). split_statements() { @@ -163,6 +186,46 @@ print_manual_recovery() { echo " npx prisma migrate resolve --applied $(shell_quote "$name") # only after every statement above succeeds" } +# Copy-paste recovery for an interrupted bare CREATE INDEX CONCURRENTLY. An +# aborted concurrent build leaves an INVALID index of the same name, so a plain +# re-run of the migration fails with "already exists"; the INVALID index must be +# dropped first. Then clear the failed record so the next deploy re-applies the +# migration natively (single-statement CONCURRENTLY needs no out-of-band run). +print_bare_create_recovery() { + name="$1" + sql="$2" + idx="$(grep -Ei 'CREATE[[:space:]]+(UNIQUE[[:space:]]+)?INDEX[[:space:]]+CONCURRENTLY' "$sql" | + grep -oE '"[^"]+"' | head -n1 | tr -d '"')" + echo "" + echo "Manual recovery for $name (interrupted bare CREATE INDEX CONCURRENTLY, copy-paste):" + if [ -n "$idx" ]; then + echo " printf '%s\\n' $(shell_quote "DROP INDEX CONCURRENTLY IF EXISTS \"$idx\";") | npx prisma db execute --schema $SCHEMA --stdin" + else + echo " # Drop any INVALID index left by the interrupted build (see $sql), e.g.:" + echo " # DROP INDEX CONCURRENTLY IF EXISTS \"\";" + fi + echo " npx prisma migrate resolve --rolled-back $(shell_quote "$name")" + echo " # Then re-run the deploy; $name re-applies natively." +} + +# Print copy-paste recovery for an INTERRUPTED CONCURRENTLY index build (a +# migrate step aborted by a timeout SIGTERM, OOM, or external stop), if — and +# only if — the failing migration is one. An aborted CONCURRENTLY build leaves +# an INVALID index of the target name, so a plain re-run cannot rebuild it. This +# only ever prints guidance; it never resolves a migration, and a non-index or +# unidentifiable failure prints nothing. Safe to call from any failure branch. +emit_interrupted_recovery_hint() { + hint_name="$(parse_failing_migration)" + [ -n "$hint_name" ] || return 0 + hint_sql="$(migration_sql_path "$hint_name")" + if is_bare_create_concurrently "$hint_sql"; then + print_bare_create_recovery "$hint_name" "$hint_sql" + elif is_recoverable_concurrently_migration "$hint_sql"; then + echo "" + echo "$hint_name is an auto-recoverable CONCURRENTLY migration; re-run the deploy to finish it (the re-run drops any INVALID index and rebuilds)." + fi +} + fail_loudly() { echo "" echo "ERROR: $1" @@ -215,7 +278,13 @@ while :; do exit 0 fi if [ "$MIGRATE_STATUS" -eq 124 ]; then - fail_loudly "prisma migrate deploy timed out after ${STEP_TIMEOUT}s (a long-running transaction may be blocking CREATE/DROP INDEX CONCURRENTLY)." 1 + # This branch also catches a normalized 143 (with_timeout maps BusyBox's + # SIGTERM exit to 124), i.e. the incident's own signal. A timed-out/aborted + # CONCURRENTLY build leaves an INVALID index + a failed record, so raising + # the timeout alone will not let a plain re-run rebuild it — surface the + # drop-index recovery here so the FIRST failure is actionable. + emit_interrupted_recovery_hint + fail_loudly "prisma migrate deploy timed out after ${STEP_TIMEOUT}s (a long-running transaction may be blocking CREATE/DROP INDEX CONCURRENTLY). Clear the blocker, then raise MIGRATION_TIMEOUT (it forwards to MIGRATE_STEP_TIMEOUT) and re-run." 1 fi attempt=$((attempt + 1)) @@ -224,6 +293,14 @@ while :; do fi if ! is_transaction_block_failure && ! is_stuck_failed_migration; then + # A non-P3018/P3009 exit is usually a genuine error (bad SQL, unreachable + # DB), but OOM (137) or another non-timeout kill can also abort an in-flight + # CONCURRENTLY build before Prisma records the failure. (A timeout SIGTERM is + # normalized to 124 above and handled there — it never reaches here.) Surface + # the drop-index recovery when the in-flight migration is a CONCURRENTLY + # build so the FIRST failure is actionable (deploy.sh promises "recovery + # steps above"); guidance only, never auto-resolves. + emit_interrupted_recovery_hint fail_loudly "prisma migrate deploy failed (exit $MIGRATE_STATUS)." fi @@ -234,6 +311,9 @@ while :; do sql="$(migration_sql_path "$name")" if ! is_recoverable_concurrently_migration "$sql"; then + if is_bare_create_concurrently "$sql"; then + print_bare_create_recovery "$name" "$sql" + fi fail_loudly "$name is not a recoverable drop-then-create CONCURRENTLY index migration (a bare CREATE is intentionally fail-loud); refusing to auto-resolve." fi diff --git a/packages/super-sync-server/tests/migrate-deploy-script.spec.ts b/packages/super-sync-server/tests/migrate-deploy-script.spec.ts index e4eb5489d1..898d877a46 100644 --- a/packages/super-sync-server/tests/migrate-deploy-script.spec.ts +++ b/packages/super-sync-server/tests/migrate-deploy-script.spec.ts @@ -75,6 +75,16 @@ case "$sub" in echo "The \\\`$m\\\` migration started at 2026-05-15 failed" [ -n "\${FAKE_DECOY:-}" ] && echo "Applying migration \\\`\$FAKE_DECOY\\\`" ;; + INTERRUPT) + # A CONCURRENTLY build killed mid-apply (external SIGTERM/OOM): + # the migration name is visible in the "Applying" line, but no + # P3018/P3009 gate marker is emitted and the process exits with + # a non-gate code (137 stands in for any external kill; a raw + # 143 would be normalized to the timeout branch instead). + echo "Applying migration \\\`$m\\\`" + echo "Terminated" + exit 137 + ;; *) echo "Error: P1001" echo "Can't reach database server" @@ -221,6 +231,99 @@ describe('migrate-deploy.sh generic CONCURRENTLY recovery', () => { expect(r.resolveApplied).toEqual([]); }); + it('prints copy-paste recovery for a stuck bare CREATE INDEX CONCURRENTLY', () => { + // An interrupted bare CREATE (e.g. a step timeout) leaves the migration + // failed (P3009) and an INVALID index of the same name; the loud failure + // must tell the operator to drop it and roll the record back — never + // auto-resolve it. + const bare = '20260701000000_add_bare_concurrent_index'; + const bareSql = `CREATE INDEX CONCURRENTLY "operations_bare_idx" + ON "operations"("user_id", "server_seq");`; + writeMigration(bare, bareSql); + const r = run({ FAKE_FAIL: bare, FAKE_CODE: 'P3009' }); + + expect(r.status).not.toBe(0); + expect(r.stdout).toContain('not a recoverable drop-then-create'); + expect(r.stdout).toContain( + 'DROP INDEX CONCURRENTLY IF EXISTS "operations_bare_idx";', + ); + expect(r.stdout).toContain(`migrate resolve --rolled-back '${bare}'`); + expect(r.resolveApplied).toEqual([]); + expect(r.resolveRolledBack).toEqual([]); + }); + + it('reports a BusyBox timeout (exit 143) as a timeout, not a generic failure', () => { + // node:*-alpine ships BusyBox `timeout`, which returns 128+SIGTERM=143 on + // expiry (GNU coreutils returns 124). A fake `timeout` on PATH reproduces + // that: run the wrapped command, then exit 143 as if the step was TERMed. + // migrate-deploy.sh must normalize this to its timeout branch. + const fakeTimeout = join(binDir, 'timeout'); + writeFileSync(fakeTimeout, '#!/bin/sh\nshift\n"$@"\nexit 143\n'); + chmodSync(fakeTimeout, 0o755); + + const r = run({ FAKE_FAIL: '', FAKE_CODE: 'P3018' }); + + expect(r.status).not.toBe(0); + expect(r.stdout).toContain('timed out after'); + expect(r.resolveApplied).toEqual([]); + }); + + it('prints bare-create recovery when a normalized-143 timeout aborts a bare CONCURRENTLY build (the incident)', () => { + // The reported incident: a bare CREATE INDEX CONCURRENTLY killed by the + // in-image step timeout (BusyBox SIGTERM -> 143 -> normalized to the 124 + // timeout branch). That branch must ALSO print the drop-INVALID-index + // recovery, because raising the timeout alone cannot rebuild an INVALID + // index left by the aborted build. + const fakeTimeout = join(binDir, 'timeout'); + writeFileSync(fakeTimeout, '#!/bin/sh\nshift\n"$@"\nexit 143\n'); + chmodSync(fakeTimeout, 0o755); + const bare = '20260701000000_add_bare_concurrent_index'; + const bareSql = `CREATE INDEX CONCURRENTLY "operations_bare_idx" + ON "operations"("user_id", "server_seq");`; + writeMigration(bare, bareSql); + const r = run({ FAKE_FAIL: bare, FAKE_CODE: 'INTERRUPT' }); + + expect(r.status).not.toBe(0); + expect(r.stdout).toContain('timed out after'); + expect(r.stdout).toContain( + 'DROP INDEX CONCURRENTLY IF EXISTS "operations_bare_idx";', + ); + expect(r.stdout).toContain(`migrate resolve --rolled-back '${bare}'`); + expect(r.resolveApplied).toEqual([]); + expect(r.resolveRolledBack).toEqual([]); + }); + + it('surfaces bare-create recovery when a CONCURRENTLY build is interrupted (non-gate exit)', () => { + // The user's incident: a bare CREATE INDEX CONCURRENTLY killed mid-build + // exits with a non-P3018/P3009 code before Prisma records the failure. The + // first failure must still print copy-paste recovery for the INVALID index + // (drop it, roll the record back), never a bare exit code. + const bare = '20260701000000_add_bare_concurrent_index'; + const bareSql = `CREATE INDEX CONCURRENTLY "operations_bare_idx" + ON "operations"("user_id", "server_seq");`; + writeMigration(bare, bareSql); + const r = run({ FAKE_FAIL: bare, FAKE_CODE: 'INTERRUPT' }); + + expect(r.status).not.toBe(0); + expect(r.stdout).toContain( + 'DROP INDEX CONCURRENTLY IF EXISTS "operations_bare_idx";', + ); + expect(r.stdout).toContain(`migrate resolve --rolled-back '${bare}'`); + // Guidance only — the interrupted migration must never be auto-resolved. + expect(r.resolveApplied).toEqual([]); + expect(r.resolveRolledBack).toEqual([]); + }); + + it('hints a re-run when an auto-recoverable CONCURRENTLY migration is interrupted', () => { + writeMigration(ENCRYPTED_OPS, ENCRYPTED_OPS_SQL); + const r = run({ FAKE_FAIL: ENCRYPTED_OPS, FAKE_CODE: 'INTERRUPT' }); + + expect(r.status).not.toBe(0); + expect(r.stdout).toContain('re-run the deploy to finish it'); + expect(r.resolveApplied).toEqual([]); + expect(r.resolveRolledBack).toEqual([]); + }); + it('does NOT mark applied when an out-of-band statement fails', () => { writeMigration(ENCRYPTED_OPS, ENCRYPTED_OPS_SQL); const r = run({ diff --git a/packages/super-sync-server/tests/migration-sql.spec.ts b/packages/super-sync-server/tests/migration-sql.spec.ts index 64e6645c17..27c31507ec 100644 --- a/packages/super-sync-server/tests/migration-sql.spec.ts +++ b/packages/super-sync-server/tests/migration-sql.spec.ts @@ -217,7 +217,15 @@ describe('performance migrations', () => { expect(deployScript).toContain('jq is required'); expect(deployScript).toContain('docker compose config --format json failed'); expect(deployScript).toContain('docker image inspect'); - expect(deployScript).toContain('run --rm --no-deps --interactive=false -T supersync'); + expect(deployScript).toContain( + 'run --rm --no-deps --interactive=false -T -e MIGRATE_STEP_TIMEOUT=$MIGRATE_STEP_TIMEOUT supersync', + ); + // The host forwards its migration budget into the image so the in-image + // per-step timeout can't silently cap a large MIGRATION_TIMEOUT at the + // image default (1800s) and kill a slow CREATE INDEX CONCURRENTLY early. + expect(deployScript).toContain( + 'MIGRATE_STEP_TIMEOUT="${MIGRATE_STEP_TIMEOUT:-$((MIGRATION_TIMEOUT', + ); expect(deployScript).toContain('prisma db execute'); expect(deployScript).toContain(migrationCommand); expect(deployScript).toContain('Migrator container started');