Release dedup claims when a batched flush fails

[pequalsnp]

The batching paths leaked what #87 fixed in processor.MergeMany. A leaked claim means the redelivery the code explicitly asks for is silently swallowed: dedup_skip fires and the event is gone.

coalesce.go and aggregator.go both take the claim at AddMany/accept time — before the durable write — and released it on no failure path at all.

The fix

Claims are released when a flush fails, on a context detached from cancellation (matching processor.go, since the most common cause of a failed flush is the context being cancelled at shutdown — releasing with that same context fails instantly).

It needs a per-EventID claim-ownership flag, because MarkSeen is fail-open: an error means "proceed unclaimed", and releasing a claim you did not win drops the winner's row and lets a third delivery re-apply the event.

What review caught, and why it mattered

The first attempt's own regression test was worthless. The reviewer mutated context.WithoutCancel(ctx) back to ctx in processor.ReleaseClaims and the entire suite stayed green — including TestCoalescer_CancelledFlushReleasesDedupClaims, which exists specifically to cover that line.

The cause: the deduper fake didn't honour its context on Release, so a cancelled context released just fine in the test and failed only in production. The fake now behaves like the real DynamoDB Deduper — returns ctx.Err() when the context is done — and the mutation now fails the test.

That is the exact shape of this repo's dominant defect class, reproduced inside a test written to prevent it. Worth stating plainly rather than quietly fixing.

Also addressed from review:

• The 5s release budget was shared across a whole failed flush. claimedIDs holds one entry per event and DefaultMaxKeys is 10,000, so a 1s FlushTick at a few thousand events/sec exhausted the budget partway and leaked the remainder.
• The aggregator called ReleaseClaims once per batch inside flushOne, so a shutdown flushAll over K failed batches cost up to K × budget.
• No test covered a mixed-outcome flush — one key succeeds, one fails, disjoint events. A mutant that released the whole pending map regardless of per-key outcome would have passed everything.

The trade this makes, stated plainly

In the coalescer, a partial flush failure releases claims for events that also landed on sibling keys that succeeded, so redelivery re-applies to those keys. That is the correct direction — at-least-once permits re-application but never permits loss — but it is a real over-count exposure and the STABILITY text now says so instead of understating it.

Test plan

• TestCoalescer_FlushFailureReleasesDedupClaims — 100 EventIDs on one key, assert 100 releases, then replay against a healthy store and assert the total is 100 (today: 0 and 0)
• TestBatchWindow_DeadLetteredFlushReleasesDedupClaims
• A claim that was not won is never released
• Mixed-outcome flush
• Large-claim-count budget case
• All verified to fail pre-fix, including under the reviewer's context mutation
• CI green

Related: #87 fixed the same class in the non-batched path.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X2YMxeLgyRc9i5XPyEV75S