-
Notifications
You must be signed in to change notification settings - Fork 452
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 rate limiting for scrape config updates #2189
Add rate limiting for scrape config updates #2189
Conversation
6a998ce
to
fce41e0
Compare
@swiatekm-sumo were you able to test this in a cluster? I'd love to see some metrics about how this change affected usage. |
I did a basic smoke test, but I didn't try any synthetic stress test where I'd constantly update a bunch of ServiceMonitors and look at TA CPU usage. I can do that and post some numbers if you're interested. |
yeah i'd appreciate that if you don't mind :) |
fce41e0
to
173ad39
Compare
@jaronoff97 Did a very simple benchmark where I updated the labels on a particular ServiceMonitor as fast as I could via kubectl. Pre-change this used 400m worth of CPU, post change, less than 2m. See attached Prometheus graph: |
Co-authored-by: Ben B. <bongartz@klimlive.de>
82dd785
to
ee8b2cb
Compare
* Add rate limiting for scrape config updates * Rename constant to lowercase Co-authored-by: Ben B. <bongartz@klimlive.de> --------- Co-authored-by: Ben B. <bongartz@klimlive.de>
Limit the rate on updates to scrape configs. The idea and code are very similar to what Prometheus' Service Discovery manager does: https://github.com/prometheus/prometheus/blob/79be1b835789d7c3fde2a907003a8799c308733f/discovery/manager.go#L341. The end result is that we emit events about changes to Prometheus CRs at most every 5 seconds.
This should help with #1544