-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add control of the configuration refresh interval #1800
Add control of the configuration refresh interval #1800
Conversation
Coverage increased (+0.1%) to 36.7% when pulling 43dc5b9e9acc42339bad9487295869ec99899a58 on maxlaverse:configurable_refresh_interval into 6816630 on kubernetes:master. |
@maxlaverse just in case this just limit the rate of the execution of the sync function not how often it runs |
43dc5b9
to
5a05865
Compare
5a05865
to
0561ea8
Compare
Sorry @aledbf , my description is confusing. But the PRs still achieve what I'm looking for. This is the upper limit for the frequency of syncs. With a current value of 0.3 it means dead backends are removed from 0s to 3s after there are actually declared dead. Therefore we told all our teams to add a preStop hook with at least 4 seconds, to have a high probabilty for the dead backend to have been removed from the Nginx Ingress and not receive any traffic anymore, before there are being stopped. This value changed twice in the past iirc. We'd like to have this configurable as it's tightly coupled to all the preStop delay we have and if it was decided that this value should change one day, we wouldn't be able to upgrade the Nginx Ingress Controller without updating all the preStop hooks or running a fork. |
Is there a command to re-trigger the tests ? |
Correct
Please understand that this flag could end affecting the uptime of nginx. Changing this flag to something like 0.1 you could end reloading nginx once per second and that means you could end with lots of worker processes (in the case of pending requests) |
Yes thanks, I'm aware of that :) |
/lgtm |
@maxlaverse thanks! |
What this PR does / why we need it:
Make the syncRateLimiter configurable, and therefore how the often the configuration might be reloaded. This value is tightly coupled with the all the upstream pods lifecycle and therefore should be configurable.