Repository navigation
Outbox drain claim: the residuals ADR-0035 does not close, and the INVARIANT-keying question it raised #519
Description
Status
The gap this issue was filed for has been answered. IPollableOutboxStore.TryClaimForDispatch shipped in #444 (merged as 7408c31) and is recorded by docs/adr/0035-the-outbox-drain-claims-the-row-it-is-about-to-publish-inside-the-transaction-that-publishes-it.md.
What shipped, at HEAD:
IPollableOutboxStore.TryClaimForDispatch(src/Chatter.MessageBrokers/src/Chatter.MessageBrokers/Reliability/Outbox/IPollableOutboxStore.cs:137) — a compare-and-set that moves a row'sNextAttemptAtUtcfrom the value the poll reported to a claimed instant and answers whether this caller moved it. It is a grant-by-default interface implementation, following theRecordDispatchAttemptprecedent, so a third-party store written against the previous shape still compiles and still satisfies the cast at the poll site.OutboxProcessor.Processtakes the claim as the first statement inside the unit of work that carries the publish (Reliability/Outbox/OutboxProcessor.cs:146), and a denial returns rather than throwing, so a denied drain publishes nothing, stamps nothing and costs the row nothing.InMemoryBrokeredMessageOutbox.TryClaimForDispatch(Reliability/InMemoryBrokeredMessageOutbox.cs:239-263) — an in-place compare-and-set on the stored row under a private monitor, gated on unprocessed, on the observed due instant, and on dueness at claim time.BrokeredMessageOutbox<TContext>.TryClaimForDispatch(src/Chatter.MessageBrokers.Reliability.EntityFramework/…/BrokeredMessageOutbox.cs:359) — transaction-scoped, so a losing drain waits on the raced row's lock for the winner's whole publish and then matches zero rows.- It rides the
NextAttemptAtUtccolumn ADR-0031 already added rather than adding a column, with no lease and no reaper.
The issue stays open for the residuals below, not because a claim member is missing.
Correction to the original framing
The gap was outbox-wide, not confined to a store without a transaction. The original title said "non-transactional"; that was false. The relational store duplicated too: its drain poll ran outside any transaction, both drain queries are plain lock-free reads at ReadCommitted, and the only arbiter was the ProcessedFromOutboxAtUtc concurrency token, whose predicate is emitted at SaveChanges — after the publish. It therefore prevented a double claim and never a double dispatch: both hosts selected the row, both published, and only then did one lose. The measured statement of this is in src/Chatter.MessageBrokers.Reliability.EntityFramework/CONTEXT.md under Relational Drain Claim.
What ADR-0035 does not close
This is what remains of this issue's original subject:
- Cross-process duplicates over a store that takes the grant-by-default, or that ignores the member entirely. The member is documented, not enforced.
- Range or scan claim predicates. Every lock measurement covers a single-row primary-key seek.
- The in-memory tier's one-backoff window. A publish running longer than one backoff leaves the row due again while the claiming drain is still publishing.
- The
<=/<boundary againstnowat the in-memory claim's due gate (InMemoryBrokeredMessageOutbox.cs:237-238), which no oracle pins for the same reason the poll's own boundary is unpinned. - A host shutdown between publish and claim leaves the outbox row unclaimed, and the re-claim cannot be reused to fix it #508 — the
OutboxProcessor.Processhost-cancellation arm never consults thepublishedflag.
Constraints the shipped design honoured, and the one it did not
- Interface-evolution precedent — honoured. The claim member defaults to GRANT, so a store written against the previous interface shape keeps exactly the behaviour it has today. What such a store gives up is arbitration.
- Claim state lives in the entry it claims — honoured. PR Claim the inbox message id before invoking the handler (#380, #507) #511 was closed unmerged after seven review iterations because it recorded claim state in side-records held outside the row, each written non-atomically with the write it described, so every iteration closed one window and opened the next. The merged relational-inbox answer (fix(reliability-ef): tell a claimed inbox marker from a handled one #518, ADR-0033) put the claim's state only in the row, and ADR-0035 does the same for the outbox: the claim is a write to the row's own
NextAttemptAtUtc. - No lease — honoured. A lease whose release is missed converts a duplicate into a message that is never sent. The claim needs neither a lease nor a reaper on either tier, and the derivation is written once in the remarks on
TryClaimForDispatch: a unit of work that rolls back has already handed the row back, and a unit of work that cannot roll back holds nothing outliving the claimed instant. - ADR-0031 — honoured. The claim adjudicates at dispatch, not at selection, so it does not become a second selection input that can disagree with durable attempt state.
- Single atomic dictionary operation — deliberately NOT met. The in-memory claim is a compare-and-set performed in place on the stored instance under a private monitor.
ConcurrentDictionary.TryUpdatewas considered and rejected on the record because it swaps in a new instance, which breaks the write-through reference identity the store documents onRecordDispatchAttempt— a poll hands back the stored instances themselves, so a drain holding a row polled a moment earlier would go on writing to an instance the dictionary no longer holds. The two properties cannot both hold in this store; write-through is pinned by six named facts and the single-operation shape by none, so the single-operation shape yielded. The honest cost is recorded with the mechanism (InMemoryBrokeredMessageOutbox.cs:28-47, 199-238): removing the monitor reddens nothing measurable in either suite on either target framework, because what it buys is a torn read no single-threaded fact can stage.
Rejected direction
Declare the both-drains combination unsupported and refuse it at startup.
Rejected. It fails the closed-by-construction acceptance test: it names no eliminated class, it only refuses one configuration, and a third-party store gets nothing from it. It also cannot be built well here, because no single place knows both facts and src/Chatter.MessageBrokers/CONTEXT.md records a deliberate decision against DI descriptor inspection for this family under Reliability-Store Facet Resolution.
Watch item
The reachability argument that made #444 a documented-and-accepted duplicate rather than an urgent defect rests on today's registration doors. The only shipped door registering OutboxProcessingBehavior<> also replaces IBrokeredMessageOutbox with the relational store, and core registers no IUnitOfWork that writes CurrentTransactionId. If a future shipped door registers OutboxProcessingBehavior<> without replacing IBrokeredMessageOutbox, or if core ever registers such an IUnitOfWork, revisit this issue first.
ADR-0027's promotion trigger has fired
Recorded here rather than as a separate issue, because this is already the structural home for this surface. Nothing below is outstanding work on the three sites it names — all three are corrected at HEAD. What is open is the design question at the end.
The class. An INVARIANT: comment names an oracle and states the mutation that turns that oracle red. Nothing binds that prose to what the named test actually asserts, so a comment can name a real test, that test can still exist and still pass, and the comment can still be wrong about what the test covers.
It is a staleness failure, not an authoring one. All three instances were written correct and falsified by a later change in the same repository. The named test still existed and still passed, so nothing flagged them.
The shared root. All three were a NON-RECORDING EXIT whose prose described the row as unchanged and due now. Each was written when no pre-dispatch write existed at all. The drain claim introduced one, and its durability is per store tier: a unit of work that rolls back discards it, a store that keeps no transaction keeps it. No exit prose was re-measured against that.
The three instances were the cancellation exemption on OutboxProcessor, the TryClaimForDispatch note on the relational BrokeredMessageOutbox, and RecordFailedDispatchAttempt's remarks on OutboxProcessor. docs/adr/0027-invariant-prose-names-the-oracle-that-falsifies-it.md records the count at one and says the trigger is not tripped at one where it asks for three. These are instances two, three and four, and none is restatement drift, which the trigger's own terms exclude.
The question for whoever picks this up
Is prose naming a test the right key at all? Candidate directions, none endorsed:
- bind the claim to the test by something a compiler or a test run can check, rather than by a name in a comment;
- make the perturbation count machine-verifiable, so an exclusivity count is produced rather than asserted;
- narrow what an
INVARIANT:is permitted to claim, so that exclusivity counts stop being assertions that can rot.
ADR-0027 already rejected the naive forms of the first two — a corpus sweep as enumeration, and a grep lint because it can verify that an oracle is named but not whether the named oracle pins that claim.
A narrower and much cheaper candidate: state the per-store durability of a pre-dispatch write once, on IPollableOutboxStore.TryClaimForDispatch, and cite it from each exit — ADR-0027's own Rule 2 — instead of restating the row's fate at every exit. That is what shipped, and it is why three comments needed the same correction.
The honest counter-argument. All three instances were caught, two by adversarial review and one by a perturbation audit, so no undetected defect is sitting in the code. What the trigger measures is cost: a dedicated audit step did nothing but re-measure written exclusivity counts against real test runs, and a full pre-PR review cycle went on top of that. The question is whether the discipline is paying for itself and whether a cheaper key exists.
An extension to ADR-0027 Rule 2 that was applied and deliberately NOT codified
The gap in Rule 2. Rule 2 of ADR-0027 says the rationale for a mechanism lives once in the comment adjacent to that mechanism, and that CONTEXT.md, README.md and CHANGELOG.md cite the mechanism and the oracle's name rather than restating the reasoning. That assumes every surface it names can follow a citation. Two cannot. A module's README.md is packed into its NuGet package — Directory.Build.props sets PackageReadmeFile for every packable library — and renders on nuget.org, where "see the remarks on IPollableOutboxStore.TryClaimForDispatch" reaches nobody. A CHANGELOG.md is release notes and is read the same way. On those two surfaces a citation is a dead end, leaving the author with either silence or a restatement.
The rule that was applied instead. On a surface that cannot follow an in-repo citation, state what a granted drain claim leaves behind as an observable OUTCOME PER STORE, and never as the lifetime mechanism that produces it.
Why that works. The outcomes genuinely differ between the two shipped stores: over a relational store a rollback hands the row back at once, and over the in-memory store the row returns one backoff later. Each is recorded by its own oracle, and both are named in the remarks on TryClaimForDispatch. Because the two outcomes differ, there is no tier-neutral outcome sentence available to write. The drifted form was only ever writable because these surfaces stated the mechanism, and at that altitude one abstract story sounds true of both stores. Move the sentence down to outcomes and the drifted sentence has no grammatical home left.
Checkable at review, structurally rather than by vocabulary. Banning words like "lease" and "reaper" from packed surfaces does not survive contact with what shipped, since both appear there legitimately in negated and store-qualified form. The check that holds: a sentence on a packed surface describing what a granted drain claim leaves behind must name a store and an observable; one that names neither is out of bounds.
Where it was applied:
src/Chatter.MessageBrokers/src/README.mdsrc/Chatter.MessageBrokers/src/Chatter.MessageBrokers/CHANGELOG.mdsrc/Chatter.MessageBrokers.Reliability.EntityFramework/src/README.mdsrc/Chatter.MessageBrokers.Reliability.EntityFramework/src/Chatter.MessageBrokers.Reliability.EntityFramework/CHANGELOG.md
The honest status: not codified. ADR-0027 was not amended, and that was a decision rather than an oversight — widening an ADR in the middle of the remediation that discovered the gap was judged out of scope. The rule therefore holds on four files by having been applied there, and nowhere else. The next packed surface written about behaviour that differs by store would rediscover the drift with nothing to stop it.
Links
#444, #508, #441, #300, #327, #328, #442, #511, #518, #520.
ADRs: docs/adr/0031-outbox-selection-is-derived-from-durable-attempt-state-not-from-the-absence-of-success.md, docs/adr/0033-the-relational-inbox-claims-before-the-handler-and-stamps-handled-after-it-in-the-same-row.md, docs/adr/0035-the-outbox-drain-claims-the-row-it-is-about-to-publish-inside-the-transaction-that-publishes-it.md, docs/adr/0027-invariant-prose-names-the-oracle-that-falsifies-it.md, docs/adr/0006-two-tier-reliability-relational-ambient-tx-vs-nosql-stage-then-commit.md.
Activity
[-]IPollableOutboxStore gives a non-transactional store no way to claim a row before a drain dispatches it[/-][+]IPollableOutboxStore gave no store — the relational one included — a way to claim a row before a drain dispatched it[/+][-]IPollableOutboxStore gave no store — the relational one included — a way to claim a row before a drain dispatched it[/-][+]Outbox drain claim: the residuals ADR-0035 does not close, and the INVARIANT-keying question it raised[/+]