Add scheduled delivery to the Mocha in-memory transport - #10119
Conversation
Patch coverage80.4% of changed lines covered (197/245)
Uncovered changed lines (JSON){
"sha": "22973d3aadcf4e08d30c83f120dfdf23d5ecd1e2",
"files": [
{ "path": "src/Mocha/src/Mocha.Transport.InMemory/Scheduling/InMemoryScheduledMessageWorker.cs", "ranges": [[41, 42], [108, 109], [111, 111], [118, 125], [127, 127], [129, 131], [145, 146], [148, 148], [166, 168], [173, 173], [175, 178], [181, 181], [183, 185], [187, 187], [194, 195], [200, 202]] },
{ "path": "src/Mocha/src/Mocha.Transport.InMemory/Scheduling/InMemoryTransportScheduledMessageStore.cs", "ranges": [[37, 39], [43, 45], [168, 169], [173, 174]] }
]
}Project coverage: 56.3% (265041/470705 lines) |
There was a problem hiding this comment.
Pull request overview
Adds native (non-durable) scheduled delivery support to Mocha’s in-memory transport, including a transport-owned in-process store + hosted delivery worker, plus scheduler-signal race hardening and documentation/test updates to reflect the new default behavior.
Changes:
- Implement in-memory scheduled message store + hosted worker, wired up automatically via
AddInMemory(). - Harden
MessageBusSchedulerSignalagainst notify/wait races; add regression tests. - Re-point Postgres EF fallback scheduling tests and update docs to reflect in-memory native scheduling and behavior differences.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| website/content/docs/mocha/transports/index.md | Updates transport decision matrix to reflect native in-memory scheduling. |
| website/content/docs/mocha/transports/in-memory.md | Documents in-memory scheduled delivery, cancellation, and non-durability. |
| website/content/docs/mocha/scheduling.md | Updates scheduling docs for in-memory native store and retry/troubleshooting distinctions. |
| website/content/docs/mocha/sagas.md | Updates saga timeout prerequisites to include in-memory native scheduling. |
| src/Mocha/test/Mocha.Transport.InMemory.Tests/Scheduling/InMemoryTransportScheduledMessageStoreTests.cs | Adds unit tests for the new in-memory scheduled message store behavior. |
| src/Mocha/test/Mocha.Transport.InMemory.Tests/Scheduling/InMemorySchedulingTests.cs | Adds end-to-end scheduled delivery tests using FakeTimeProvider. |
| src/Mocha/test/Mocha.Transport.InMemory.Tests/Mocha.Transport.InMemory.Tests.csproj | Adds Microsoft.Extensions.TimeProvider.Testing package for FakeTimeProvider. |
| src/Mocha/test/Mocha.Transport.InMemory.Tests/Helpers/MessageBusBuilder.cs | Starts hosted services in test helper so scheduled worker runs during tests. |
| src/Mocha/test/Mocha.Tests/Scheduling/SchedulingMiddlewareIntegrationTests.cs | Updates integration tests to use an in-memory transport without native scheduling registration when testing “no store” behavior. |
| src/Mocha/test/Mocha.Tests/Scheduling/MessageBusSchedulerSignalTests.cs | Adds regression tests for notify-before-wait and stale-target scenarios. |
| src/Mocha/test/Mocha.Sagas.Tests/IntegrationTests.cs | Removes custom test scheduled store registration now that in-memory has native scheduling. |
| src/Mocha/test/Mocha.EntityFrameworkCore.Postgres.Tests/PostgresSchedulingIntegrationTests.cs | Ensures EF Core Postgres fallback scheduling remains exercised by using an in-memory transport without native scheduling store. |
| src/Mocha/src/Mocha/Scheduling/MessageBusSchedulerSignal.cs | Implements lost-wakeup hardening for scheduler signal notify/wait handshake. |
| src/Mocha/src/Mocha.Transport.InMemory/Scheduling/InMemoryTransportScheduledMessageStore.cs | Introduces in-memory scheduled message store with ordering + cancellation + dedicated signal. |
| src/Mocha/src/Mocha.Transport.InMemory/Scheduling/InMemoryScheduledMessageWorker.cs | Adds hosted worker that dispatches due scheduled messages via normal dispatch pipeline. |
| src/Mocha/src/Mocha.Transport.InMemory/Mocha.Transport.InMemory.csproj | Adds Mocha.Outbox reference needed by the scheduled dispatch worker. |
| src/Mocha/src/Mocha.Transport.InMemory/MessageBusBuilderExtensions.cs | Wires in-memory scheduled store + worker into AddInMemory() by default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Orders entries by scheduled time, then by ID, so distinct entries with the same due time coexist | ||
| /// in the <see cref="SortedSet{T}"/> (a total order is required, otherwise adds are dropped). | ||
| /// </summary> | ||
| internal sealed class ScheduledEntryComparer : IComparer<ScheduledEntry> |
There was a problem hiding this comment.
is it really worth the complexity to make this ordered? like wouldnt a simple linked list be enough. the in memory scheduler is a fallback anyway no?
There was a problem hiding this comment.
The part flagged as complexity is really just the (ScheduledTime, Id) comparer, which any duplicate-tolerant ordered structure needs. A sorted linked list wouldn't be simpler: we'd still need the ordering, plus a Dictionary alongside it for O(1) cancel-by-token (cancellation looks up by token, not by position). SortedSet + Dictionary gives O(log n) add/cancel/peek with true removal.
If the goal is less code and we accept worse worst-case perf, the genuinely simpler option is a plain List<ScheduledEntry> with a linear min-scan and linear cancel, which is fine for the low pending-count an in-memory transport typically holds. A linked list specifically doesn't buy anything over either.
I lean toward keeping SortedSet (correct, scales, comparer is small), but happy to drop to the List version if you'd prefer simplicity. Which way do you want it?
Drafted by Claude (Anthropic AI assistant).
| @@ -10,18 +10,23 @@ internal sealed class MessageBusSchedulerSignal(TimeProvider timeProvider) | |||
|
|
|||
| private DateTimeOffset _target = DateTimeOffset.MaxValue; | |||
| private CancellationTokenSource? _delayCts; | |||
There was a problem hiding this comment.
this race condition fix seems to be in the wrong place. isnt this an order of monitor acquisition issue?
There was a problem hiding this comment.
There's only one lock here (_lock), taken by both Notify and WaitUntilAsync, so there's no acquisition-order hazard between them. The race is a lost wakeup: the wake is edge-triggered (_delayCts.Cancel()), but the sleep (Task.Delay) has to run outside the lock, since we can't hold a lock across an await. So a Notify that fires when no CTS is armed (before a wait starts, or in the gap between two waits) cancels nothing, and the wakeup is lost.
That can't be closed by lock ordering; it needs level-triggered state that survives the gap and is checked under the lock before sleeping. _notified is exactly that pending-wakeup flag, and _isWaiting guards the scheduledTime >= _target early-skip so a stale target left behind by a completed wait can't swallow a notify. It's the standard condition-variable pattern, just hand-rolled because the wait is async rather than Monitor.Wait/Pulse.
Where your instinct is right: this primitive is subtle. The cleaner version bases it on a level-triggered async primitive (SemaphoreSlim(0,1), a Channel, or an async manual-reset event), which makes the notify-before-wait case impossible by construction and lets _notified go away; the _target comparison stays only as an early-skip optimization. That's a broader change though, since this signal is shared with the EF Postgres dispatcher. Happy to do it here or as a follow-up.
Drafted by Claude (Anthropic AI assistant).
Summary
ScheduledTime(viaScheduleSendAsync/SchedulePublishAsync, or a saga'sdescriptor.Timeout(...)) is delivered at ~that time instead of immediately, and can be cancelled via the returned token.AddInMemory()registers a transport-scoped store plus a hosted delivery worker on the post-Rework scheduling #10036ScheduledMessageStoreRegistration/resolver model. Delivery is in-process, non-durable (scheduled messages are lost on restart), and best-effort (no retry).ISchedulerSignalrather than the shared singleton, becauseMessageBusSchedulerSignalis a single-waiter primitive (two workers on one instance would corrupt each other's wait). Hardening that signal's notify/wait handshake against lost wake-ups (notify-before-wait and stale-target-between-waits) also benefits the Postgres dispatcher, which shares the type.Mocha.EntityFrameworkCore.Postgres.Testsscheduling tests are re-pointed onto a bare in-memory transport so they still exercise the EFUsePostgresScheduling()fallback: with in-memory now having native scheduling, the previous "in-memory transport + EF fallback" pairing is obsolete. Docs updated: in-memory scheduling is native, on by default, cancellable, and non-durable, with Postgres-specific troubleshooting scoped accordingly.Test plan
FakeTimeProvider): not-delivered-before-due, delivered-after-due, cancellation prevents delivery, body/headers preserved across pooled-context reuse, same-due-time ordering.Mocha.Tests3940, Sagas 568 pass/4 skip,Mocha.EntityFrameworkCore.Postgres.Testsscheduling 52 (Squadron/Docker).