feat(xds): add configurable max receive message size for xDS gRPC server#9564
feat(xds): add configurable max receive message size for xDS gRPC server#9564linmosko wants to merge 4 commits into
Conversation
Signed-off-by: Lin Moskovitch <lin.moskovitch@sap.com>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Lin Moskovitch <lin.moskovitch@sap.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2ef866102
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| if r.EnvoyGateway.XDSServer != nil && r.EnvoyGateway.XDSServer.MaxRecvMsgSize != nil { | ||
| maxRecvMsgSize, _ := r.EnvoyGateway.XDSServer.MaxRecvMsgSize.AsInt64() | ||
| baseKeepaliveOptions = append(baseKeepaliveOptions, grpc.MaxRecvMsgSize(int(maxRecvMsgSize))) |
There was a problem hiding this comment.
Configure the Envoy receive limit for large snapshots
For the documented scale case where the xDS snapshot/DiscoveryResponse is over 4MiB, Envoy Gateway is the sender and the Envoy proxy is the receiver; grpc.MaxRecvMsgSize here only raises the Go server's inbound DiscoveryRequest limit. Setting xdsServer.maxRecvMsgSize therefore leaves the advertised received message larger than max failure on Envoy's receive path unchanged, so large snapshots will still fail unless the Envoy bootstrap/client receive limit is configured instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
DeltaDiscoveryRequest.resource_names_subscribe and DeltaDiscoveryRequest.initial_resource_versions from the XDS Client may grow large when the snapshot contains many resources. This is the concern addressed here. WDYT @codex ?
There was a problem hiding this comment.
Codex Review: Didn't find any major issues. What shall we delve into next?
Reviewed commit: 4f97097f92
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
/retest |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9564 +/- ##
==========================================
+ Coverage 75.58% 75.59% +0.01%
==========================================
Files 252 252
Lines 41743 41755 +12
==========================================
+ Hits 31550 31564 +14
+ Misses 8065 8064 -1
+ Partials 2128 2127 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Lin Moskovitch <lin.moskovitch@sap.com>
|
/retest |
Adds
xdsServer.maxRecvMsgSizeto the EnvoyGateway API, allowing to increase the maximum message size the xDS gRPC server will accept from Envoy proxy.At scale, when the xDS snapshot grows large (e.g. tens of thousands of
clusters/listeners), the default 4MiB gRPC receive limit is exceeded, causing the xDS stream to fail with:
grpc: received message larger than max (4236915 vs. 4194304)PR Checklist
git commit -s). See DCO: Sign your work./api), the API was discussed and agreed before the implementation. The API change can be in a separate PR, or in the same PR, but the API must be agreed before implementation. N/A if this PR does not contain API changes.make generate gen-check,make lint, and the unit-test/coverage build pass. (Flaky e2e failures are not considered breakages, butgen-check,lint, and coverage MUST pass.)release-notes/current/<section>/<pr-number>-<slug>.md(seerelease-notes/current/README.mdfor sections and naming). N/A if this PR does not contain non-trivial changes.make gen-checkand committed the result if API/helm charts/modules changed.release-notes/current/breaking_changes/.