fix(sqlite): use BEGIN EXCLUSIVE for migration lock; TTL sweep + heartbeat for non-migration advisory locks#2666
Merged
jeremydmiller merged 2 commits intomainfrom May 4, 2026
Merged
Conversation
The row-based wolverine_locks scheme couldn't serialize migration: the table is created by the migration it's supposed to protect, so the first migration per tenant burned ~5.5s of failed lock retries before running unprotected. Migration now uses BEGIN EXCLUSIVE; polling keeps the row lock (by then the table exists). From #2636 (dmytro-pryvedeniuk): - SqliteAdvisoryLock.TryAttainLockAsync now idempotent via HasLock - SqliteAdvisoryLock.DisposeAsync NRE: dropped duplicate close+dispose - SqliteMessageStore polling overrides delegate to SqliteAdvisoryLock (TryAttain checks affected rows; Release actually deletes the row) - Dropped racy "not yet delivered" assertion in scheduled-tenant test Not picked: CreateLocksTableIfMissing in the lock hot path -- unneeded once migration uses BEGIN EXCLUSIVE. Also skips can_send_from_one_node_to_another_by_destination on the SQLite local fixture (single-host, no second node) and adds 4 focused migration-lock tests.
Closed
wolverine_locks rows aren't bound to the writing connection, so a hard-killed holder leaves a row no peer reaps. Sweep stale rows on attempt; refresh acquired_at when the live holder re-attains. Live holders re-attempt on every poll tick (HealthCheck/ScheduledJob), so the heartbeat is implicit. Default TTL 2m. Split sqlite_migration_lock.cs: migration-lock tests stay; advisory- lock tests (idempotency, release, TTL/heartbeat) move to a new sqlite_advisory_lock.cs.
mysticmind
changed the title
This was referenced May 4, 2026
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
2 participants
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.
The row-based wolverine_locks scheme couldn't serialize migration: the table is created by the migration it's supposed to protect, so the first migration per tenant burned ~5.5s of failed lock retries before running unprotected. Migration now uses BEGIN EXCLUSIVE; polling keeps the row lock (by then the table exists).
From #2636 (dmytro-pryvedeniuk):
Not picked: CreateLocksTableIfMissing in the lock hot path -- unneeded once migration uses BEGIN EXCLUSIVE.
Also skips can_send_from_one_node_to_another_by_destination on the SQLite local fixture (single-host, no second node) and adds 4 focused migration-lock tests.
non-migration advisory lock:
wolverine_locks rows aren't bound to the writing connection, so a hard-killed holder leaves a row no peer reaps. Sweep stale rows on attempt; refresh acquired_at when the live holder re-attains. Live holders re-attempt on every poll tick (HealthCheck/ScheduledJob), so the heartbeat is implicit. Default TTL 2m.
Split sqlite_migration_lock.cs: migration-lock tests stay; advisory-lock tests (idempotency, release, TTL/heartbeat) move to a new sqlite_advisory_lock.cs.