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

producer: bugfix for broker flushers getting stuck #367

Merged
merged 1 commit into from
Mar 18, 2015

Conversation

eapache
Copy link
Contributor

@eapache eapache commented Mar 18, 2015

In certain unusual circumstances, the producer could have added new references
to a flusher that was shutting down, preventing it from shutting down and
causing it to try to produce on a network connection that was already closed.

Track "current" and "active" flushers separately - remove flushers from the
"current" set immediately when they begin shutdown so that nothing else tries to
take a reference, but leave them in "active" so that they can be cleaned up
properly when their reference count hits 0.

Add a test which fails without this fix in place.

@Shopify/kafka

@eapache eapache force-pushed the fix-flusher-references branch 2 times, most recently from 84f13cc to 7214846 Compare March 18, 2015 22:21
In certain unusual circumstances, the producer could have added new references
to a flusher that was shutting down, preventing it from shutting down and
causing it to try to produce on a network connection that was already closed.

Track "current" and "active" flushers separately - remove flushers from the
"current" set immediately when they begin shutdown so that nothing else tries to
take a reference, but leave them in "active" so that they can be cleaned up
properly when their reference count hits 0.

Add a test which fails without this fix in place.
eapache added a commit that referenced this pull request Mar 18, 2015
producer: bugfix for broker flushers getting stuck
@eapache eapache merged commit 83a294c into master Mar 18, 2015
@eapache eapache deleted the fix-flusher-references branch March 18, 2015 22:51
eapache added a commit that referenced this pull request Mar 19, 2015
Very similar to #367 (on the producer): the consumer could have added new
references to a brokerConsumer that was shutting down, leading it to spin with
an error unnecessarily.

Add a test for this case.
eapache added a commit that referenced this pull request Mar 19, 2015
Very similar to #367 (on the producer): the consumer could have added new
references to a brokerConsumer that was shutting down, leading it to spin with
an error unnecessarily.

Add a test for this case.
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.

1 participant