feat(db): add heartbeat_runs.issue_lock_release_count (BLO-22060, split from #1124) - #1371
Merged
Merged
Conversation
Split out of #1124 so the code change stops colliding with every migration master lands. Additive column only — no logic reads or writes it yet. sweepStaleIssueLocks bounds a pre-claim issue lock at 6h measured from issues.execution_locked_at, and on expiry clears execution_run_id / execution_locked_at without cancelling the run, so a `scheduled_retry` park survives to fire at its deadline. That post-sweep state is the precondition for re-adoption in enqueueWakeup's legacy-run fallback, which re-stamps execution_locked_at and restarts the clock. The release therefore has to leave a mark on the run itself; the issue row is the wrong place because the sweep nulls it. This counter is that mark. ADD COLUMN with a non-volatile DEFAULT is a catalog-only rewrite on PostgreSQL 11+, so this does not scan heartbeat_runs (~1.8 GB). Refs BLO-22060 Co-Authored-By: Claude <noreply@anthropic.com>
Author
|
🔗 Paperclip issue: BLO-22060 |
1 similar comment
Author
|
🔗 Paperclip issue: BLO-22060 |
Author
|
Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
Follows the one-test-per-migration convention in packages/db. Asserts the two load-bearing properties: the column is NOT NULL with a server default of 0, and pre-existing rows backfill to 0 rather than to the bound, so a run that is legitimately mid-flight at deploy time keeps a full adoption budget instead of being stranded. Resolves the migration by filename suffix rather than a hardcoded number — this one has already been renumbered four times by rebases (0213 -> 0214 -> 0216 -> 0219), and a stale literal fails as an ENOENT that reads as unrelated to the rename. Verified by tampering the migration's DEFAULT to 5, which fails the test, so the assertions genuinely exercise the migration rather than passing vacuously. Refs BLO-22060 Co-Authored-By: Claude <noreply@anthropic.com>
kkroo
pushed a commit
that referenced
this pull request
Aug 16, 2026
…(BLO-22060) `sweepStaleIssueLocks` clears `executionRunId`/`executionLockedAt` on an expired pre-claim lock but deliberately leaves the run parked, so the park can still fire. That post-sweep state is exactly the precondition for re-adoption in `enqueueWakeup`: the legacy-run SELECT matched any run in `EXECUTION_PATH_HEARTBEAT_RUN_STATUSES` with no exclusion of the run that just lost the lock, and `cancelStaleScheduledRetry` declines to cancel a park owned by the issue's own assignee. The same dead holder could therefore re-acquire a fresh 6h lock on every wake, indefinitely. Bound it with a persisted release counter: `sweepStaleIssueLocks` increments `heartbeat_runs.issue_lock_release_count` when it releases a holder, and the legacy-run SELECT filters on `issueLockReleaseCount < MAX_SWEPT_ISSUE_LOCK_RELEASES`. A released holder is passed over exactly once, so total lock time attributable to one parked run is bounded regardless of how many wakes arrive. The bound applies to every status the fallback selects -- `queued` (BLO-18995) and silent-`running` (BLO-19941) holders are released and counted too. Also makes the sweep's optimistic-concurrency skip observable: the pinned UPDATE at `recovery/service.ts` matching zero rows is now logged and counted rather than silently deferring to the next tick. Records a decision on `deferRunForK8sIsolationConflict`: its uncapped re-queue attempts are intentional. That path leaves `executionLockedAt` untouched, so it never resets the stale-lock clock, and the conflict it waits on is transient by construction with the delay already capped at 5 min. Rationale is at the site. The column landed separately on master as `0219` via #1371, so this carries no migration -- which is also why this branch is now squash-linearized: it had accumulated 7 merge commits, leaving `rebaseable=false` against a REBASE merge queue that could never build it. Refs: BLO-22060
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
1 participant
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.
Thinking Path
Linked Issues or Issue Description
Refs #1124 — this is the migration half, split out of it.
Paperclip issue: BLO-22060 (
Stale-lock cap is renewable: enqueueWakeup re-adopts the same parked scheduled_retry run and resets executionLockedAt).What Changed
0219_heartbeat_runs_issue_lock_release_count.sql—ALTER TABLE heartbeat_runs ADD COLUMN issue_lock_release_count integer DEFAULT 0 NOT NULL._journal.jsonentry (idx: 219).issueLockReleaseCounton theheartbeatRunsDrizzle schema so schema and DB stay in sync (no drift on master).No logic reads or writes the column yet. The consuming change stays in #1124.
Verification
Run from
packages/db:The numbering check was exercised against a deliberately-broken control (last journal tag duplicated onto
0218), which exits 1 withDuplicate migration number 0218and returns to exit 0 on restore — so the pass is a real pass, not a silently-skipped run.Diff vs
masteris 59 insertions, 0 deletions across exactly 3 files.Risks
Low.
ADD COLUMNwith a non-volatileDEFAULTis a catalog-only rewrite on PostgreSQL 11+, so it does not scanheartbeat_runs(~1.8 GB) and holdsACCESS EXCLUSIVEonly for the catalog update.0: pre-existing rows have no recorded release, so they keep exactly one more adoption. That is the conservative direction — it cannot strand a run that is legitimately mid-flight at deploy time.DROP COLUMN.master.Model Used
Claude Opus 4.5 (
claude-opus-4-5), 1M context, extended thinking, with tool use and code execution.Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue templatepackages/db/src/heartbeat-runs-issue-lock-release-count-migration.test.ts, following the one-test-per-migration convention. Tests for the consuming logic remain in fix(heartbeat): bound stale-lock re-adoption of a swept parked retry (BLO-22060) #1124.