Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support least connections #98

Merged
merged 9 commits into from
Dec 13, 2023

Conversation

jiuker
Copy link
Contributor

@jiuker jiuker commented Dec 5, 2023

support least connections

support least connections
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@klauspost
Copy link

klauspost commented Dec 6, 2023

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
Copy link
Contributor Author

jiuker commented Dec 6, 2023

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

@jiuker jiuker requested a review from klauspost December 6, 2023 11:05
main.go Outdated Show resolved Hide resolved
@jiuker jiuker requested a review from klauspost December 6, 2023 11:14
@harshavardhana
Copy link
Member

@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

main.go Outdated Show resolved Hide resolved
@klauspost
Copy link

klauspost commented Dec 6, 2023

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.

main.go Outdated Show resolved Hide resolved
@jiuker
Copy link
Contributor Author

jiuker commented Dec 6, 2023

it is by far better overall

1 and 2 are there.
3. 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

main.go Outdated Show resolved Hide resolved
Co-authored-by: Harshavardhana <harsha@minio.io>
@jiuker
Copy link
Contributor Author

jiuker commented Dec 8, 2023

@harshavardhana cc

@harshavardhana harshavardhana merged commit 2cf91ba into minio:master Dec 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants