Skip to content

Bug in LeastConnSelection Policy used in reverseproxy #5609

@yashkundu

Description

@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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bug 🐞Something isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions