Skip to content

Run in-process retries in a fresh scope so saga concurrency conflicts resolve - #10327

Merged
PascalSenn merged 8 commits into
ChilliCream:mainfrom
alisan3:ali/mocha-saga-store-stale-retries
Sep 5, 2026
Merged

PascalSenn merged 8 commits into
ChilliCream:mainfrom
alisan3:ali/mocha-saga-store-stale-retries

Conversation

@alisan3

@alisan3 alisan3 commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Problem

When several messages for one saga instance are handled concurrently, their writes conflict on the saga state's version column. The EF store detects this correctly, but the in-process retry can never resolve it: under contention a saga settles on the order of its own timeout and loses. This makes .Send + .OnReply effectively unusable for a saga whose other inputs arrive on its own queue, because the reply arrives on the shared reply endpoint, outside any queue-level serialization.

Tuning the retry policy does not help. A downstream experiment raised the policy from 3 attempts to 50 attempts at 40 ms for DbUpdateConcurrencyException and measured no improvement, with roughly ten times the conflicts. The retries are not exhausting too early; they cannot succeed.

Root cause: a retry re-runs inside the failed attempt's scope

ReceiveEndpoint.ExecuteAsync creates one DI scope per delivery. The consume-tier ConsumerRetryMiddleware retries by re-invoking the handler with the same context, so every attempt shares that scope and its scoped services. The handler is re-run against whatever the failed attempt left behind.

For the EF saga store that means the DbContext still tracks the instance the failed save used. LoadAsync queries with tracking, EF identity resolution serves that instance back instead of the committed row, and the original version never advances. Every retry re-issues UPDATE … WHERE version = <already moved>. Verified against real Postgres by replaying the store's statements:

load       {"n":0}
save       DbUpdateConcurrencyException      (a concurrent writer committed {"n":1})
retry 1    reload {"n":0,"a":1}  save conflicts
retry 2    reload {"n":0,"a":2}  save conflicts
retry 3    reload {"n":0,"a":3}  save conflicts
fresh scope: load {"n":1}, save succeeds on the first attempt

Note the retry does not even see the stale loaded state: it sees the failed attempt's own write ({"n":0,"a":1}), so a succeeding retry would have applied the transition twice.

This is not a saga problem. A plain IEventHandler with an ordinary scoped DbContext that adds a row, throws once, and saves on the retry inserts the row twice (attempts=2 rowsInserted=2). Every EF-using handler under Mocha's retry was affected; the saga store is only where the symptom became visible, because optimistic concurrency turned the stale tracker into a loud failure instead of a silent duplicate.

Fix

Each retry runs in its own scope. In ConsumerRetryMiddleware, the first attempt keeps the delivery scope, so the common path allocates nothing extra. A retry creates a scope, swaps context.Services, sets the ambient ConsumeContextAccessor there so correlation keeps propagating, runs the handler, and restores the delivery scope afterwards so a later consumer on the same delivery is unaffected. A retry now starts from the same point a redelivery would, one tier down.

Scope-bound caches are reset at the boundary. SagaConsumer caches the saga store on SagaFeature; without a reset a retry would reuse a store from the disposed scope. IScopeBoundFeature marks features that hold a scoped service; SagaFeature is the one implementor and is reset before and after the attempt. A marker was chosen over resolving the store unconditionally because several test harnesses inject a store through the feature and rely on the cache.

The store stops disposing what it does not own. With retries in their own scope, no framework path loads the same saga key twice through one DbContext, so the store needs no detach (an earlier revision of this branch had one; it also cost a DetectChanges scan per load). What remained was that LoadAsync disposed the JsonDocument owned by the entity it had just tracked, so any later read of that entity in the same scope failed on a disposed document. The document now stays with the tracked entity and SaveAsync disposes the one it replaces. A load never followed by a save keeps its document until the scope ends, a pool miss on the failure path rather than a leak.

The save path is otherwise untouched. Its query returning the load-tracked instance is what makes the update check the version observed by this attempt's load; reading fresh at save time would let a transition computed from older state overwrite a concurrent write, the lost-update defect #10085 fixed in the Postgres store. A guard test pins it.

Behavior change

Anything a handler did in a failed attempt that lives in scoped services is now discarded on retry rather than carried into it. That is the intended semantic, and it is what redelivery already does; a handler that deliberately accumulated scoped state across in-process attempts would notice. None was found in this repository. The first attempt and the non-retry path are unchanged.

Tests

Written first, red against the unmodified retry tier, green with it:

Test Without fix With fix
Retry resolves its own scoped service instance (in-memory transport) 1 instance 2
EF handler adds a row, throws once, saves on retry (Sqlite) ["only-once", "only-once"] ["only-once"]
Saga retried after a failed save uses a fresh scoped store 1 store 2, state persisted
A message published from a retry keeps the original ConversationId pass pass (guards the accessor wiring in the new scope)

Store tests against real Postgres: the lost-update guard (a write between load and save still conflicts), loading twice on one store (the disposed-document failure on main), and loading after a save on one store (exercises the replaced document's disposal). These are the first tests DbContextSagaStore has; the InternalsVisibleTo entry mirrors how the sibling Postgres store is tested.

Suites: Mocha.Transport.InMemory.Tests 812/812, Mocha.Sagas.Tests 576 (4 pre-existing skips), Mocha.Tests 4048/4048, Mocha.EntityFrameworkCore.Tests 40/40, Mocha.EntityFrameworkCore.Postgres.Tests 348/348, Mocha.Transport.RabbitMQ.Tests 1836/1836.

Expected effect

With a retry seeing committed state, a conflicted attempt's first retry succeeds, as the fresh-scope control above shows. The default policy of 3 attempts at 200 ms should be sufficient for realistic contention, without policy tuning. Worth re-running the downstream contention scenario against a build with this change; settling should collapse from tens of seconds to roughly one round trip per conflicted message.

Out of scope

  • The batch-consumer path runs the same consume pipeline and its context has a settable Services, so the change applies there too, but it is verified by reasoning rather than by a batch-specific test.
  • Creation races (two messages creating the same saga instance; a correlated message arriving before the create commits) have no defined semantics today and are tracked separately.

🤖 Generated with Claude Code

When a saga write conflicts on the version column, the consume-tier retry
re-invokes the pipeline inside the same DI scope, so every attempt shares one
DbContext. DbContextSagaStore loads with a tracked query, and EF identity
resolution then serves the instance the failed attempt left behind rather
than the committed row: the original version never advances, and every retry
re-issues the same doomed update. Raising the attempt count only multiplies
the conflicts, so under contention a saga settles on the order of its own
timeout and loses.

The tracked instance is also mutated by the failed save, so a retry reads
back the state the failed attempt tried to write rather than what the load
observed. And because LoadAsync disposes the JsonDocument that is the tracked
entity's State property, a second load in the same scope, as happens when a
retry follows a failure before any save, throws ObjectDisposedException.

Detach any tracked instance for the saga key before the load, so each attempt
materializes the committed row fresh. The save path is deliberately untouched:
its query returning the load-tracked instance is what makes the update check
the version observed by this attempt's load, and re-reading at save time would
let a transition computed from older state overwrite a concurrent write. A
guard test pins that a write landing between load and save still conflicts.

A delete retried after a conflict is covered by the same change, since the
saga reloads before it deletes. These are the first tests this store has; the
internals-visible-to entry for the Postgres test project mirrors how the
sibling Postgres store is tested.

Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
alisan3 and others added 2 commits September 3, 2026 18:23
The consume-tier retry re-invoked the handler with the same context, inside
the single scope the receive endpoint creates per delivery. Every attempt
therefore shared the failed attempt's scoped services, and re-ran against
whatever it had left behind: a change tracker still holding the entities it
failed to save, a unit of work half applied. A plain handler that adds a row,
fails once, and saves on the retry inserted the row twice. A saga whose write
conflicted on its version column re-issued the same stale update on every
attempt, so the retries could not succeed at all.

A retry now runs in its own scope, with the ambient consume context set there
so correlation still propagates, and the delivery scope restored afterwards so
a later consumer on the same delivery is unaffected. The first attempt keeps
the delivery scope, so the common path allocates nothing extra. A retry thus
starts from the same point a redelivery would, one tier down.

Features that cache a scoped service must drop it at the scope boundary, or a
retry would reuse an instance from the disposed scope. IScopeBoundFeature
marks them; SagaFeature, which caches the saga store, is the one such feature.
It is reset before the attempt and again after it.

Co-Authored-By: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
With retries running in their own scope, the store no longer needs to detach
the instance a failed attempt left in the change tracker before loading; no
framework path reads the same saga key twice through one DbContext anymore.
The detach also cost a DetectChanges scan over the application's context on
every load.

What remained worth fixing is that LoadAsync disposed the JsonDocument owned by
the entity it had just tracked, so any later read of that entity in the same
scope failed on a disposed document. The document now stays with the tracked
entity, and SaveAsync disposes the one it replaces, at the point the store
owns the swap. A load that is never followed by a save keeps its document
until the scope ends, a pool miss on the failure path rather than a leak.

The tests that exercised a retry through the same store are removed with the
behavior; that guarantee now lives in the pipeline and is tested there. The
lost-update guard stays, since the save must keep checking the version its own
load observed, as does the double-load test, which was the disposed-document
failure on main.

Co-Authored-By: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
@alisan3 alisan3 changed the title Make saga concurrency conflicts resolvable by in-scope retries Run in-process retries in a fresh scope so saga concurrency conflicts resolve Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filename seems to be wrong here

PascalSenn and others added 4 commits September 5, 2026 22:25
A ReceiveContext rented as an attempt clone no longer restores its parked pooled features, which shadowed the receive context's instances through the fallback. The parsing feature is created on the receive context so the message is deserialized once per receive. Batch consumers get a receive scope again, the saga store disposes the documents it loads, and RetryFeature docs match the always-on retry middleware.

Claude-Session: https://claude.ai/code/session_01SRDEKC2sASpSfqXSHrLJBV
The attempt clone only creates its own ReceiveConsumerFeature when the context has none, so consumers that inspect the consumer set, like the reply consumer, see the receive context's instance.

Claude-Session: https://claude.ai/code/session_01SRDEKC2sASpSfqXSHrLJBV

@PascalSenn PascalSenn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PascalSenn
PascalSenn merged commit ad390e9 into ChilliCream:main Sep 5, 2026
147 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants