-
Notifications
You must be signed in to change notification settings - Fork 164
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
unable nginx keepalive to cortex components #330
Conversation
5bad9bf
to
24083b6
Compare
24083b6
to
194b031
Compare
What about load-balancing @jmcarp? Wouldn't the keep-alive blocks cause problems and lead to further uneven load distribution on specifically the distributors? I can already see that happening without this change on my distributors using the default helm-chart "flow" (Internet -> Ingress -> nginx -> distributor) Edit: I just realised this only affects the keep alive connections to the upstream backends, right? So this actually must be a good idea. |
4442c20
to
89bb799
Compare
Signed-off-by: Josh Carp <jm.carp@gmail.com>
Signed-off-by: nschad <niclas.schad@gmail.com>
c73ea6e
to
dfde39c
Compare
Signed-off-by: nschad <niclas.schad@gmail.com>
dfde39c
to
1e4b3f2
Compare
@jmcarp Hey Josh, I did some experiencing and setting keepalive for cortex is actually in my test very problematic as connections are kept and therefore load is distributed unevenly. But the idea with the individual upstreams had/has a very positive effect in my tests as it seems that nginx is now forced to always resolve said upstream server I did some changes to your PR and I hope you don't mind |
Thanks for fixing up the PR @nschad! Just curious, what are you plotting in that graph? I'm actually a little surprised that moving configurations to upstream blocks without keepalives would help, but great if it does. |
This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions. |
Closed in favor of #366 . Thank you so much for contribution! |
Signed-off-by: Josh Carp jm.carp@gmail.com
What this PR does:
Add a small nginx optimization: enable keepalive to cortex components. Note that this requires configuring upstreams with explicit
upstream
blocks. I noticed this in a post from nginx at https://www.nginx.com/blog/avoiding-top-10-nginx-configuration-mistakes/#no-keepalives; I tested it out and got a modest drop in nginx cpu use.Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]