Skip to content

Commit 4c6c42a

Browse files
committed
fix: ensure uniform distribution for random policies
1 parent ff6423e commit 4c6c42a

File tree

1 file changed

+33
-44
lines changed

1 file changed

+33
-44
lines changed

pkg/activator/net/lb_policy.go

Lines changed: 33 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -43,71 +43,60 @@ func randomLBPolicy(_ context.Context, targets []*podTracker) (func(), *podTrack
4343
if len(targets) == 0 {
4444
return noop, nil
4545
}
46-
// Try to find a non-nil tracker with limited retries
47-
for range targets {
48-
idx := rand.Intn(len(targets)) //nolint:gosec
49-
if targets[idx] != nil {
50-
return noop, targets[idx]
46+
47+
// Filter out nil trackers to ensure uniform distribution
48+
validTargets := make([]*podTracker, 0, len(targets))
49+
for _, t := range targets {
50+
if t != nil {
51+
validTargets = append(validTargets, t)
5152
}
5253
}
53-
// All trackers were nil
54-
return noop, nil
54+
55+
if len(validTargets) == 0 {
56+
return noop, nil
57+
}
58+
59+
return noop, validTargets[rand.Intn(len(validTargets))] //nolint:gosec
5560
}
5661

5762
// randomChoice2Policy implements the Power of 2 choices LB algorithm
5863
func randomChoice2Policy(_ context.Context, targets []*podTracker) (func(), *podTracker) {
59-
// Avoid random if possible.
60-
l := len(targets)
61-
// One tracker = no choice.
62-
if l == 1 {
63-
pick := targets[0]
64-
if pick != nil {
65-
pick.increaseWeight()
66-
return pick.decreaseWeight, pick
64+
// Filter out nil trackers first to ensure uniform distribution
65+
validTargets := make([]*podTracker, 0, len(targets))
66+
for _, t := range targets {
67+
if t != nil {
68+
validTargets = append(validTargets, t)
6769
}
68-
return noop, nil
6970
}
70-
r1, r2 := 0, 1
7171

72-
// Two trackers - we know both contestants,
73-
// otherwise pick 2 random unequal integers.
74-
// Attempt this only n/2 times for each podTracker
75-
var pick *podTracker
76-
pickTrys := 0
77-
var alt *podTracker
78-
altTrys := 0
79-
// Skip nil trackers (unhealthy pods removed from rotation)
80-
// TODO: Should we explain why n/2 was chosen as the retry limit?
81-
for pick == nil && pickTrys < len(targets)/2 {
82-
if l > 2 {
83-
r1 = rand.Intn(l) //nolint:gosec // We don't need cryptographic randomness for load balancing
84-
}
85-
pick = targets[r1]
86-
pickTrys++
87-
}
88-
// If we couldn't find a non-nil pick after n/2 tries, fail
89-
if pick == nil {
72+
l := len(validTargets)
73+
if l == 0 {
9074
return noop, nil
9175
}
9276

93-
// Try to find an alternative tracker for comparison
94-
for alt == nil && altTrys < len(targets)/2 {
77+
// One tracker = no choice.
78+
if l == 1 {
79+
pick := validTargets[0]
80+
pick.increaseWeight()
81+
return pick.decreaseWeight, pick
82+
}
83+
84+
// Two trackers - we know both contestants,
85+
// otherwise pick 2 random unequal integers.
86+
r1, r2 := 0, 1
87+
if l > 2 {
88+
r1 = rand.Intn(l) //nolint:gosec // We don't need cryptographic randomness for load balancing
9589
r2 = rand.Intn(l - 1) //nolint:gosec // We don't need cryptographic randomness here.
9690
// shift second half of second rand.Intn down so we're picking
9791
// from range of numbers other than r1.
9892
// i.e. rand.Intn(l-1) range is now from range [0,r1),[r1+1,l).
9993
if r2 >= r1 {
10094
r2++
10195
}
102-
alt = targets[r2]
103-
altTrys++
104-
}
105-
// If we couldn't find an alternative, just use pick
106-
if alt == nil {
107-
pick.increaseWeight()
108-
return pick.decreaseWeight, pick
10996
}
11097

98+
pick, alt := validTargets[r1], validTargets[r2]
99+
111100
// Possible race here, but this policy is for CC=0,
112101
// so fine.
113102
if pick.getWeight() > alt.getWeight() {

0 commit comments

Comments
 (0)