Skip to content

Conversation

@jai1
Copy link
Contributor

@jai1 jai1 commented Mar 1, 2017

Motivation

C++ Client - Async call for getConsumerStats - as a part of #216

Modifications

Slightly changed the CPP Client flow for getConsumerStats and added support for async call

Result

CPP Client will have the ability to get consumer stats asynchronously

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it better to call the local getConsumerStatsAsync() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can call Consumer-> getConsumerStatsAsync() instead of impl_-> getConsumerStatsAsync
but I saw the code for acknowledge() and unsubscribe() - they both call the async function in the impl_

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the lock here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, to protect brokerConsumerStats_ in case getConsumerStats is called using multiple threads using the same consumer objects.

Copy link
Contributor

Choose a reason for hiding this comment

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

this callback isn't safe as you just dropped the lock. need to copy brokerConsumerStats_ before unlocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@msb-at-yahoo - handled

@merlimat merlimat added this to the 1.17 milestone Mar 3, 2017
@jai1 jai1 force-pushed the AsyncCallForBrokerStats branch from 2ee87b3 to 2618954 Compare March 3, 2017 00:28
@jai1
Copy link
Contributor Author

jai1 commented Mar 21, 2017

Closing the PR - need to think a little more about this - sorry for the unneccessary noise.

@jai1 jai1 closed this Mar 21, 2017
sijie added a commit to sijie/pulsar that referenced this pull request Mar 4, 2018
hrsakai pushed a commit to hrsakai/pulsar that referenced this pull request Dec 10, 2020
* Fix perf-produce cannot be closed

Signed-off-by: xiaolong.ran <rxl@apache.org>

* fix comments

Signed-off-by: xiaolong.ran <rxl@apache.org>

* fix comments

Signed-off-by: xiaolong.ran <rxl@apache.org>

* fix comments

Signed-off-by: xiaolong.ran <rxl@apache.org>
hangc0276 pushed a commit to hangc0276/pulsar that referenced this pull request May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants