Due workflow runs can starve behind twenty older runs that are backing off
- Dominant language
- C#
- Stars
- 6
- Forks
- 7
- Avg merge
- 4h 42m
- Merged PRs (30d)
- 307
Description
`WorkflowRunner.RunOnceAsync` selects candidates by age and status only, then decides due-ness afterwards in `NextDue`:
```csharp
var due = await query.Query()
.Where(r => r.Status == RunStatus.Pending || r.Status == RunStatus.Running)
.OrderBy(r => r.CreatedAt)
.Take(20)
.ToListAsync(ct);
```
`NextDue` then returns null for an attempt whose `NextAttemptAt` is in the future, and null for one another node holds under a live lease. If all twenty of the oldest candidates are in that state, the pass does nothing for the tenant and moves on.
A run that became due one second ago is never looked at, because it is twenty-first by `CreatedAt`. It is reachable whenever a tenant accumulates twenty runs in backoff, and `WorkflowRetryPolicy.Backoff` reaches ten minutes plus jitter, so a provider outage produces exactly that. On a multi-node deployment the twenty slots also fill with attempts other nodes hold, which has the same effect with no failures involved at all.
### What to change
Push due-ness into the query instead of filtering after it. That needs a run-level next-due timestamp on `WorkflowRun`, maintained by `Recompute`, so it can be indexed alongside the existing `Status` and `CreatedAt` indexes in `ServiceCollectionExtensions`.
### Related, same loop: tenants are not treated fairly
`PartitionsWithWorkAsync` returns tenants in whatever order Postgres yields, the loop visits them in that order, and `RunOnceAsync` returns on the first successful claim. A tenant that always has work and sorts early can therefore crowd out every tenant after it indefinitely. A rotating start offset across passes is enough, and it is worth doing in the same change since it is the same fifteen lines.
### How it should be tested
Construct the case where broken and fixed differ: queue more than twenty runs in one tenant with future `NextAttemptAt`, queue one that is due now, run a pass, assert the due one ran. The existing tests drain to completion and never build that state, so they pass either way.
### Where I checked
`barakoCMS/Features/Workflows/WorkflowRunner.cs` (`RunOnceAsync`, `NextDue`, `PartitionsWithWorkAsync`), the `WorkflowRun` schema registration in `barakoCMS/Extensions/ServiceCollectionExtensions.cs`. Searched open issues for runner starvation, fairness and noisy neighbour. Nothing open covers either half.
Contributor guide
Research direction
Start in barakoCMS/Features/Workflows/WorkflowRunner.cs at RunOnceAsync, NextDue, and PartitionsWithWorkAsync, then inspect WorkflowRun registration in barakoCMS/Extensions/ServiceCollectionExtensions.cs and the existing workflow tests. Reproduce the more-than-twenty backoff case and add coverage showing the due run executes; verify tenant work is rotated fairly and the new next-due field and indexes are maintained by Recompute.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, postgresql
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100