Bug in LeastConnSelection Policy used in reverseproxy

[yashkundu]

There is a BUG in the way LeastConnSelection policy is selecting an upstream for the reverse proxy.
The following is the code of the select method of LeastConnSelection struct which chooses the upstream:
caddy/blob/master/modules/caddyhttp/reverseproxy/selectionpolicies.go

// Select selects the up host with the least number of connections in the
// pool. If more than one host has the same least number of connections,
// one of the hosts is chosen at random.
func (LeastConnSelection) Select(pool UpstreamPool, _ *http.Request, _ http.ResponseWriter) *Upstream {
	var bestHost *Upstream
	var count int
	leastReqs := -1

	for _, host := range pool {
		if !host.Available() {
			continue
		}
		numReqs := host.NumRequests()
		if leastReqs == -1 || numReqs < leastReqs {
			leastReqs = numReqs
			count = 0
		}

		// among hosts with same least connections, perform a reservoir
		// sample: https://en.wikipedia.org/wiki/Reservoir_sampling
		if numReqs == leastReqs {
			count++
			if count > 1 || (weakrand.Int()%count) == 0 { //nolint:gosec
				bestHost = host
			}
		}
	}

	return bestHost
}

This always selects the last upstream which has the least number of active requests, but it should randomly choose among all the upstreams which have the least number of active requests, reservoir sampling is implemented incorrecly, because of the extra condition (count>1) in the if clause.

[francislavoie]

Oh I think that was meant to be an && and not an ||. The point of the change in b19946f to add that clause is that x % 1 == 0 is always true so there's no use in generating a random number in that case. Minor optimization.

[yashkundu]

I think we can use the condition:

if (count == 1) || (weakrand.Int()%count)

It will handle the condition: if the count is 1, random number won't be generated, otherwise will be generated. @francislavoie

[mholt]

Does this still need to be fixed, @yashkundu ?

[yashkundu]

@mholt , yes, It's a small fix, I can fix it and raise a pr?

[mholt]

That would be splendid 🙏 thank you!

[francislavoie]

FYI @yashkundu #5862