Feat/reminder concurrency control - #10232
Draft
rkargMsft wants to merge 8 commits into
Draft
Conversation
Adds an opt-in mechanism for bounding the rate and concurrency of reminder tick dispatches on a silo. When unconfigured, the default registration is a zero-allocation no-op throttle; behavior is unchanged. When configured via .AddReminderConcurrencyControl(c => c.PerSilo(...)), dispatches must acquire a lease from a SemaphoreSlim + token-bucket throttle before firing IRemindable.ReceiveReminder, trading tick-time accuracy for downstream protection. New public surface lives in Orleans.Reminders.Concurrency (IReminderDeliveryThrottle SPI, NoOp/Local throttle implementations, ThrottleConfig with a closed-factory ThrottleBlockMode that prevents inconsistent configurations, ReminderConcurrencyBuilder). Observability flows through new Microsoft.Orleans.Reminders ActivitySource, new orleans-reminders-throttle-* metrics with OTel-compliant tag keys, and a TickSkipped diagnostic event. See src/Orleans.Reminders/Concurrency/README.md for user-facing documentation. Phase 1 ships only the per-silo tier; the SPI shape leaves room for global / per-grain-interface / per-reminder cluster-scoped tiers planned for Phase 2. Tests: 21 unit tests covering the SPI, validators, semaphore + token-bucket behavior, lease semantics, and cancellation; 5 cluster-based integration tests (using InProcessTestCluster + ReminderTestClock) validating DI wiring, startup validation, end-to-end delivery with throttling enabled, and TickSkipped events firing under rate-limit pressure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three concurrency bugs surfaced by post-PR adversarial review of LocalReminderDeliveryThrottle and fixed with regression tests: 1. Concurrency permit leak when rate-acquire phase throws OperationCanceledException. The rate-acquire call is now wrapped in try/catch that releases the previously-acquired concurrency permit before rethrowing. Under the previous code, every in-flight reminder waiting on a rate token at silo shutdown would permanently consume a concurrency permit. 2. Concurrency permit leak in WaitSemaphoreWithTimeoutAsync when the timeout fires at the same instant the semaphore grants. The previous implementation used a ContinueWith(_ => true) projection that erased the distinction between a successful grant and a cancelled wait; if the delay won the WhenAny race after the grant succeeded, the permit was held but no caller was tracking it. Replaced with a single linked-token CTS that relies on SemaphoreSlim.WaitAsync's documented atomic grant-or-cancel contract. 3. Rate-acquire deadline math used TimeProvider.System.TimestampFrequency to scale seconds-to-ticks while comparing against _timeProvider.GetTimestamp() from a configurable TimeProvider. On platforms where Stopwatch.Frequency differs from the configured TimeProvider's frequency (e.g., Linux, or any test using FakeTimeProvider with a non-default frequency), the WaitWithTimeout rate path would terminate early or hang. Rewritten to use TimeSpan-based comparison via _timeProvider.GetElapsedTime, eliminating cross-provider unit mixing. Regression tests: Cancellation_DuringRateWait_ReleasesConcurrencyPermit verifies #1 by driving the test throttle into the rate-wait state then cancelling. WaitUpTo_OnRateLimitedPath_SkipsWhenWaitExceedsTimeout exercises the corrected #3 deadline path on a FakeTimeProvider. Issue #2 is verified by inspection of the new implementation; the contract is now a direct consequence of SemaphoreSlim.WaitAsync's semantics. All 28 tests (21 unit + 5 cluster integration + 2 new regression) pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Four correctness bugs surfaced by a second adversarial review (gpt-5.5) of the LocalReminderDeliveryThrottle and LocalReminderService integration; fixed with regression tests: 1. Shutdown deadlock with WaitMode throttle. StopDeliveringReminders waited for delivery quiescence BEFORE invoking LocalReminderData.StopAsync (which is what cancels the per-reminder _stopCancellation token). A throttle wait in ThrottleBlockMode.Wait was counted as an active delivery but its cancellation token would never be signalled, causing silo shutdown to hang indefinitely. Reordered to initiate StopAsync first; in-flight grain calls do not observe _stopCancellation so they still drain naturally during the quiescence wait. 2. WaitUpTo timeout applied per phase instead of as a single budget for the acquire. When both concurrency and rate limits were configured, a WaitUpTo(800ms) call could spend 800ms waiting on the semaphore then start a fresh 800ms budget on the rate bucket, producing lease.WaitedFor > timeout. Restructured AcquireAsync to compute a single shared budget at entry and pass the remaining budget into each phase helper. 3. TickStatus.CurrentTickTime constructed before throttle admission. A delayed grain call observed a stale timestamp, and TickFiring + tardiness were emitted for ticks the grain would never see (when SkipImmediately or WaitUpTo skipped them). Moved status construction, TickFiring emission, and tardiness recording to after admission; the throttle context carries a provisional status with the pre-wait timestamp so the throttle still has accurate scheduling info. 4. Pre-cancelled cancellation tokens could still consume a permit via the fast-path Wait(0) or bucket-consume. Added cancellationToken.ThrowIfCancellationRequested() at the top of AcquireAsync. Regression tests added (all passing): WaitUpTo_BudgetIsSharedAcrossConcurrencyAndRatePhases, AcquireAsync_ThrowsImmediately_WhenTokenAlreadyCancelled, SiloShutdown_CompletesPromptly_WhenThrottleWaitsAreInFlight (cluster), NoTickFiringEvent_WhenTickIsSkippedByThrottle (cluster). All 31 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…throttle Brings reminder concurrency control into alignment with the equivalent capabilities in Orleans.DurableJobs (IOverloadDetector integration and slow-start ramp-up) while preserving the reminder-specific SPI and block-mode model. Both features are opt-in and require an explicit ThrottleBlockMode (Wait / WaitUpTo / SkipImmediately) at configuration time so that the resulting behavior is a deliberate user decision, not a silent default. Overload backoff: .RespectOverload(onOverload) integrates with the silo's IOverloadDetector (the same cross-cutting signal already honored by the gateway, placement directors, and DurableJobs). When overload is reported, dispatch behavior follows the configured block mode: Wait polls until clear, WaitUpTo polls up to a bounded timeout, SkipImmediately returns Skipped(SiloOverloaded). The overload check runs before any permit or token is consumed, so an overloaded silo never costs the throttle a permit. RespectOverload is also valid as a sole tier. Slow-start ramp-up: .SlowStart(initialCapacity, interval, onCapacityExceeded) mirrors the equivalent behavior in DurableJobsOptions (SlowStartInitialConcurrency, SlowStartInterval). The configured MaxConcurrent becomes a target; effective concurrency starts at initialCapacity and doubles every interval until reaching MaxConcurrent. Slow-start mitigates cold-start thundering herds after silo restart or membership change. The configured block mode applies while the ramping capacity is exhausted: Wait blocks until the ramp opens more capacity, WaitUpTo waits up to a bounded timeout, SkipImmediately returns Skipped(SlowStartLimited). Pit-of-success guarantees enforced at compile/build time: SlowStart requires MaxConcurrent (validated), InitialCapacity cannot exceed MaxConcurrent (validated), both features require an explicit block mode (no defaults), and RespectOverload throws a clear error at silo startup if IOverloadDetector is not registered. The 'at least one of MaxConcurrent / PermitsPerSecond / RespectOverload' validation makes RespectOverload a valid sole tier. Implementation: LocalReminderDeliveryThrottle is restructured into a four-phase pipeline (overload -> slow-start -> concurrency -> rate). Earlier phases run first so an overloaded silo's protection is honored before permits or tokens are consumed. Concurrency and rate phases continue to share a single WaitUpTo budget (Round 2 fix from prior reviews); overload and slow-start each apply their own block mode. The ramp-up task is started synchronously (not via Task.Run) so that its initial Task.Delay registration with the supplied TimeProvider happens on the calling thread before the constructor returns; this is important for tests that drive a FakeTimeProvider. Tests: 16 new tests (14 unit + 2 cluster) covering each block mode for both features, ramp progression over time, validation failures, sole-tier-RespectOverload, missing-IOverloadDetector, and end-to-end skip events with a fake IOverloadDetector replacing the silo's default. All 47 tests pass. Documentation: README updated with sections 4 and 5 covering the new options, decision prompts, ramp progression example, and updated pit-of-success traps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rkargMsft
force-pushed
the
feat/reminder-concurrency-control
branch
from
June 17, 2026 18:52
f534e0c to
2df3ce4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces an opt-in reminder delivery admission pipeline for Orleans reminders to protect the silo from bursty reminder work during overload, recovery, and ownership changes. The goal is to add explicit, configurable backpressure around reminder dispatch so reminder ticks do not overwhelm the system when demand spikes.
What this change does
WaitUpTo(...)timeout, later gates only get the remaining timeConfiguration
These settings are opt-in and existing reminder behavior is preserved if nothing is configured.
Configuration notes
Intended structure
The implementation is organized as a small layer above the existing reminder service:
This keeps the current per-silo feature working while making the implementation easier to extend later with additional scopes such as global, grain-specific, or reminder-name-specific limiters.
Why
Reminder delivery can become bursty, especially when many reminders become due at once. This change gives the reminder pipeline a way to absorb and shape that load before it reaches grain execution, making overload behavior more predictable and more explicitly configurable.