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

Added async producer mode for kafka #4150

Closed
wants to merge 1 commit into from

Conversation

apollo-3
Copy link

@apollo-3 apollo-3 commented May 15, 2018

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@danielnelson
Copy link
Contributor

Can you tell me a little bit about why the async producer is helpful? When would a user want to enable async?

@apollo-3
Copy link
Author

apollo-3 commented May 15, 2018 via email

@danielnelson danielnelson added performance problems with decreased performance or enhancements that improve performance area/kafka labels May 15, 2018
@danielnelson
Copy link
Contributor

I think the main difference between this method and the current one is that you are not waiting for success or errors. I'd like to stop using the sync producer (which just wraps the async producer) completely, but then we need to have a configurable number of messages in flight and we still need to monitor the errors channel.

Another potential performance improvement would be to use SerializeBatch to place the full batch into a single Kafka message.

@apollo-3
Copy link
Author

apollo-3 commented May 16, 2018 via email

@danielnelson
Copy link
Contributor

I will try to work on this when I have some free time but I can't guarantee when that will be. If you would like to address the issues we may be able to add it sooner. To summarize we need to:

  • Remove SyncProducer, only use AsyncProducer
  • Check errors
  • Investigate if we need to introduce options for configuring in flight messages, message batch sizes.
  • Unittests
  • Bonus: benchmarks

@apollo-3
Copy link
Author

apollo-3 commented Jun 4, 2018 via email

@danielnelson
Copy link
Contributor

@apollo-3 In 1.8.0 we switched to using SendMessages instead of SendMessage, and it improved performance substantially.

#4491

With this change I don't believe we need to switch to the AsyncProducer directly. Can you test out the new release?

Sorry about missing your last comment about not reconnecting, it should be automatic. If you are still having trouble in 1.8.0 can you open a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kafka performance problems with decreased performance or enhancements that improve performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants