Skip to content

fix: refuse graphs that can exhaust consumer workers - #47

Merged
alice-viola merged 1 commit into
masterfrom
fix/bound-graph-workers
Sep 7, 2026
Merged

fix: refuse graphs that can exhaust consumer workers#47
alice-viola merged 1 commit into
masterfrom
fix/bound-graph-workers

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problema

nodes[].concurrency e GATE_STAGE_CONCURRENCY arrivavano fino a queen-mq come u32 senza alcun limite complessivo. L'SDK crea una Vec con quella capacità e avvia un task/long-poll per worker: un documento minuscolo con concurrency: 4294967295 poteva quindi abortire il processo per OOM prima che Gate riuscisse a rispondere al PUT.

Inoltre il server compilava il piano con i knob reali ma validate_with lo ricompilava con i default della libreria. Di conseguenza una configurazione globale pericolosa non era visibile alla validazione.

Fix

  • introduce MAX_GRAPH_WORKERS = 4096 come limite totale per grafo, sommando tutti gli stage;
  • aggiunge la refusal stabile graph-workers, con conteggio risolto e indicazioni operative;
  • espone validate_plan_with e fa validare al server lo stesso piano che sta per avviare, inclusi fatti del broker e GATE_STAGE_CONCURRENCY;
  • conserva validate/validate_with per i caller esistenti;
  • documenta la nuova regola nel design.

Il limite è volutamente sul totale e non sul singolo nodo: molti valori singolarmente ragionevoli possono comunque moltiplicarsi attraverso path e stage. 4096 worker corrispondono già a circa 4 milioni di item/s usando la stima pessimistica di 1000 item/s per lane; oltre questa scala è più sicuro separare la topologia in più grafi.

Test

  • il confine 4096 è accettato e 4097 viene rifiutato;
  • un override globale 4097 viene rilevato sul piano risolto;
  • cargo test --workspace;
  • cargo clippy -p gate-core -p gate-server --all-targets -- -D warnings;
  • cargo fmt --all -- --check.

Per Alice

La scelta da valutare è la soglia 4096. La proprietà di sicurezza non dipende dal numero specifico: serve un tetto finito prima dell'allocazione. La soglia proposta lascia un margine molto superiore alle topologie e ai rate descritti nel repository, senza reintrodurre il vecchio limite operativo di 16 worker.

@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
alice-viola merged commit de3e5d4 into master Sep 7, 2026
1 check passed
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