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

Performance Regression on Master #732

Closed
nemosupremo opened this issue Aug 23, 2016 · 1 comment · Fixed by #733
Closed

Performance Regression on Master #732

nemosupremo opened this issue Aug 23, 2016 · 1 comment · Fixed by #733

Comments

@nemosupremo
Copy link
Contributor

Versions

Sarama Version: 4f47ee4
Kafka Version: 0.8.2.1
Go Version: 1.6

Problem Description

We build off Sarama's master for various reasons that were needed at one point but probably not now. Everything has been smooth up until a week ago, where performance seemed to fall off a cliff for us (from 60k msg/s to 3k msg/s).

I began to notice it started to take almost exactly 100ms for us to retrieve a new message off the Consumer channel (which is a problem for us because it takes ~3ms for us to process a message). Since it took 100ms, I guessed the problem was with MaxProcessingTime, and seeing how a recent change included this value, we reverted to an earlier commit and that fixed it.

I traced it back to this commit: 906ed72 so there seems to be a logic issue in how this was implemented.

@nemosupremo
Copy link
Contributor Author

nemosupremo commented Aug 23, 2016

Looks like we are hitting a race condition on Reset:

From the golang docs on Reset:

Note that it is not possible to use Reset's return value correctly, as there is a race condition between draining the channel and the new timer expiring. Reset should always be used in concert with Stop, as described above. The return value exists to preserve compatibility with existing programs.

nemosupremo added a commit to ChannelMeter/sarama that referenced this issue Aug 23, 2016
…cs. Docs advice to call Stop inorder to reuse a timer. Should fix IBM#732.
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 a pull request may close this issue.

1 participant