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

Least connections algorithm #128

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

karlvr
Copy link
Contributor

@karlvr karlvr commented Nov 13, 2024

Perhaps to reopen the conversation from #71, I have ported just the least connections algorithm to the master branch. We have abandoned the weighted and slow start that complicated the previous implementation. We have been using LEAST in production for > 5 years. We've tried switching back to the default RR implementation but experience issues when one of our backends runs slowly.

As you noted in #71 it isn't a guarantee of least connections (as the connection is not reserved) however we are treating it as a heuristic, and as such it has worked really well for us.

Again, if you're interested, I am happy to work this code to get it into a shape you like. I've tried to reproduce the basic logic from dom_find in dom_find_leastconn without worrying about the alt behaviour as it uses dom_find as a fall-back anyway. I look forward to hearing your thoughts when you have the time & energy!

@nigoroll
Copy link
Owner

nigoroll commented Nov 14, 2024

I really wish we had a way in Varnish-Cache to add backends to directors other than from VCL 1, such that dynamic could be given an instance of another director to use as the next layer. This way, we would not need to reinvent the wheel whenever we want to add another load balancing method, avoiding the otherwise unavoidable path towards "unidirectors" and keeping the separation of concerns...
Maybe I should really bite the bullet and look into how such an implementation could look like, but I guess the main issue will be additional parameters...
Other than that, yes, I get your point and I appreciate bringing the topic up again, thank you.

Footnotes

  1. Also relevant in another context, see "Reverse HTTP" here

@nigoroll nigoroll added enhancement needs ♡Sponsor looking for support ♥ needs varnish-cache needs work in varnish-cache labels Nov 14, 2024
@karlvr
Copy link
Contributor Author

karlvr commented Nov 14, 2024

@nigoroll thank you, and thanks for that explanation. I really clearly understand what you're thinking and I agree... we don't want to reinvent wheels, especially as they'll end up buggy (see my several additional commits!). This sounds like a joyful change to Varnish but well outside of my wheel-house. Can you point me to a spot to look into what supporting this kind of effort looks like?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs ♡Sponsor looking for support ♥ needs varnish-cache needs work in varnish-cache
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants