Skip to content

Commit

Permalink
[release-1.10] RandomChoice 2 policy wasn't random when the number of…
Browse files Browse the repository at this point in the history
… targets is 2 (with equal weight) (#14052)

* RandomChoice 2 policy wasn't random when the number of targets is 2

* fix linting

---------

Co-authored-by: dprotaso <[email protected]>
  • Loading branch information
knative-prow-robot and dprotaso authored Jun 1, 2023
1 parent cdac06e commit 500756c
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/activator/net/lb_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ func randomChoice2Policy(_ context.Context, targets []*podTracker) (func(), *pod
// so fine.
if pick.getWeight() > alt.getWeight() {
pick = alt
} else if pick.getWeight() == alt.getWeight() {
//nolint:gosec // We don't need cryptographic randomness here.
if rand.Int63()%2 == 0 {
pick = alt
}
}
pick.increaseWeight()
return pick.decreaseWeight, pick
Expand Down
26 changes: 26 additions & 0 deletions pkg/activator/net/lb_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,32 @@ import (
"knative.dev/serving/pkg/queue"
)

func TestRandomChoice_TwoTrackersDistribution(t *testing.T) {
podTrackers := makeTrackers(2, 0)
counts := map[string]int{}

total := 100
for i := 0; i < total; i++ {
cb, pt := randomChoice2Policy(context.Background(), podTrackers)
cb()
counts[pt.dest]++
}

first := counts[podTrackers[0].dest]
second := counts[podTrackers[1].dest]

// probability of this occurring is 0.5^100
if first == 0 {
t.Error("expected the first tracker to get some requests")
}
if second == 0 {
t.Error("expected the second tracker to get some requests")
}
if first+second != total {
t.Error("expected total requests to equal 100 - was ", first+second)
}
}

func TestRandomChoice2(t *testing.T) {
t.Run("1 tracker", func(t *testing.T) {
podTrackers := makeTrackers(1, 0)
Expand Down

0 comments on commit 500756c

Please sign in to comment.