@marcel91200 / caddy Mirror · read-only / commits / b2be5482
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 11, 08:30 UTC

reverseproxy: fix broken reservoir sampling in random_choose policy (#7873)

The random_choose selection policy is meant to implement power-of-d-choices: sample d available upstreams uniformly, then pick the least-loaded of that sample. The sampling loop, however, was not a correct reservoir sample (Algorithm R): it never filled the first k reservoir slots unconditionally, instead writing every candidate to a random slot j = rand(i+1), which can evict an earlier candidate while leaving another slot nil. It also derived j from the upstream's index in the pool rather than from the number of available upstreams seen, skewing the sample whenever unavailable upstreams precede available ones. As a result the reservoir frequently held fewer than min(k, available) upstreams, so the least-load comparison often never happened. Most notably, with two upstreams and 'random_choose 2' (the canonical power-of-two-choices setup), half of all requests were routed to the more-loaded upstream even when it was saturated and the other idle -- identical behavior to plain 'random'. The nil slots this leaves behind were also the cause of the panic reported in #3810, which was patched by skipping nils in leastRequests rather than by fixing the sampling. Replace the loop with a standard Algorithm R reservoir sample over the available upstreams: fill the first k slots, then replace a random slot with probability k/n. Every available upstream is now sampled uniformly and the reservoir always holds min(k, available) candidates. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

TowyTowy committed Jul 12, 2026 at 01:25 UTC b2be548275c99cd061ec1c2f9c27eedae64a17b8
2 files changed +41 -3
modules/caddyhttp/reverseproxy/selectionpolicies.go
+14 -3
@@ -233,12 +233,23 @@ func (r RandomChoiceSelection) Validate() error {
233 // Select returns an available host, if any.
234 func (r RandomChoiceSelection) Select(pool UpstreamPool, _ *http.Request, _ http.ResponseWriter) *Upstream {
235 k := min(r.Choose, len(pool))
236 - choices := make([]*Upstream, k)
237 - for i, upstream := range pool {
236 +
237 + // reservoir sampling (Algorithm R) over the available upstreams:
238 + // the first k available upstreams fill the reservoir, then each
239 + // subsequent one replaces a random reservoir entry with probability
240 + // k/n, so every available upstream is sampled uniformly
241 + choices := make([]*Upstream, 0, k)
242 + var available int
243 + for _, upstream := range pool {
244 if !upstream.Available() {
245 continue
246 }
241 - j := weakrand.IntN(i + 1) //nolint:gosec
247 + available++
248 + if len(choices) < k {
249 + choices = append(choices, upstream)
250 + continue
251 + }
252 + j := weakrand.IntN(available) //nolint:gosec
253 if j < k {
254 choices[j] = upstream
255 }
modules/caddyhttp/reverseproxy/selectionpolicies_test.go
+27
@@ -725,6 +725,33 @@ func TestRandomChoicePolicy(t *testing.T) {
725 }
726 }
727
728 +func TestRandomChoicePolicyLeastLoaded(t *testing.T) {
729 + // when the number of available upstreams does not exceed the choose
730 + // count, all of them must be candidates, so the least-loaded one
731 + // must always be selected; the pool intentionally starts with an
732 + // unavailable upstream to verify that reservoir sampling counts
733 + // available upstreams rather than pool indices
734 + pool := testPool()
735 + pool[0].Dial = "localhost:8080"
736 + pool[1].Dial = "localhost:8081"
737 + pool[2].Dial = "localhost:8082"
738 + pool[0].setHealthy(false)
739 + pool[1].setHealthy(true)
740 + pool[2].setHealthy(true)
741 + pool[1].countRequest(30)
742 + // pool[2] has no active requests
743 +
744 + request := httptest.NewRequest(http.MethodGet, "/test", nil)
745 + randomChoicePolicy := RandomChoiceSelection{Choose: 2}
746 +
747 + for i := 0; i < 100; i++ {
748 + h := randomChoicePolicy.Select(pool, request, nil)
749 + if h != pool[2] {
750 + t.Fatalf("with 2 available upstreams and choose=2, the least-loaded upstream (pool[2]) must always be selected; got %v on iteration %d", h, i)
751 + }
752 + }
753 +}
754 +
755 func TestCookieHashPolicy(t *testing.T) {
756 ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()})
757 defer cancel()