Skip to content

INotificationOutbox.ack() carries no nodeId, so its compare-and-set can prove a claim exists but not whose #11859

Description

@os-sam

Measured while implementing the ack() status precondition on #11453 (PR #11858). Filed unassigned; not touched in that PR because closing it requires a signature change, which is a contract decision rather than a repair.

What the compare-and-set does and does not close

SqlNotificationOutbox.ack now re-states its precondition in the write:

where: { id, status: 'in_flight' }, multi: true

That closes the defect #11453 named — an ack on an unclaimed pending row, or on an already-terminal one, matches nothing and is refused.

It cannot close the ownership case. ack(id: string, result: AckResult) takes no nodeId, so the predicate can only ask "is this row claimed?", never "is this row claimed by the caller acking it?". The reachable sequence:

  1. node A claims row R and starts a send;
  2. the send outruns claimTtlMs;
  3. another node's claim() reaps R back to pending and re-claims it — R is in_flight again, claimed_by = B;
  4. node A finishes and acks. status = 'in_flight' matches, so A's outcome is written over B's live attempt.

The row carries the answer (claimed_by), and the contract has no way to hand it to the write. MemoryNotificationOutbox has the identical gap.

Why it was not just fixed

Adding the claim owner to ack changes a declared member of a declared interface, and there is more than one shape:

  • ack(id, result, opts?: { nodeId }) — additive, but an optional owner is unenforced by construction: every existing caller keeps the current semantics and the guarantee is only as good as the callers that opt in.
  • ack(id, result, nodeId) required — declared = enforced, and a breaking signature change for both implementations plus every call site.
  • Carry the claim token on the recordclaim() already returns the row; the dispatcher could pass back what it was given, which makes the owner un-forgeable without the caller having to know its own nodeId.

The third is the most interesting and the least explored. It wants deciding with the cancellation surface deferred by the #11454 ruling rather than in isolation, since both are questions about what an outbox row's lifecycle members should carry.

Blast radius today

Bounded but real: the window is a send slower than claimTtlMs (default lockTtlMs * 2), and the consequence is a double delivery whose terminal state records one of the two attempts. The dispatcher's per-partition cluster lock does not help — the reap is what re-opens the row, and any node may hold the partition after a TTL lapse.

Blocked-by: #11453

Links

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions