Repository navigation
fix: bound the alias lock wait in migration apply - #1747
Merged
Merged
Conversation
ensureProjectAliases took LOCK TABLE lcm.project_aliases with no lock timeout of its own. While another session held an uncommitted alias write, apply waited until statement_timeout cancelled the statement and then reported a generic storage error instead of a closed pre-hold refusal. The reconcile now sets a transaction-local lock_timeout of 5 s around the LOCK statement only, restoring the prior setting once the lock is held. A 55P03 or 57014 failure of that statement becomes the typed PostgreSqlIdentityAliasLockTimeoutError, and target-identity maps it to migration-target-identity-unresolvable with evidence alias-lock-timeout. Nothing is written and no hold is taken. The translation happens outside executor.transaction: the real runtime rethrows the first failed query's cached error ahead of the callback's error, so a typed error thrown inside the callback would be replaced. The wrapper only maps when the rejection is the very error recorded from the LOCK statement; later-statement timeouts and set_config failures stay unchanged. A 57014 at the LOCK is attributed to the configured statement_timeout; a caller cancellation is indistinguishable and accepted. The table lock stays. A per-project lock would not exclude the other alias writers, which never take it. Refs #1691 Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
bcdonadio
enabled auto-merge
October 25, 2026 04:10
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Fixes #1691. During
lcm migration apply, reconciling the destination project's aliases takesLOCK TABLE lcm.project_aliases IN SHARE ROW EXCLUSIVE MODEwith no lock deadline. If another session holds a conflicting alias write, the lock waits until the sessionstatement_timeout(60 s by default) cancels it, and apply fails with a generic storage error instead of a closed refusal. Nothing is written and no hold is taken, but the operator gets no actionable reason.How
ensureProjectAliasessets a transaction-locallock_timeoutof 5 s for theLOCKstatement only, and restores the previous value before the reconcile read, so later inserts keep their normal deadlines.55P03(lock timeout) or57014(statement timeout) raised by thatLOCKbecomes the newPostgreSqlIdentityAliasLockTimeoutError(non-retryable, project-scoped, no path or alias data).executor.transactionrejects, and only when the rejection is the exact error recorded from theLOCK. The PostgreSQL runtime rethrows the first failed query's error ahead of any error thrown by the callback, so translating inside the callback would be discarded.resolveMigrationTargetProjectIdmaps the typed error tomigration-target-identity-unresolvablewith evidencealias-lock-timeout(exit 5, before the hold), on both the existing-project and post-create paths. Other errors still propagate.Testing
LOCK, and that the same codes from later statements,set_configfailures, other SQLSTATEs and non-storage errors stay unmapped. New tests fail at the base commit.statementTimeoutMs30000): with an uncommitted alias insert in another session, reconciliation rejects with the typed error in under 15 s, apply's resolver refusesalias-lock-timeout, nothing is written, and reconciliation succeeds once the blocker rolls back.test/postgresql/identity.integration.tspasses 13/13, including the existing wait-then-commit race.identity-repository.tsandtarget-identity.tsat 100% statements, branches, functions and lines.pnpm run typecheckand eslint pass.Reviewer guide
Start with
ensureProjectAliasesinsrc/storage/postgresql/identity-repository.ts(the lock-timeout setup, provenance capture and outside translation), thenadoptinsrc/migration/target-identity.ts. Docs: thealias-lock-timeoutevidence indocs/migration-operations.md. Metadata: patch changeset andtest/codecov-ownership/1691-alias-lock-timeout.yml.