Skip to content

fix(breaker): reject nodes wider than one atomic hold - #42

Closed
albertovincenzi wants to merge 1 commit into
fix/atomic-breaker-tripfrom
fix/bound-breaker-width
Closed

fix(breaker): reject nodes wider than one atomic hold#42
albertovincenzi wants to merge 1 commit into
fix/atomic-breaker-tripfrom
fix/bound-breaker-width

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Perché

Queen limita un batch KV HTTP a 256 operazioni. Il breaker deve scrivere tutti i counter node-wide e il record di audit nella stessa transazione (la garanzia introdotta dalla PR #27), quindi può gestire al massimo 255 chiavi distinte. Finora una dichiarazione con 256+ counter validava correttamente, ma POST .../backoff e il reset fallivano solo a runtime con kv_too_many_ops, lasciando il vendor throttle non applicato.

Fix

  • nuova validazione breaker-width sopra 255 chiavi unscoped distinte;
  • il conteggio usa le chiavi compilate: budget con lo stesso sharedKey coerente contano una volta;
  • test espliciti sui boundary 255 (accettato) e 256 (rifiutato);
  • guardia runtime per documenti legacy già persistiti, con risposta 422 descrittiva sia su trip sia su reset;
  • documentazione della regola nel design.

Verifica

  • cargo test --workspace;
  • cargo clippy --workspace --all-targets -- -D warnings;
  • cargo fmt --all e git diff --check.

Dipendenza / nota per Alice

Questa PR è intenzionalmente basata su #27: spezzare il breaker in più batch eliminerebbe il limite ma reintrodurrebbe il bug più grave, cioè un hold senza record (o viceversa). 255 counter + 1 record riempiono esattamente il limite atomico del broker.

@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

@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 9975913 is an ancestor of master. GitHub could not mark it merged automatically because its base is fix/atomic-breaker-trip, 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