elsa-workflows / elsa-workflows/elsa-foundation

A budget-truncated drain reports Quiesced, so incident strategies resolve while work is still queued

Open
#1,256 0 comments 0 reactions 0 assignees View on GitHub
bug ready-for-human tech-debt
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.