-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Issue-10611] consumer related topic stats only available while consumer or reader are connected #10644
Conversation
…mer or reader are connected
subscriptions.remove(subscriptionName); | ||
PersistentSubscription sub = subscriptions.remove(subscriptionName); | ||
// preserve accumulative stats form removed subscription | ||
SubscriptionStats stats = sub.getStats(false, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it'd be worth adding specific getters for the two counters we want here? I am specifically concerned about the overhead of building out the whole SubscriptionStats
object many times. I'm not sure how expensive the getStats
method really is, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removeSubscription
does not happen that frequently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right. I had been thinking of consumers, not subscriptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
@codelipenghui PTAL |
…mer or reader are connected (apache#10644) Fixes apache#10611
…mer or reader are connected (apache#10644) Fixes apache#10611
(If this PR fixes a github issue, please add
Fixes #<xyz>
.)Fixes #10611
Motivation
Stats report wrong values when consumers unsubscribe.
Modifications
Preserved specific stats for the consumer on its removal.
Verifying this change
(Please pick either of the following options)
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation