Skip to content

fix(lease): fix flaky primary/follower leasing test - #2

Merged
mcollina merged 1 commit into
mainfrom
fix/flaky-leasing-test
Oct 13, 2026
Merged

mcollina merged 1 commit into
mainfrom
fix/flaky-leasing-test

Conversation

@mcollina

Copy link
Copy Markdown
Member

Fixes the intermittent failure of Replica.run() with a leaser (primary/follower) › one replica is primary and the other follows; takeover on release (timed out at ~2081ms).

Root cause

Two bugs combined:

  1. Replica.leaser setter didn't demote to follower. The constructor option sets leasePrimary = false, but the setter (which the test uses) left it at the default true. waitFor(() => replicaA.isPrimary()) therefore returned before A acquired anything, so B's run() raced A for the lock. When B won, waitFor(() => !replicaB.isPrimary(), 2000) timed out.
  2. FileLeaser.acquireLease wasn't atomic. open(path, 'wx') created an empty lock.json and wrote it afterwards. A racing acquirer hitting EEXIST could read the empty file and throw a SyntaxError instead of LeaseExistsError.

Changes

  • src/replica.ts: the setter now mirrors the constructor (leasePrimary = leaser === null), except while run() is active.
  • src/backends/file/leaser.ts: write the lock to a temp file and link() it into place, so it appears atomically with full contents. Temp names get a per-process counter to avoid same-millisecond collisions.
  • test/integration/leasing.test.ts: added a setter regression test (fails deterministically without the fix) and a concurrent-acquire test (4 leasers × 50 rounds, exactly one winner, the rest get LeaseExistsError).

Verification

  • pnpm test:all (119 + 18), pnpm typecheck, pnpm lint all pass.
  • Leasing test file: 15/15 consecutive passes. The original flake didn't reproduce locally, so the deterministic setter test is the main evidence.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KdmZiQnN1N4wmoaX7dpbJS

…create

The leasing integration test was flaky because two bugs combined:

- `Replica.leaser = …` did not reset `leasePrimary`, unlike the
  constructor option, so `isPrimary()` stayed true before any lease was
  acquired. The test's wait for A to become primary returned immediately,
  letting B race A for the lock; when B won, the test timed out.
- `FileLeaser.acquireLease` created the lock with `open(path, 'wx')` and
  wrote it afterwards, so a racing acquirer hitting EEXIST could read an
  empty file and fail with a SyntaxError instead of LeaseExistsError.
  The lock is now written to a temp file and hard-linked into place.

Temp file names also get a per-process counter to avoid same-millisecond
collisions between leasers in one process.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KdmZiQnN1N4wmoaX7dpbJS
@mcollina
mcollina merged commit 4a2fafc into main Oct 13, 2026
3 checks passed
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.

1 participant