fix(budgets): keep conditional limits out of node-wide aggregates - #35
Closed
albertovincenzi wants to merge 2 commits into
Closed
fix(budgets): keep conditional limits out of node-wide aggregates#35albertovincenzi wants to merge 2 commits into
albertovincenzi wants to merge 2 commits into
Conversation
…egates `node_wide` was answering two different questions with one iterator. `node-unscoped-budget` asks whether a counter every item meets exists, and the migration's `PASSTHROUGH_BUDGET_ID` is there precisely to answer it for a v1 node that declared no limit of its own. Scheduling, the ETA and utilisation ask what rate the node HAS — and a million a second is a sentinel, not a rate. `fitting_workers` already made that distinction inline and said why; the three aggregates this change routes through `node_wide` did not, and they run on exactly the nodes that carry the sentinel. The effect was an aggregate that always answers "there is room": an ETA of zero for any backlog under a million, a flow utilisation near zero so a working node reads as idle, and a claim sized against a limit nobody declared. `node_wide_rates` is the second question, and the aggregates read it — including `fitting_workers`, whose inline filter it replaces. Validation keeps `node_wide`, so a migrated document still declares clean. Claude-Session: https://claude.ai/code/session_012K8u7BEJyd6nDNMCQAgH3z
Contributor
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.
Problem
whenOpbudgets apply only to selected messages, but four node-wide calculations treated every unscoped budget as a rate every item meets:photo.deleteper hour reduced a 4,000/s node from four lanes to one);Enforcement itself already evaluates
whenOpper message and remains correct.Fix
NodePlan::node_wide()definition: unscoped budgets withoutwhenOp;Notes for Alice
This PR is stacked on #34 because #34 guarantees the node-wide set is non-empty for valid v2 documents and supplies a passthrough base during v1 migration. Review #34 first; after it merges, this PR can be retargeted to
master.Ignoring a conditional budget in ETA is intentionally conservative in the API's documented direction: the unconditional schedule remains a valid “no earlier than” lower bound, while a selected operation may actually wait longer. The response now says that explicitly in
assumes. Inventing an operation mix from aggregate queue depth would instead risk returning a bound later than the item can really run.Verification
cargo fmt --all -- --checkcargo test --workspacecargo clippy --workspace --all-targets -- -D warnings