Skip to content

Keep writer lease renewal from waiting behind fenced writes - #487

Merged
lavaman131 merged 1 commit into
mainfrom
fix/writer-lease-renewal
Oct 12, 2026
Merged

lavaman131 merged 1 commit into
mainfrom
fix/writer-lease-renewal

Conversation

@lavaman131

Copy link
Copy Markdown
Collaborator

Fixes #486. Every fenced write transaction locked the storage_leases row
FOR UPDATE for its whole duration, and renewal is an UPDATE of that same
row, so a renewal queued behind any write that was slow or waiting on
another lock. Renewal also shared the ten-connection pool, so writers
queued on the lease row could leave it no connection, and an attempt had
no timeout: one hung renewal blocked every later one until the watchdog
stopped the server.

Fenced writes now take a per-lease advisory lock, which keeps them
serialized as before, and hold the lease row only FOR KEY SHARE. That
mode does not conflict with renewal's UPDATE of expires_at, but it does
conflict with FOR UPDATE, which acquire now takes explicitly before a
takeover. A new holder therefore still waits for every write in flight,
and a write that starts after a takeover still sees the new fencing
token and fails.

Renewal runs on its own connection with a five-second statement
timeout, and the adapter abandons a connection that has not answered a
second later. main.ts retries a failed renewal every second until the
safety deadline instead of exiting on the first error, measures that
deadline from when the successful renewal was sent, and logs the last
renewal time, how late the watchdog ran, any attempt in flight and the
last renewal error when it fires. docs/storage.md describes the lease
behavior.

Assistant-model: Claude Opus 5.5 (fast)
Assistant-workflow: inline (fast subagents)
Assistant-verification: red before fix: on 518bd44 the three new renewal tests failed against PostgreSQL 17 (renewal still waiting after 2 s behind a fenced commit and behind a full pool; an attempt blocked on the lease row unsettled after 10 s)
Assistant-verification: red before fix: pg_stat_activity showed the renewal UPDATE blocked by a fenced commit that held the lease row while it waited on a channel lock
Assistant-verification: takeover safety test passes before and after, and fails when acquire's FOR UPDATE is removed
Assistant-verification: PostgreSQL suite passed: 92 pass, 0 fail (apps/server/src/storage/postgres against a disposable PostgreSQL 17)
Assistant-verification: bun test passed: 4849 pass, 4 skip, 0 fail (GIT_CONFIG_GLOBAL=/dev/null)
Assistant-verification: bun run types passed
Assistant-verification: bun run ci passed: dprint, oxlint, tokens, design contract, design record, Impeccable (no new findings)
Assistant-verification: local server run: lease row held 18 s, renewal timed out, retried and the server stayed up; held 40 s, watchdog stopped it at 25 s and logged the last renewal and error
Co-authored-by: Alex Lavaee alexlavaee@microsoft.com

Fixes #486

Assistant-verification: Sol and Luna fast review passed: fencing interleavings against a disposable PostgreSQL 17, no P0/P1 (P2 follow-ups: renewal can extend a just-expired lease after a lock wait without a second writer; tidy watchdog/leaseRetry cleanup on drain and startup failure)

Fixes #486. Every fenced write transaction locked the storage_leases row
FOR UPDATE for its whole duration, and renewal is an UPDATE of that same
row, so a renewal queued behind any write that was slow or waiting on
another lock. Renewal also shared the ten-connection pool, so writers
queued on the lease row could leave it no connection, and an attempt had
no timeout: one hung renewal blocked every later one until the watchdog
stopped the server.

Fenced writes now take a per-lease advisory lock, which keeps them
serialized as before, and hold the lease row only FOR KEY SHARE. That
mode does not conflict with renewal's UPDATE of expires_at, but it does
conflict with FOR UPDATE, which acquire now takes explicitly before a
takeover. A new holder therefore still waits for every write in flight,
and a write that starts after a takeover still sees the new fencing
token and fails.

Renewal runs on its own connection with a five-second statement
timeout, and the adapter abandons a connection that has not answered a
second later. main.ts retries a failed renewal every second until the
safety deadline instead of exiting on the first error, measures that
deadline from when the successful renewal was sent, and logs the last
renewal time, how late the watchdog ran, any attempt in flight and the
last renewal error when it fires. docs/storage.md describes the lease
behavior.

Assistant-model: Claude Opus 5.5 (fast)
Assistant-workflow: inline (fast subagents)
Assistant-verification: red before fix: on 518bd44 the three new renewal tests failed against PostgreSQL 17 (renewal still waiting after 2 s behind a fenced commit and behind a full pool; an attempt blocked on the lease row unsettled after 10 s)
Assistant-verification: red before fix: pg_stat_activity showed the renewal UPDATE blocked by a fenced commit that held the lease row while it waited on a channel lock
Assistant-verification: takeover safety test passes before and after, and fails when acquire's FOR UPDATE is removed
Assistant-verification: PostgreSQL suite passed: 92 pass, 0 fail (apps/server/src/storage/postgres against a disposable PostgreSQL 17)
Assistant-verification: bun test passed: 4849 pass, 4 skip, 0 fail (GIT_CONFIG_GLOBAL=/dev/null)
Assistant-verification: bun run types passed
Assistant-verification: bun run ci passed: dprint, oxlint, tokens, design contract, design record, Impeccable (no new findings)
Assistant-verification: local server run: lease row held 18 s, renewal timed out, retried and the server stayed up; held 40 s, watchdog stopped it at 25 s and logged the last renewal and error
Co-authored-by: Alex Lavaee <alexlavaee@microsoft.com>
@lavaman131 lavaman131 self-assigned this Oct 12, 2026
@lavaman131
lavaman131 merged commit 0e522d3 into main Oct 12, 2026
3 checks passed
@lavaman131
lavaman131 deleted the fix/writer-lease-renewal branch October 12, 2026 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server exits with "storage writer lease was not renewed before its safety deadline" under light load

1 participant