Skip to content

fix(budgets): keep conditional limits out of node-wide aggregates - #35

Closed
albertovincenzi wants to merge 2 commits into
fix/require-unconditional-node-budgetfrom
fix/node-wide-budget-aggregates
Closed

fix(budgets): keep conditional limits out of node-wide aggregates#35
albertovincenzi wants to merge 2 commits into
fix/require-unconditional-node-budgetfrom
fix/node-wide-budget-aggregates

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problem

whenOp budgets apply only to selected messages, but four node-wide calculations treated every unscoped budget as a rate every item meets:

  • claim sizing could shrink every batch to a rare operation's allowance;
  • worker derivation could under-provision unrelated traffic (for example, one photo.delete per hour reduced a 4,000/s node from four lanes to one);
  • ETA applied a selective counter to the entire queue depth and could return a confidently late, false bound;
  • the history flow chart divided all node admissions by a selective ceiling, producing impossible utilisation.

Enforcement itself already evaluates whenOp per message and remains correct.

Fix

  • add one explicit NodePlan::node_wide() definition: unscoped budgets without whenOp;
  • use it for batch sizing, worker derivation, ETA binding and aggregate flow-chart ceilings;
  • keep conditional budgets in the relay, detailed budget views and breaker operations;
  • add an ETA caveat naming selectors that a queue-level depth cannot resolve;
  • add regressions for batch size, worker count and ETA binding;
  • update the corresponding runtime/design documentation.

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 -- --check
  • cargo test --workspace
  • cargo clippy --workspace --all-targets -- -D warnings

albertovincenzi and others added 2 commits September 5, 2026 00:20
…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
@alice-viola

Copy link
Copy Markdown
Contributor

Landed on master via #67 (merge commit 944ee9b) as part of the 62-PR integration — this PR's head commit 1d32e9e is an ancestor of master. GitHub could not mark it merged automatically because its base is fix/require-unconditional-node-budget, not master. Closing as landed.

@alice-viola alice-viola closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants