Skip to content

Commit 622a41d

Browse files
authored
xds/clusterimpl: fix uint32 overflow computing EDS drop ratio (grpc#9257)
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
1 parent 216640c commit 622a41d

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

internal/xds/balancer/clusterimpl/balancer_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,3 +327,30 @@ func (s) TestPickerUpdatedSynchronouslyOnConfigUpdate(t *testing.T) {
327327
t.Fatal("Timed out waiting for client conn update to be completed.")
328328
}
329329
}
330+
331+
func (s) TestDropRequestsPerMillion(t *testing.T) {
332+
tests := []struct {
333+
name string
334+
numerator uint32
335+
denominator uint32
336+
want uint32
337+
}{
338+
{name: "zero", numerator: 0, denominator: million, want: 0},
339+
{name: "five percent hundred", numerator: 5, denominator: 100, want: 50000},
340+
// numerator*million overflows a uint32 once numerator reaches 4295 with
341+
// the million denominator, which is only a 0.43% drop.
342+
{name: "point four three percent million", numerator: 4295, denominator: million, want: 4295},
343+
{name: "one percent million", numerator: 10000, denominator: million, want: 10000},
344+
{name: "fifty percent million", numerator: 500000, denominator: million, want: 500000},
345+
{name: "hundred percent million", numerator: million, denominator: million, want: million},
346+
// A fraction above 100% is capped at a million.
347+
{name: "over hundred percent", numerator: 150, denominator: 100, want: million},
348+
}
349+
for _, tt := range tests {
350+
t.Run(tt.name, func(t *testing.T) {
351+
if got := dropRequestsPerMillion(tt.numerator, tt.denominator); got != tt.want {
352+
t.Errorf("dropRequestsPerMillion(%d, %d) = %d, want %d", tt.numerator, tt.denominator, got, tt.want)
353+
}
354+
})
355+
}
356+
}

internal/xds/balancer/clusterimpl/clusterimpl.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,19 @@ type clusterImplBalancer struct {
151151
lrsReportEndpointMetrics *xdsresource.LRSReportEndpointMetricsConfig // LRS metrics to propagate.
152152
}
153153

154+
// dropRequestsPerMillion scales a drop overload's numerator and denominator to
155+
// a number of requests to drop per million. numerator can be as large as a
156+
// million (a 100% drop), so the multiplication is done in uint64 to avoid
157+
// overflowing a uint32, and the result is capped at a million because a drop
158+
// ratio above 100% is treated as 100%.
159+
func dropRequestsPerMillion(numerator, denominator uint32) uint32 {
160+
rpm := uint64(numerator) * million / uint64(denominator)
161+
if rpm > million {
162+
rpm = million
163+
}
164+
return uint32(rpm)
165+
}
166+
154167
// handleClusterConfigLocked updates the internal state of the balancer with the
155168
// new cluster configuration. It returns true if a new picker needs to be
156169
// generated as a result of these changes. It must be called with b.mu held.
@@ -172,7 +185,7 @@ func (b *clusterImplBalancer) handleClusterConfigLocked(clusterConfig xdsresourc
172185
for _, d := range edsUpdate.Drops {
173186
newDrops = append(newDrops, DropConfig{
174187
Category: d.Category,
175-
RequestsPerMillion: d.Numerator * million / d.Denominator,
188+
RequestsPerMillion: dropRequestsPerMillion(d.Numerator, d.Denominator),
176189
})
177190
}
178191
if !slices.Equal(b.dropCategories, newDrops) {

0 commit comments

Comments
 (0)