Skip to content

xds/clusterimpl: fix uint32 overflow computing EDS drop ratio#9257

Open
nvxbug wants to merge 2 commits into
grpc:masterfrom
nvxbug:eds-drop-ratio-overflow
Open

xds/clusterimpl: fix uint32 overflow computing EDS drop ratio#9257
nvxbug wants to merge 2 commits into
grpc:masterfrom
nvxbug:eds-drop-ratio-overflow

Conversation

@nvxbug

@nvxbug nvxbug commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

handleClusterConfigLocked turns each EDS drop_overload into a per-million rate with Numerator * million / Denominator in uint32. The numerator comes from the control plane and reaches a million for a 100% drop, so with the million denominator the product overflows a uint32 once the numerator hits 4295 (a 0.43% drop), and the rate then collapses toward zero: a 50% drop computes to 0.18% and a 100% drop to 0.36%. A fraction above 100% instead lands RequestsPerMillion above a million, which underflows million - RequestsPerMillion in newDropper.

Move the computation into dropRequestsPerMillion, which multiplies in uint64 and caps the ratio at a million. Keeping the clamp next to the arithmetic that builds DropConfig leaves the raw numerator and denominator on the parsed resource and applies drops at the configured rate.

RELEASE NOTES:

  • xds: fix xDS EDS drop policies being applied at a much lower rate than configured due to an integer overflow

The EDS drop_overload numerator comes from the control plane and can be as large as a million (a 100% drop). Scaling it with Numerator * million / Denominator in uint32 overflows once the numerator reaches 4295 with the million denominator, so a 0.43% or larger drop collapses toward zero and a fraction above 100% wraps past a million. Do the multiply in uint64 and cap the ratio at a million.
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.10%. Comparing base (75f3c0b) to head (3be127e).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9257      +/-   ##
==========================================
- Coverage   83.23%   83.10%   -0.13%     
==========================================
  Files         421      422       +1     
  Lines       34086    34833     +747     
==========================================
+ Hits        28370    28948     +578     
- Misses       4281     4394     +113     
- Partials     1435     1491      +56     
Files with missing lines Coverage Δ
internal/xds/balancer/clusterimpl/clusterimpl.go 88.46% <100.00%> (+0.20%) ⬆️

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread internal/xds/balancer/clusterimpl/picker.go Outdated
@easwars

easwars commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Moving to @mbissa for second set of eyes

@easwars easwars assigned mbissa and unassigned easwars Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants