-
Notifications
You must be signed in to change notification settings - Fork 102
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
Increase scaler stream interval to 200ms #745
Conversation
Signed-off-by: Tommy Chen <tommy351@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice improvement! The resources saving is awesome, but I'm afraid about if this can impact in current applications, I don't think so because 5 checks per second sounds enough, but IDK if it makes sense adding a parameter for setting this value from users side (with a default value).
WDYT? @t0rr3sp3dr0 @tomkerkhove
I like the idea of making this configurable and also think that 200ms is a good default value for it. |
@tommy351 , could you add a parameter for that value (and document it) ? 🙏 |
Hi @tommy351 |
Hello @tommy351 |
Hi @JorTurFer, is the parameter you mentioned application-wide or just for a single |
I think that we can start with a global parameter because the improvement will be huge. If someone has a use case where it's required at HTTPScaledObject, we can just iterate. |
I add the config value in another PR because we want to cut a release tomorrow |
Currently the scaler uses a lot of memory when there're a lot of
HTTPScaledObject
s (We have 300+). Based on the heap snapshots, it seems that theStreamIsActive
handler consumes most of memory. I adjusted the interval from 5ms to 200ms to fix this issue.Part of heap graph, you can open heap snapshot files below for details.
Heap snapshots:
Memory usage:
Before:
After:
Checklist
README.md
docs/
directory