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

Fixed the multithreading issue which is happening when nrOfThread is mor... #64

Merged
merged 2 commits into from
Feb 12, 2015

Conversation

raunak-a
Copy link

...e than 1. Refer #60

@raunak-a
Copy link
Author

Hi @kristofa

Please find the pull request for the fix: https://github.com/kristofa/brave/pull/64/files

Let me know if you find any issues with it.

Thanks,
Raunak Agrawal

for (int i = 1; i <= params.getNrOfThreads(); i++) {

//Creating a client provider for every spanProcessingThread.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good if you could extract the client creation in a separate private method. This would make the constructor easier to read.

@kristofa
Copy link
Member

Thanks @raun007, if you can have a look at my remark. Once that's done I'll merge it.

@raunak-a
Copy link
Author

Hi @kristofa,

I have made the change as suggested by you. Have a look.

@kristofa
Copy link
Member

Thanks, merging.

kristofa added a commit that referenced this pull request Feb 12, 2015
Fixed bug in ZipkinSpanCollector when nr of threads is > 1.
@kristofa kristofa merged commit 4249286 into openzipkin:master Feb 12, 2015
@raunak-a
Copy link
Author

Thanks @kristofa

@kristofa
Copy link
Member

@raun007 I made the brave release that contains this pull request. See here for release notes. It should become available in Maven Central shortly.

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