Skip to content

fix: let heavy work wait without losing attempts - #211

Merged
yesbert merged 5 commits into
mainfrom
fix/let-heavy-work-wait-without-losing-attempts
Oct 23, 2026
Merged

yesbert merged 5 commits into
mainfrom
fix/let-heavy-work-wait-without-losing-attempts

Conversation

@yesbert

@yesbert yesbert commented Oct 23, 2026

Copy link
Copy Markdown
Owner

What this changes

On the Orleans execution model, a recorded command no longer uses up its attempts by waiting. A burst of heavy commands many times the cluster-wide limit becomes a backlog that is worked off. Only a handler that runs and fails counts against MessageRetry:MaxDeliveryAttempts.

  • Batched renewal. IntentRenewals, one loop per silo registered by AddStrataraAggregateGrains, renews every command the silo holds every third of the grace, in batches of 1,000. The holders are aggregate orders, runners and heavy pools, and the renewal goes through the new ICommandIntentStore.RenewManyAsync. Its default renews one at a time, and the EF store overrides it with one UPDATE per batch. Leases register and release their commands instead of running a timer each. A lease built without the loop, as in the unit tests, keeps its own timer. A failed batch is logged once (117_134).
  • A bounded heavy pool. HeavyWorkOptions.QueuedLimitPerPool (default 256) bounds the commands one pool holds, running and waiting. A hand-over beyond it is refused, its attempt is given back, and it is logged at debug (117_133). The command waits in the store and is handed over again after the grace.
  • Duplicates give their attempt back. A stamped hand-over that finds its command still held, in the heavy pool, a runner or an aggregate's order, is dropped as before and now calls ReturnAttemptAsync.
  • Docs: A burst of heavy work in the operations guide, a sentence in the concepts page, and the log-event reference.

The first commit archives let-a-consumer-choose-the-pool-size (#209).

Implements the OpenSpec change let-heavy-work-wait-without-losing-attempts (approved). It adds Waiting costs a recorded command no attempt to orleans-execution.

Why

A consumer measured 10,000 documents with four heavy commands each and ClusterWideLimit = 4. Per-lease renewals exhausted the connection pool, the records of waiting commands lapsed, the drain handed them over again, and each hand-over counted an attempt. After 32 minutes 16 commands were kept, 13 of them with no failure recorded.

How it was verified

  • IntentRenewalsTests (new, unit):

    • ten and a thousand held commands each cost one store call;
    • more than a batch is chunked, with a failed chunk logged once;
    • a released command is not renewed;
    • a lease holds its command until it ends.
  • HeavyBacklogTests (new, PostgreSQL):

    • The finding's chain, reproduced. A store decorator fails every renewal for four graces, so the records lapse and the drain hands their commands over again. On the previous code this fails with "a record reached attempt 3 while its command only waited"; with the change, no record exceeds one attempt, none is kept, and all complete once.
    • A burst of 200 commands into a pool of 16, limit 2: all complete once, and attempts never exceed one.
    • A silo stopped mid-burst hands the burst to another without a second attempt.
    • RenewManyAsync renews exactly the listed, not-kept records.
  • Full Stratara.Orleans.IntegrationTests: 213/213.

  • ./scripts/local-gauntlet.sh passes

Copilot AI balanced review requested due to automatic review settings October 23, 2026 11:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@yesbert
yesbert force-pushed the fix/let-heavy-work-wait-without-losing-attempts branch from fbec483 to 5bdd0ce Compare October 23, 2026 13:35
@yesbert

yesbert commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

Copilot could not review this PR because the quota is exhausted, so a local review at high stood in for it. The findings, and how each was handled in 5bdd0ce:

  1. The renewal loop stopped with the host, before the silo, so a handler still running during a stop lost its renewals. Fixed: the loop starts with the first hold and stops only when the container disposes it. It is no longer a hosted service.
  2. Holds were not counted, so of two leases of one intent the first to end unregistered the other. Fixed: holds are reference-counted (A_command_held_twice_stays_renewed_until_both_holds_are_released).
  3. A duplicate's give-back could erase the only counted attempt of two claims stamped in one millisecond. Fixed at the root, together with 4 and 5. The claim no longer counts an attempt. TryRenewFromAsync, the fencing renewal with which a silo takes a resumed hand-over over, counts it where it moves the stamp. Refused and duplicate hand-overs count nothing, the duplicate give-backs are gone, and only a first hand-over that the pool refuses gives back its record's attempt. ICommandIntentStore documents the new meaning, and a store that still counts at the claim stays bounded. IntentClaimTests and the counting helper of ResumedOnceInOrderTests follow the new rule. Recorded as D3 in design.md, approved by the owner.
  4. Every refused hand-over cost a claim plus a give-back. Fixed by 3: a refused resumed hand-over now costs only its claim.
  5. Altitude: count when the handler starts instead of compensating at five call sites. Done as in 3. The count is taken when a silo takes the command over. Within an aggregate's order or the heavy pool that is acceptance, so waiting after acceptance costs nothing more.
  6. A failed give-back was swallowed silently. Fixed: it is logged as 117_135 (AttemptGiveBackFailed, Warning).
  7. A QueuedLimitPerPool below what a pool runs at once silently capped heavy work. Fixed: it fails the start (OrleansOptionsValidatorTests).
  8. A timeout inside one batch aborted the other batches silently. Fixed: only the loop's own stop ends the tick, and any other failure is logged per batch.
  9. A lease held before the loop started, or after it stopped, was never renewed. Fixed by 1.
  10. The lease keeps a second renewal path for host-free use. Kept on purpose: the lease unit tests and host-free uses build leases without the loop.

The finding's chain is still reproduced by HeavyBacklogTests.Records_that_lapse…, which fails on the previous code and passes here. Unit tests 343/343, the gauntlet passes, and the full Stratara.Orleans.IntegrationTests run gave 212/213 before the rebase. The one failure is HardKillTimerTests, the StackExchange.Redis 3.1.31 crash in RESPite.Buffers.CycleBuffer.Commit fixed by the dependency update in #210. A full run on the rebased branch, which now carries 3.4.5, is in progress.

@yesbert
yesbert merged commit e6af2a2 into main Oct 23, 2026
6 checks passed
@yesbert
yesbert deleted the fix/let-heavy-work-wait-without-losing-attempts branch October 23, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants