Skip to content
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

Improve KafkaConsumer cleanup #1339

Merged
merged 1 commit into from
Jan 10, 2018
Merged

Improve KafkaConsumer cleanup #1339

merged 1 commit into from
Jan 10, 2018

Conversation

dpkp
Copy link
Owner

@dpkp dpkp commented Jan 10, 2018

After digging into #1331 a bit it I found a few issues:

(1) heartbeat thread is not stopped unless KafkaConsumer is closed explicitly (or python interpreter exits).
(2) most connection objects have a circular reference preventing garbage collection prior to interpreter shutdown
(3) the client object also has a circular reference preventing gc prior to shutdown

This PR should fix these issues by using weakrefs to break circular references and by shutting down local resources like the heartbeat thread during gc via __del__

It appears that there are still circular references preventing gc of KafkaMetrics and possibly SubscriptionState. These do not prevent the consumer, coordinator, client, or connection objects from being cleaned up, so I'm going to leave further fixes for a later PR.

@dpkp
Copy link
Owner Author

dpkp commented Jan 10, 2018

To test that gc is triggered correctly, I add logging statements to the __del__ methods, create a consumer in a repl and then check the logs following del consumer

@dpkp dpkp merged commit 0a74924 into master Jan 10, 2018
@jeffwidman
Copy link
Collaborator

Nice catches!

@jeffwidman jeffwidman deleted the reference_cleanup_close branch January 11, 2018 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants