-
Notifications
You must be signed in to change notification settings - Fork 0
plan(v0.37): file REQ-325 — the stall classifier names the wrong cause - #896
Merged
Merged
plan(v0.37): file REQ-325 — the stall classifier names the wrong cause #896avrabe merged 1 commit into
avrabe merged 1 commit into
Conversation
Found while watching the v0.36.0 release run, in the code REQ-317 shipped a day earlier. `create-release` sat queued behind its `needs` while the compliance build was still running, and `classify_stall` returned `hosted-starved` — because every queued job carried `ubuntu-latest` and the classifier reasons only about labels and capacity. The fleet was almost entirely idle: online=12, busy=1, with spare capacity under every label. Nothing was starved; the job was waiting its turn. That is the same defect REQ-317 exists to fix, in its own fix: a diagnostic confidently naming a cause it cannot observe. It is reachable rather than theoretical — the probe fires on a run queued past thirty minutes, and this release's builds legitimately take that long, so a slow release would have auto-filed an issue blaming GitHub-hosted capacity for a dependency wait. The fixture set had no dependency-blocked case, which is why it passed. A queued job whose `needs` are incomplete is not a stall at all and has to be excluded before any capacity reasoning runs. Scoped to v0.37.0 alongside REQ-323 (fleet-restart suppression, deployed but not yet demonstrated) and REQ-324 (mutants reported in `#[cfg(kani)]` code that no test the gate runs can kill). Refs: REQ-325, REQ-317 Trace: skip
📐 Rivet artifact delta
Graphgraph LR
REQ_325["REQ-325"]:::added
classDef added fill:#d4edda,stroke:#28a745,color:#155724
classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Added
Posted by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 4178947 | Previous: 16bff52 | Ratio |
|---|---|---|---|
traceability_matrix/1000 |
60573 ns/iter (± 523) |
46077 ns/iter (± 202) |
1.31 |
query/10000 |
331986 ns/iter (± 7225) |
244880 ns/iter (± 1851) |
1.36 |
This comment was automatically generated by workflow using github-action-benchmark.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Artifact only — no code change.
Found while watching the v0.36.0 release run, in the code REQ-317 shipped a
day earlier.
create-releasesat queued behind itsneedswhile the compliance build wasstill running.
classify_stallreturnedhosted-starved— because everyqueued job carried
ubuntu-latest, and the classifier reasons only about labelsand runner capacity.
The fleet at that moment:
Nothing was starved. The job was waiting its turn.
Why this is worth an artifact rather than a shrug
It's the same defect REQ-317 exists to fix, in its own fix: a diagnostic
confidently naming a cause it cannot observe.
And it's reachable, not theoretical. The probe fires on a run queued past thirty
minutes, and this release's builds legitimately take that long — so a slow
release would have auto-filed an issue blaming GitHub-hosted capacity for a
dependency wait, sending the next reader exactly where the old alert did.
My fixture set had no dependency-blocked case, which is why it passed. A
classifier that can only see labels and capacity will name capacity as the cause
of every wait it observes.
Proposed fix (not implemented here)
A queued job whose
needsare incomplete is not a stall and must be excludedbefore any capacity reasoning runs. The jobs API doesn't expose
needs, so theworkflow definition has to be read. Add
dependency-blockedas an explicitoutcome rather than folding it into
capacity-available, so the distinction isvisible.
Scoped to v0.37.0 alongside REQ-323 and REQ-324.