Add outbound HTTP action concurrency limits - #2345
Conversation
|
👋 justinkaseman, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
✅ API Diff Results -
|
There was a problem hiding this comment.
Pull request overview
This pull request extends the CRE settings schema to support configurable concurrency limits for outbound HTTP actions handled by the gateway, alongside existing gateway HTTP rate/concurrency controls.
Changes:
- Adds two new CRE settings for outbound HTTP action concurrency limiting (global + per-node).
- Wires the new settings into the settings schema defaults in both TOML and JSON.
- Updates the CRE settings flowchart documentation to include the new settings.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/settings/cresettings/settings.go | Introduces the two new settings in the schema and sets their default values. |
| pkg/settings/cresettings/defaults.toml | Adds TOML default entries for the new concurrency limits. |
| pkg/settings/cresettings/defaults.json | Adds JSON default entries for the new concurrency limits. |
| pkg/settings/cresettings/README.md | Updates the settings flowchart to document the new settings. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "GatewayHTTPActionMtlsRequestRate": "every30s:0", | ||
| "GatewayHTTPActionMtlsConcurrencyLimit": "50", | ||
| "GatewayHTTPActionOutboundConcurrencyLimit": "256", | ||
| "GatewayHTTPActionOutboundPerNodeConcurrencyLimit": "32", |
There was a problem hiding this comment.
We need to validate these against the existing rate limits that we set
I think we have RPS of 100 with a burst of 100 atm; so your per-node concurrency limit is too low I think
There was a problem hiding this comment.
(Best to validate against the traffic we see in prod though)
No description provided.