You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR re-attempts the checkpoint advancement introduced by #11249, which was reverted by #11330. The original optimization could remain disabled for an agent's lifetime after a recoverable failure. The re-attempt establishes explicit recovery, ownership and retention guarantees before advancing checkpoints.
Release gate: cut a release from the reverted state before considering this PR for a later release. The reverted tree is a standalone release state. It restores subscription-based checkpoints while preserving #11268's shutdown-draining and handshake-ownership guarantees. Idle subscriptions can still pin conservative checkpoint progress, with the associated replay tradeoff accepted for that release.
#11330 is merged as 39ea49999e. This branch has now been rebased onto that actual main commit, retaining the explicit reapplication and recovery rework. All 13 reapplication/fix commits are patch-identical to the prepared series. The ordinary PR diff now includes the complete reapplication against the reverted baseline. When the intervening release is published, renew compatibility validation against that released baseline before landing this PR.
Solution
The built-in Event Hubs components cooperate through an internal certified-prefix protocol. Cursors scan unrelated records and retain selected matching records until acknowledged, filtered, or explicitly skipped by delivery-failure policy. Recovery reconciles receiver-owned source positions and staged notifications; registration and handshake ownership remain independent barriers. The minimum subscription prefix, bounded by the last fully accounted read, governs checkpoint publication and Event Hubs eviction.
Unresolved work retries on pump ticks. Certified progress is published before every read-loop capacity check, so sustained nonempty reads refresh checkpoint and purge authority. Same-cursor re-handshakes retain and rewind unsettled selections when older deliveries finish; replacement cursors retain their requested replay positions. Removing the last unresolved subscriber releases a settled registration pin. Sticky rewind misses preserve the unresolved range while the consumer and failure handler receive the error. The shutdown-draining and handshake-ownership guarantees from #11268 are preserved.
Capacity controls new reception. Already-accepted read accounting, read recovery, and registration retries continue at zero capacity, including when a receiver notification handoff or the agent's admission/discovery accounting filled the cache.
Failure policy and bounded retention
StreamPullingAgentOptions.RetryFailedDeliveries defaults to false. After the existing delivery retry budget is exhausted, the built-in Event Hubs path notifies the consumer and failure handler, skips that selected batch, and continues. Those events can be absent from the subscription's output, and checkpointing can advance past them.
Setting the option to true retains the batch for later pump retries and holds checkpointing and reclamation until it is resolved or the subscription is removed. Subscription faulting/removal follows the configured failure handler. Both settings preserve failed-read, incomplete-selection, cache-miss, and handshake replay obligations. Ownership changes defer skip decisions and preserve replay.
Native certified admission reserves one possible new pool buffer per requested record, independently of averaged consumer pressure. The existing cache constructor's defaultMaxAddCount supplies both maximum read size and buffer budget. The native factory permits 1,000 buffers of 1 MiB per partition. Deployment sizing also includes metadata, SDK prefetch, free pooled buffers and partition count. A factory returning the native cache can choose a smaller constructor budget. Age-eligible, certified records release capacity; staged handoffs can finish while new reception is paused. Packing rollback and exception-safe purge cleanup preserve actual buffer ownership, including when observers throw.
Public surface and compatibility
Recovery, cursor progress, filtering and checkpoint capabilities are internal. Publication reuses the released IQueueCache.UpdateDeliveryProgress callback. Public additions remain the opt-in delivery-policy boolean and IPurgeObservable.TryRemoveOldestMessage, whose default calls the existing removal method and returns true. The Event Hubs API diff records an explicit implementation of the existing ICacheDataAdapter.Compare member.
The Orleans 10.3.1 nullable callback/default and existing custom or derived cache, transport, adapter and eviction behavior are preserved. Ordinary receipt providers keep their notification/skip policy. Certified processing is selected for the native Event Hubs composition and stays fixed for the provider run. Normal Release package validation and unchanged 10.3.1 binary implementations/callers pass without compatibility suppressions.
On the actual post-revert baseline 39ea49999e, 314 shared/Functional and 78 Event Hubs/monitor cases pass on each of net8.0 and net10.0. Coverage includes handshake ownership, both failure policies, error notification, registration cleanup, continuous publication, partial records, atomic admission, full-cache recovery, capacity reopening and guarded reclamation. The rebase preserves the streaming implementation, tests and documentation unchanged.
Runtime cost
Already-accounted cursors stay idle while the read boundary is unchanged. Mutation-safe traversals reuse pooled snapshots. Certified bookkeeping is enabled only for checkpoint-capable owners; ordinary grouped delivery tracks node ranges with explicit mappings only for sliced records.
A historical 22-case actual-build BenchmarkDotNet comparison measured the per-read publication/agent repairs against 660fba8e3d. Warm idle allocation remained 208 B/tick at 1, 32 and 256 subscriptions, and every active allocation result was unchanged. Elapsed differences ranged from approximately 8% lower to 7% higher; the one-subscription certified case increased from 22.10 to 23.71 microseconds per 64-input operation. The repaired fixture verified certification through the current completed read, whereas the prior implementation reported the preceding pump's prefix.
Earlier original-base comparisons showed certified processing approximately 6–15% above the successful-delivery baseline, and ordinary receipt cases from 3% faster to 11% slower. These measurements predate the final buffer-cleanup/full-cache-gate changes and subsequent rebases; they are not fresh-head throughput claims.
Measurements use .NET 10.0.12, BenchmarkDotNet 0.15.8, Windows Hyper-V and real agent/SimpleQueueCache code with synchronous local consumers. They exclude Event Hubs pooled-cache admission, SDK/network, remote RPC, serialization and checkpoint-store I/O. VM variation applies. The earlier additional approximately 214 KB/s of warm idle allocation was eliminated.
Shared boundaries remain separate from #10588's positioning work, #10316's adaptive pressure policy and #7464's persisted checkpoint reset. This update does not approve, merge, release, or enable auto-merge on this PR.
The newest successful coverage run tested 39ea499, not current main 598c0d2.
Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities.
The comparison remains report-only while normal line and branch variance is calibrated.
ReubenBond
changed the title
fix(streaming): restore checkpoint progress after retained cursor recovery
feat(streaming)!: use certified checkpoint prefixes
Sep 16, 2026
Queue<T>.Contains makes every normal buffer allocation scan all currently owned buffers, turning repeated allocation into O(n²) work as a cache retains more blocks. Keep a hash set alongside the purge queue (removing entries when buffers are freed) so the idempotence check remains O(1).
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
A nullability mismatch in EventHubAdapterReceiver.UpdateDeliveryProgress vs ICheckpointingQueueCache.UpdateDeliveryProgress will likely surface as warnings-as-errors and should be corrected before merge.
Get a fresh assessment by requesting another Copilot review.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
It introduces broad, behavior-changing streaming runtime contracts (caches, cursors, receiver recovery, Event Hubs eviction/checkpointing) that warrant final human review despite strong test coverage.
GetEvents() casts every payload event to T before applying the slice (Skip(firstEventIndex)). Since the payload is a List, this can throw if any skipped event is not of type T, even though it should be excluded by the slice. It also does unnecessary work for skipped events.
Consider iterating from firstEventIndex and casting only the events which will actually be returned.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
PersistentStreamPullingAgent.ReadFromQueue clears _pendingRead on an empty batch without resetting _pendingReadAdded, which can leave inconsistent state and skip cache admission on the next read.
Get a fresh assessment by requesting another Copilot review.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
There are unhandled exception paths in the new certified-progress rewind logic (RecordDeliveryFailure) and a likely hot-path performance regression in ChronologicalEvictionStrategy which should be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
ChronologicalEvictionStrategy.OnBlockAllocated now calls inUseBuffers.Contains(newBlock) on every allocation. Since inUseBuffers is a Queue, this is O(n) per buffer allocation and can become a significant hot-path cost as the number of in-use buffers grows. Preventing duplicate notifications likely needs a separate O(1) membership structure (eg, a HashSet of buffer Ids) which is updated when buffers are dequeued/disposed in FreePurgedBuffers.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
There is at least one concrete functional issue (handshake retry passes a default GrainId into subscription-attached telemetry) plus a shutdown robustness issue (CloseAsync can mask failures), both of which should be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
A confirmed InvalidCastException risk exists in certified Event Hubs cache capacity calculation when certified mode is forced on with non-chronological eviction strategies.
GetMaxAddCount unconditionally casts evictionStrategy to ChronologicalEvictionStrategy when certifiedDeliveryProgress is true. Since EnableCertifiedDeliveryProgress() can be called even with non-chronological strategies (eg, fault-injection tests), this can crash the receiver with an InvalidCastException. Guard the cast (or validate the strategy when enabling certified mode) so certified mode cannot be enabled into an incompatible eviction strategy silently.
Linear Contains() in OnBlockAllocated makes buffer tracking O(n^2)
OnBlockAllocated uses inUseBuffers.Contains(newBlock) to make allocation notifications idempotent. Since Queue.Contains is O(n) and this runs for every allocated block, the total work becomes O(n^2) as buffer counts grow (up to 1,000 buffers/partition per the new certified budget). Consider tracking allocated buffers with a HashSet (removing entries when buffers are freed) or otherwise making the idempotence check O(1).
Reapply the checkpoint-advancement portion of dotnet#11249 after its dedicated revert (4bf635d). The following commits in dotnet#11269 replace lifetime fallback with certified recovery and preserve the reviewed final implementation. Land the revert first, then rebase this re-attempt onto its actual main merge commit.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
There are confirmed runtime-exception/race risks in the new retry-handshake path and in certified Event Hubs max-add-count computation which need to be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Reapply the checkpoint-advancement portion of dotnet#11249 after its dedicated revert (4bf635d). The following commits in dotnet#11269 replace lifetime fallback with certified recovery and preserve the reviewed final implementation. Land the revert first, then rebase this re-attempt onto its actual main merge commit.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The change set is large and touches core streaming correctness (checkpointing, eviction, and recovery), and at least one concrete correctness issue was found which should be addressed before approval.
public void OnBlockAllocated(FixedSizeBuffer newBlock)
{
if (inUseBuffers.Contains(newBlock)) return;
this.inUseBuffers.Enqueue(newBlock);
//report metrics
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
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.
Purpose and release sequencing
This PR re-attempts the checkpoint advancement introduced by #11249, which was reverted by #11330. The original optimization could remain disabled for an agent's lifetime after a recoverable failure. The re-attempt establishes explicit recovery, ownership and retention guarantees before advancing checkpoints.
Release gate: cut a release from the reverted state before considering this PR for a later release. The reverted tree is a standalone release state. It restores subscription-based checkpoints while preserving #11268's shutdown-draining and handshake-ownership guarantees. Idle subscriptions can still pin conservative checkpoint progress, with the associated replay tradeoff accepted for that release.
#11330 is merged as
39ea49999e. This branch has now been rebased onto that actualmaincommit, retaining the explicit reapplication and recovery rework. All 13 reapplication/fix commits are patch-identical to the prepared series. The ordinary PR diff now includes the complete reapplication against the reverted baseline. When the intervening release is published, renew compatibility validation against that released baseline before landing this PR.Solution
The built-in Event Hubs components cooperate through an internal certified-prefix protocol. Cursors scan unrelated records and retain selected matching records until acknowledged, filtered, or explicitly skipped by delivery-failure policy. Recovery reconciles receiver-owned source positions and staged notifications; registration and handshake ownership remain independent barriers. The minimum subscription prefix, bounded by the last fully accounted read, governs checkpoint publication and Event Hubs eviction.
Unresolved work retries on pump ticks. Certified progress is published before every read-loop capacity check, so sustained nonempty reads refresh checkpoint and purge authority. Same-cursor re-handshakes retain and rewind unsettled selections when older deliveries finish; replacement cursors retain their requested replay positions. Removing the last unresolved subscriber releases a settled registration pin. Sticky rewind misses preserve the unresolved range while the consumer and failure handler receive the error. The shutdown-draining and handshake-ownership guarantees from #11268 are preserved.
Capacity controls new reception. Already-accepted read accounting, read recovery, and registration retries continue at zero capacity, including when a receiver notification handoff or the agent's admission/discovery accounting filled the cache.
Failure policy and bounded retention
StreamPullingAgentOptions.RetryFailedDeliveriesdefaults tofalse. After the existing delivery retry budget is exhausted, the built-in Event Hubs path notifies the consumer and failure handler, skips that selected batch, and continues. Those events can be absent from the subscription's output, and checkpointing can advance past them.Setting the option to
trueretains the batch for later pump retries and holds checkpointing and reclamation until it is resolved or the subscription is removed. Subscription faulting/removal follows the configured failure handler. Both settings preserve failed-read, incomplete-selection, cache-miss, and handshake replay obligations. Ownership changes defer skip decisions and preserve replay.Native certified admission reserves one possible new pool buffer per requested record, independently of averaged consumer pressure. The existing cache constructor's
defaultMaxAddCountsupplies both maximum read size and buffer budget. The native factory permits 1,000 buffers of 1 MiB per partition. Deployment sizing also includes metadata, SDK prefetch, free pooled buffers and partition count. A factory returning the native cache can choose a smaller constructor budget. Age-eligible, certified records release capacity; staged handoffs can finish while new reception is paused. Packing rollback and exception-safe purge cleanup preserve actual buffer ownership, including when observers throw.Public surface and compatibility
Recovery, cursor progress, filtering and checkpoint capabilities are internal. Publication reuses the released
IQueueCache.UpdateDeliveryProgresscallback. Public additions remain the opt-in delivery-policy boolean andIPurgeObservable.TryRemoveOldestMessage, whose default calls the existing removal method and returnstrue. The Event Hubs API diff records an explicit implementation of the existingICacheDataAdapter.Comparemember.The Orleans 10.3.1 nullable callback/default and existing custom or derived cache, transport, adapter and eviction behavior are preserved. Ordinary receipt providers keep their notification/skip policy. Certified processing is selected for the native Event Hubs composition and stays fixed for the provider run. Normal Release package validation and unchanged 10.3.1 binary implementations/callers pass without compatibility suppressions.
On the actual post-revert baseline
39ea49999e, 314 shared/Functional and 78 Event Hubs/monitor cases pass on each of net8.0 and net10.0. Coverage includes handshake ownership, both failure policies, error notification, registration cleanup, continuous publication, partial records, atomic admission, full-cache recovery, capacity reopening and guarded reclamation. The rebase preserves the streaming implementation, tests and documentation unchanged.Runtime cost
Already-accounted cursors stay idle while the read boundary is unchanged. Mutation-safe traversals reuse pooled snapshots. Certified bookkeeping is enabled only for checkpoint-capable owners; ordinary grouped delivery tracks node ranges with explicit mappings only for sliced records.
A historical 22-case actual-build BenchmarkDotNet comparison measured the per-read publication/agent repairs against
660fba8e3d. Warm idle allocation remained 208 B/tick at 1, 32 and 256 subscriptions, and every active allocation result was unchanged. Elapsed differences ranged from approximately 8% lower to 7% higher; the one-subscription certified case increased from 22.10 to 23.71 microseconds per 64-input operation. The repaired fixture verified certification through the current completed read, whereas the prior implementation reported the preceding pump's prefix.Earlier original-base comparisons showed certified processing approximately 6–15% above the successful-delivery baseline, and ordinary receipt cases from 3% faster to 11% slower. These measurements predate the final buffer-cleanup/full-cache-gate changes and subsequent rebases; they are not fresh-head throughput claims.
Measurements use .NET 10.0.12, BenchmarkDotNet 0.15.8, Windows Hyper-V and real agent/SimpleQueueCache code with synchronous local consumers. They exclude Event Hubs pooled-cache admission, SDK/network, remote RPC, serialization and checkpoint-store I/O. VM variation applies. The earlier additional approximately 214 KB/s of warm idle allocation was eliminated.
Shared boundaries remain separate from #10588's positioning work, #10316's adaptive pressure policy and #7464's persisted checkpoint reset. This update does not approve, merge, release, or enable auto-merge on this PR.