Skip to content

fix(validation): require a node-wide breaker budget - #34

Merged
alice-viola merged 1 commit into
masterfrom
fix/require-unconditional-node-budget
Sep 7, 2026
Merged

fix(validation): require a node-wide breaker budget#34
alice-viola merged 1 commit into
masterfrom
fix/require-unconditional-node-budget

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problem

node-unscoped-budget accepted a budget with no scopeBy even when it had whenOp. Such a counter is not node-wide: a non-matching operation skips it entirely. If every unscoped budget was conditional, those operations touched no counter and could continue through the node after the breaker had spent every declared budget on a vendor 429.

The same shape also gave ETA a conditional rate as though every queued item met it.

Fix

  • require every v2 node to have at least one budget with neither scopeBy nor whenOp, so every item meets a real base counter;
  • keep the existing validation rule name but make its condition and diagnostic precise;
  • teach v1 migration to add the existing passthrough sentinel when a legacy node has only scoped or conditional budgets;
  • document the stronger contract and add focused validation/migration regressions.

Notes for Alice

This intentionally tightens direct v2 declarations: a conditional-only node must add an unconditional base budget. That is the smallest way to uphold the advertised node-wide breaker without adding a KV read to every hot-path batch.

Legacy compatibility is preserved. Real v1 fixtures contain conditional-only nodes, so migration now appends the established 1,000,000/s passthrough budget and emits a warning. This preserves their selective limits and previously-unlimited non-matching traffic while giving the v2 breaker one counter every operation must meet. Operators should replace that sentinel with the vendor's real aggregate limit when one exists.

Verification

  • all three real v1 graph fixtures migrate into documents accepted by this build;
  • cargo fmt --all -- --check
  • cargo test --workspace
  • cargo clippy --workspace --all-targets -- -D warnings

@albertovincenzi

Copy link
Copy Markdown
Collaborator Author

This rule is retroactive on documents that are already in the store.

restore and reconcile both go through declare_from_storedeclare_locked(.., from_caller = false)validate_with, with no exception. A single Problem therefore means the graph is not registered on the next restart: its pushes answer 404 and its ingress queue fills behind one WARN line.

#66 draws the line — a stored document is refused only for nodes, paths or a name that cannot become a queue name and a kv key, and everything else is logged and kept running. Suggest basing this PR on it, or merging it first.

For #34 specifically this is not hypothetical: airbnb/content and airbnb/messages in crates/core/tests/testdata/v1_channel_go_graphs.json carry only conditional budgets, and the passthrough remedy in migrate.rs never runs again once a document has been re-saved in v2 shape.

https://claude.ai/code/session_012K8u7BEJyd6nDNMCQAgH3z

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