Skip to content

fix(driver-sql): bound the metadata-lock wait on boot schema-sync's MySQL widening ALTER too, keeping boot's swallow (#9542) - #9565

Merged
os-sam merged 1 commit into
mainfrom
claude/issue-9542-boot-bounded-lock-wait
Aug 18, 2026
Merged

fix(driver-sql): bound the metadata-lock wait on boot schema-sync's MySQL widening ALTER too, keeping boot's swallow (#9542)#9565
os-sam merged 1 commit into
mainfrom
claude/issue-9542-boot-bounded-lock-wait

Conversation

@os-sam

Copy link
Copy Markdown
Collaborator

Fixes#9542

Boot schema-sync reaches the MySQL widening ALTER TABLE … MODIFY COLUMN through the same runWideningAlters seam #9354 added, but off the deferred flush — and the seam returned early there, running the ALTER through the pool on MySQL's default lock_wait_timeout of 31,536,000 seconds (one year). One other session holding a metadata lock on the table parked boot at schema-sync for that long, printing nothing. The widening's own logger.warn could not help: it sits in a catch, and an ALTER that never returns is never caught.

This is the card's Option 1 as adjudicated in triage's auto-adjudication comment (5326016191): arm the bound unconditionally, keep the refusal gated on the flush, keep boot's swallow.

The change — one seam in packages/drivers/driver-sql/src/sql-driver.ts

  • The early return narrows from if (!this.flushingDeferredDdl || !this.isMysql) to if (!this.isMysql), so boot also reaches the pinned session and the 120s bound. The dialect check stays: no other dialect takes this lock, and lock_wait_timeout is MySQL's variable.
  • The refusal moves behind the flag: if (this.flushingDeferredDdl && isMysqlLockWaitTimeout(err)). Off the flush the server's own error is rethrown, and the widening's existing catch logs and swallows it. Boot's policy is unchanged — correctness never depends on the widening having run, and a migration must never take boot down, so throwing here would trade a silent hang for a failed boot, which is a different answer rather than the same one.
  • Two doc blocks are rewritten because the code they describe no longer does what they say: the runWideningAlters paragraph that read "Only armed for the flush", and the DEFERRED_DDL_LOCK_WAIT_TIMEOUT_SECONDS header. The constant keeps its name — renaming it would touch four further sites for no behavioural gain — and now states in its own header that it governs both callers.

120s is #9354's number, kept deliberately. The card's open question asked whether boot should be more patient than an operator at a prompt; triage ruled the same bound. The reasoning behind 120s is about how long a legitimate metadata-lock holder can plausibly hold the lock — a property of the lock, not of who is waiting on it. Boot's difference from the flush is what happens when the bound fires, never how long it waits. No retry logic, no configurability.

What happened to the two pins in sql-driver-deferred-ddl-lock-wait.test.ts

leaves BOOT sync unbounded and swallowing — it is not the flush — FLIPPED, not deleted. It is now bounds BOOT sync too, and still swallows — boot is not the flush. Exactly one of its two assertions inverted: expect(setStatements(driver)).toHaveLength(0) became a positive pin that the bound is armed at [120], on the ALTER's own session, before the ALTER, and restored afterwards. The other half — await expect(driver.initObjects([WIDGET])).resolves.toBeUndefined() — does not invert and is called out in the test as the half that stays: it is what pins that boot still swallows and no refusal escapes.

clears the flush flag after a refusal, so a later boot sync is unaffected — KEPT, with its observable swapped for one that still discriminates. This pin guards flag hygiene, not boot policy; it only ever shared an assertion with its neighbour. Its setStatements → 0 line says nothing about the flag any more, because a clean boot now arms the bound too and that count is 2 either way — updating it to match the flipped pin would have retired the guard silently. The discriminating observable is the swallow on the line above (a stuck flag makes this same lock wait escape initObjects as a refusal); the replaced line now pins alterStatements(driver) at 1, so the swallow cannot pass vacuously — the widening genuinely ran and genuinely hit the lock wait.

The boot logger.warn actually fires — asserted, not assumed

A bound that produces a swallowed error with no log output would deliver nothing and would look identical in a green run, so the new test finally reaches the boot logger.warn asserts on a sink: the fake driver captures logger.warn, and the test pins that the warn names the table and carries the server's Lock wait timeout exceeded as its error field — and that the ADR-0112 refusal's operator sentence is absent, since that envelope stays flush-only.

Verification

Reverse verification, run from the committed state by reverse-applying the source diff (test file untouched), then restored and re-run:

  • ablation (b7f7e2e source reverted): Tests 3 failed | 8 passed (11) — the flipped boot pin, the warn pin and the flag-hygiene pin all red; every flush pin unmoved.
  • restored: Tests 11 passed (11), working tree byte-identical to HEAD (git diff --exit-code clean).

No rebuild was needed for either leg: the suite imports ./sql-driver.js, a relative specifier inside its own package, so vitest resolves it to src/ and no dist/ participates.

One detail worth recording, because it is why the warn pin asserts on meta.error rather than merely on a warn existing: under the ablation the warn does fire in the fake, carrying a SQLite syntax error — the fake's real in-memory SQLite rejects MODIFY COLUMN, whereas production hangs. The assertion that discriminates the fix is the lock-wait text, which is only reachable once the bounded, pinned path is taken.

Gates, re-derived with node scripts/pm/dispatch-gates.mjs (no paths) after the final commit and run on that tree, b7f7e2e:

  • full @objectstack/driver-sql suite: Test Files 101 passed | 5 skipped (106), Tests 1757 passed | 62 skipped (1819)
  • pnpm --filter @objectstack/driver-sql typecheck: clean
  • CLI schema-migrate.deferred-ddl.integration.test.ts: 3 passed
  • check:nul-bytes, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:type-check-coverage, check:engine-double-contract, check:where-matcher, docs-audit/check-affected-docs.mjs — all green
  • check:type-check-debt --re-measure on the built closure: 33 ledger entr(ies) re-measured in 246.7s, 1926 raw tsc error(s) total, none above its recorded number — measured, not refused
  • re-derivation added five gates the dispatch list did not name, all from the changeset file, all green: check:changeset-gate-self-tests, check:objectui-changeset, check-adr-0087-registration.mjs, check-changeset-no-major.mjs, check-empty-changeset.mjs

Generated by Claude Code


Generated by Claude Code

…ySQL widening ALTER too, keeping boot's swallow (#9542)
#9354 armed the 120s `lock_wait_timeout` bound only while the deferred-DDL
flush was running, so boot schema-sync reached the same widening ALTER through
`runWideningAlters`'s early return and inherited MySQL's one-year default. A
metadata lock held by another session parked boot there silently, and the
widening's own `logger.warn` was unreachable — it is in a `catch`, and the
ALTER never returned.
The bound is now armed unconditionally; `flushingDeferredDdl` gates only the
escape from the swallow. Boot warns and carries on, `os migrate apply` keeps
its DATABASE_ERROR/500 refusal.
The suite's boot pin is flipped rather than deleted, and the neighbouring
flag-hygiene pin swaps to an observable that still discriminates a stuck flag.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qYPmkKEsfbWY1yVg83p8F
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 2 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx(via SqlDriver (symbol))
  • content/docs/plugins/packages.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/index.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/lifecycle.mdx(via SqlDriver (symbol))
  • content/docs/protocol/objectql/query-syntax.mdx(via SqlDriver (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via SqlDriver (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 65d4fff7cb4b5fe1ac4555f602f6bd120003f4c2packageMentionDocs.

Which tree this was computed on

This run read content/docs from 0ea2086e78fce5e85aed9435229c35b1f37c62e8 — the merge of head b7f7e2e64138ad59c5fe154c1fde257836ea59b2 into base 65d4fff7cb4b5fe1ac4555f602f6bd120003f4c2, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0ea2086e78fce5e85aed9435229c35b1f37c62e8 && git checkout 0ea2086e78fce5e85aed9435229c35b1f37c62e8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 65d4fff7cb4b5fe1ac4555f602f6bd120003f4c2 b7f7e2e64138ad59c5fe154c1fde257836ea59b2 && git checkout -B drift-repro 65d4fff7cb4b5fe1ac4555f602f6bd120003f4c2 && git merge --no-ff b7f7e2e64138ad59c5fe154c1fde257836ea59b2
node scripts/docs-audit/affected-docs.mjs --json 65d4fff7cb4b5fe1ac4555f602f6bd120003f4c2

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 65d4fff7cb4b5fe1ac4555f602f6bd120003f4c2 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

driver-sql: boot schema-sync's MySQL widening ALTER is still unbounded (one year) and still swallows a metadata-lock timeout

2 participants

@os-sam@claude