-
Notifications
You must be signed in to change notification settings - Fork 1
Fix memory leak #28
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
Fix memory leak #28
Conversation
@cityofships reported it reaching ~30GB |
Debug info: Top 10 objects before patch, approx ~30 scrapes:
Top 10 objects before patch, no scrapes:
Note, no high count for Top 10 objects with patch, approx 30 scrapes:
Top 10 objects, closing the connection after each scrape, approx 30 scrapes:
^ NOTE: this /should/ clean up, but it doesn't |
Every time the collector is scraped, it makes a new connection. The connection is never cleaned up, even if you add a conn.close() after the scrape [1]. This is a likely bug. This patch moves to using the instance connection, which works around the leak and is more efficient. [1] https://docs.openstack.org/openstacksdk/latest/user/connection.html#openstack.connection.Connection.close
9eecc6a
to
3cad01a
Compare
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.
LGTM - the empirical evidence is nice too!
thanks for the review! |
Every time the collector is scraped, it makes a new connection. The connection is never cleaned up, even if you add a conn.close() after the scrape [1]. This is a likely bug.
This patch moves to using the instance connection, which works around the leak and is more efficient.
[1] https://docs.openstack.org/openstacksdk/latest/user/connection.html#openstack.connection.Connection.close