minio / sidekick

High Performance HTTP Sidecar Load Balancer
GNU Affero General Public License v3.0
540 stars 82 forks source link

support least connections #98

Closed jiuker closed 9 months ago

jiuker commented 9 months ago

support least connections

klauspost commented 9 months ago

This is fixing #16 - right?

@harshavardhana While I do believe this is a better default than round-robin, I think RR should still be an option, or did you already discuss this?

jiuker commented 9 months ago

This is fixing #16 - right?

@harshavardhana While I do believe this is a better default than round-robin, I think RR should still be an option, or did you already discuss this?

yeah. Fixing #16

harshavardhana commented 9 months ago

@harshavardhana While I do believe this is a better default than round-robin, I think RR should still be an option, or did you already discuss this?

It can be an option, we can take a flag that says --host-balance=leastconn or --host-balance=random

Currently what we support is random not round-robin

klauspost commented 9 months ago

Code LGTM, but some minor things:

A) change "leastconn" to just least B) Default should be least - it is by far better overall. Deals better with requests of varying latency and varying host speed. C) We should never select hosts that are marked offline, since this will return many more errors if a host is down.

jiuker commented 9 months ago

it is by far better overall

1 and 2 are there.

  1. for this:
    
    func (s *site) upBackends() []*Backend {
    var backends []*Backend
    for _, backend := range s.backends {
        if backend.Online() {
            backends = append(backends, backend)
        }
    }
    return backends
    }

func (s site) nextProxy() (Backend, func()) { backends := s.upBackends() if len(backends) == 0 { return nil, func() {} }


@klauspost 
jiuker commented 9 months ago

@harshavardhana cc