elsa-workflows / elsa-workflows/elsa-foundation
A budget-truncated drain reports Quiesced, so incident strategies resolve while work is still queued
- Dominant language
- C#
- Stars
- 5
- Forks
- 1
- Avg merge
- 3h 18m
- Merged PRs (30d)
- 261
Description
Found while reviewing PR #1243 (item D1, #1226). Latent under default configuration, reachable by any host that sets a drain work-item budget.
## Problem
A drain truncated by its work-item budget is **indistinguishable from a settled one**, so incident strategies are applied while work is still queued.
The chain:
1. `WorkflowSchedulerDrainer.DrainAsync` loops `while (remaining > 0 && !stoppedOnTerminalStatus)`, with `remaining` seeded from `request.MaxWorkItems ?? int.MaxValue` (`WorkflowSchedulerDrainer.cs:87`). Exhausting the budget exits the loop with items still queued.
2. That exit sets no stop reason, but `RuntimeSchedulerDrainResult`'s constructor coalesces: `StopReason = stopReason ?? InferStopReason(itemSnapshot)` (`RuntimeSchedulerDrain.cs:52`), and `InferStopReason` returns `Quiesced` for an all-completed item set. So a budget-truncated drain reports `Quiesced`.
3. The orchestrator's aggregate seeds `var stopReason = RuntimeSchedulerDrainStopReason.Quiesced` (`WorkflowDrainOrchestrator.cs:282`) and reassigns it only for fault, pause, and outbox-delivery failure. Budget exhaustion is never consulted.
4. **Quiescence is measured by outbox delivery, not queue emptiness.** The cycle breaks on `outboxResult.DeliveredCount == 0`. Undrained scheduler queue items are not outbox items, so a cycle that delivered no new outbox work reports `Quiesced` even with a non-empty scheduler queue.
`IncidentStrategyResolutionDrainObserver` gates on exactly `result.StopReason != RuntimeSchedulerDrainStopReason.Quiesced`. It therefore runs.
## Failure scenario
A host configures `ImmediateWorkflowSchedulerDrainPolicy` with a budget, say `new ImmediateWorkflowSchedulerDrainPolicy(maxWorkItems: 50)`. A workflow records a blocking incident from an activity fault, and still has queued work that would have handled it, a compensating branch or a catch path.
The drain hits 50 items and stops. The result reports `Quiesced`. The strategy observer sees an undecided blocking incident with an `ActivityExecutionId`, resolves the pinned strategy (default `Fault/1`), and commits `FaultWorkflow`. The workflow is now `Faulted`, which is terminal, and the drainer's terminal-status gate stops the remaining queued work from ever running.
The strategy made a final decision on incomplete information. Nothing distinguishes this from a genuine quiescence at the point the decision is taken.
## Why it is latent, not dead
`ImmediateWorkflowSchedulerDrainPolicy`'s constructor is `(int? maxWorkItems = null)` and it is registered by type (`RuntimeCoreServiceCollectionExtensions.cs:278`), so the default path passes null and `remaining` seeds to `int.MaxValue`. Budget exhaustion cannot occur out of the box.
But the parameter is a supported, validated configuration knob: the constructor explicitly rejects values `<= 0`, which means it is intended to be set. Any host that sets it inherits the behaviour above.
## Do
Decide which of these is intended, then make the code say so:
**A. Budget exhaustion is not quiescence.** Add a distinct stop reason (the enum already has the shape for it, `CycleCapExhausted` is the neighbouring concept) and have the drainer report it, so the strategy observer skips a truncated drain and the next drain resolves the incident once the queue genuinely settles. This is the behaviour the strategy observer's quiescence gate appears to intend.
**B. It is deliberate, and the incident decision should not wait on queue depth.** Then document why in `IncidentStrategyResolutionDrainObserver` and in `docs/runtime-fault-behavior.md`, because the current code reads as though it were A.
Option A is the safer default and matches the observer's stated intent. Confirm before implementing.
## Start at
- `src/Elsa/Workflows/Runtime/Services/WorkflowSchedulerDrainer.cs` (`DrainAsync`, the `remaining` bound and the stop-reason computation)
- `src/Elsa/Workflows/Runtime/Core/Models/RuntimeSchedulerDrain.cs` (`InferStopReason`, and the non-nullable `StopReason` coalesce)
- `src/Elsa/Workflows/Runtime/Services/WorkflowDrainOrchestrator.cs` (`DrainSchedulerAndPostCommitWorkAsync`, the `DeliveredCount == 0` break)
- `src/Elsa/Workflows/Runtime/Services/IncidentStrategyResolutionDrainObserver.cs` (the `!= Quiesced` gate)
- `src/Elsa/Workflows/Runtime/Services/ImmediateWorkflowSchedulerDrainPolicy.cs`
## Done when
A budget-truncated drain is distinguishable from a settled one at the observer boundary, or the current behaviour is documented as intended with its reasoning. Either way a test pins it, since this is currently unreachable by default and so nothing would catch a regression.
## Provenance
Surfaced during review of #1243. I originally asserted the opposite consequence, that budget exhaustion leaves a null stop reason and therefore *skips* the strategy observer. That was wrong: `StopReason` is non-nullable and coalesces to `Quiesced`. The worker on that PR checked it, corrected me, and the real behaviour is this one. Recording that here because the wrong version is the intuitive reading of `WorkflowSchedulerDrainer` alone, which is the exact hazard #1226's fault map exists to address.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at src/Elsa/Workflows/Runtime/Services/WorkflowSchedulerDrainer.cs and trace DrainAsync into RuntimeSchedulerDrain.cs, WorkflowDrainOrchestrator.cs, and IncidentStrategyResolutionDrainObserver.cs. Review ImmediateWorkflowSchedulerDrainPolicy.cs and decide whether budget exhaustion needs a distinct stop reason or documentation. Done means the observer boundary distinguishes a truncated drain from quiescence, or the intended behavior is documented, with a regression test pinning it.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100