Skip to content

fix(awareness): stop concurrent dispatchers claiming the same outbox events - #1173

Merged
coodos merged 1 commit into
mainfrom
fix/awareness-outbox-claim-exclusivity
Sep 28, 2026
Merged

coodos merged 1 commit into
mainfrom
fix/awareness-outbox-claim-exclusivity

Conversation

@coodos

@coodos coodos commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #1172, which bounded the claim query's work. That PR left one requirement from the brief unmet — concurrency safety — because the gap turned out to predate it.

The bug

Outbox claiming was not mutually exclusive between dispatchers. Four dispatchers claiming in parallel did not merely overlap occasionally: each returned the identical set of events. Racing four workers against a single packet returned ['d0', 'd0', 'd0', 'd0'].

Neo4j takes the exclusive node lock at SetProperties time, which is after the eligibility Filter has read the row. So every concurrent dispatcher evaluated the predicate against state that was current when it read and stale by the time it wrote, and all of them claimed.

This predates #1172 — I reproduced it against that PR's merge-base, where MATCH ... WHERE ... SET had the same read-then-lock ordering. It is absorbed today rather than corrupting: finish() matches on {eventId, leaseToken} so a loser's completion is a no-op, and AaaS IngestService is idempotent on eventId. The cost is every replica delivering every event, and a lease that means nothing.

The fix

Take the lock before reading the state the row is judged on, by writing a property first:

MATCH (a:AwarenessOutbox { eventId: eventId })
SET a.claimProbe = $leaseToken   // forces the exclusive node lock
WITH a
WHERE <eligible> AND NOT EXISTS { <predecessor> }
SET a.status = 'delivering', ...

A dispatcher that loses the race now blocks on the lock, re-reads the winner's delivering status, and filters itself out.

Three details worth flagging for review:

  • claimProbe is written before the predicate, so it must be a property nothing relies on. Using leaseToken as the probe would have clobbered the winner's token and broken the fencing in finish().
  • The lock probe is bounded by a new read-only shortlist pass. Probing the whole candidate page would mean up to 1000 property writes per cycle and would undo fix(awareness): bound the eVault outbox claim before its predecessor checks #1172. Shortlisting first means the write transaction touches at most limit rows — for the default batch, 50.
  • The predecessor lookup is deliberately left unlocked. Its blocking set is pending/failed/delivering, and the only exit from that set is genuine completion, so a concurrent claim of a predecessor keeps it blocking. No lock needed.

Verified

  • Two new tests: four dispatchers claiming 60 independent events return no duplicates; four racing one packet's stream yield exactly one claim.
  • Both were written failing first and reproduce the original bug. Deleting just the SET a.claimProbe line makes them fail again, so the probe is load-bearing rather than incidentally passing on timing.
  • Ran the spec five times end to end: 7/7 each time, no flakes.
  • EXPLAIN on all four queries — both discovery lanes, the new shortlist, and the lease write — is index-backed with no NodeByLabelScan, so fix(awareness): bound the eVault outbox claim before its predecessor checks #1172's plan shape is intact.
  • Full evault-core suite green: 21 files, 273 tests.

Not measured here: the extra round trip and the lock contention under a production-sized backlog. The shortlist adds one indexed read per page, and the probe adds one property write per shortlisted row; both are bounded by batch size rather than backlog size, but the drain rate is worth watching on /metrics after deploy alongside the #1172 numbers.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: fa59a23e-6668-46a9-bb4e-44fe060ff846


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coodos
coodos merged commit 425b166 into main Sep 28, 2026
6 of 7 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