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

MC-1075 - fix ReliableTopic totalPublishes metric #19642

Merged

Conversation

promanenko
Copy link
Contributor

@promanenko promanenko commented Sep 27, 2021

Previously ReliableTopic stats (published/received messages) were reported only when ReliableTopicProxy was used. No stats were reported If ClientReliableTopicProxy was used. Since ReliableTopic is always backed by a RingBuffer, we can report ReliableTopic stats from the RB operations if the RB name has a prefix "_hz_rb_"

Fixes #19555

Breaking changes (list specific methods/types/messages):

  • none

@promanenko promanenko marked this pull request as draft September 27, 2021 11:35
@hazelcast hazelcast deleted a comment from hz-devops-test Sep 28, 2021
@promanenko promanenko changed the title WIP [MC-1075] - fix ReliableTopic totalPublishes metric MC-1075 - fix ReliableTopic totalPublishes metric Sep 28, 2021
@promanenko promanenko marked this pull request as ready for review September 28, 2021 08:37
@promanenko promanenko requested a review from a team as a code owner September 28, 2021 08:37
@promanenko promanenko added this to the 5.1 milestone Sep 28, 2021
@sancar sancar added All Languages Should Check Used by clients team to track fixes on the java client that should potentially backported to others Source: Internal PR or issue was opened by an employee Type: Defect labels Sep 28, 2021
@sancar sancar added Team: Client Add to Release Notes and removed All Languages Should Check Used by clients team to track fixes on the java client that should potentially backported to others labels Sep 28, 2021
@sancar
Copy link
Contributor

sancar commented Sep 28, 2021

Looks good. Are we going to backport this one to 4.x/5.0.x series ?

@promanenko
Copy link
Contributor Author

Looks good. Are we going to backport this one to 4.x/5.0.x series ?

I'm not sure we need it for 4.X since it's a minor issue. For 5.0.x probably yes, we have it in scope of MC5.0.1 release.
@emre-aydin please confirm

@emre-aydin
Copy link
Contributor

@promanenko agreed.

@promanenko promanenko merged commit d07cd69 into hazelcast:master Sep 28, 2021
@promanenko promanenko deleted the fix/report-reliable-topic-metrics branch September 28, 2021 12:46
promanenko added a commit to promanenko/hazelcast that referenced this pull request Sep 28, 2021
Previously ReliableTopic stats (published/received messages) were reported only when ReliableTopicProxy was used. No stats were reported If ClientReliableTopicProxy was used. Since ReliableTopic is always backed by a RingBuffer, we can report ReliableTopic stats from the RB operations if the RB name has a prefix "_hz_rb_"

Fixes hazelcast#19555
@rksharma1972
Copy link

rksharma1972 commented Sep 28, 2021

I reported this issue and we have no plans to move to 5.0 for some time. Please consider fixing it in 4.2.2.

Thanks,
Rakesh

@rksharma1972
Copy link

rksharma1972 commented Sep 28, 2021

Also as per Hazelcast docs ReliableTopic Name and RingBuffer Name should be same. But that's not the case either. Hazelcast docs should be updated to include that _hz_rb_ in RingBuffer name to get the correct stats.

Thanks,
Rakesh

@promanenko
Copy link
Contributor Author

I reported this issue and we have no plans to move to 5.0 for some time. Please consider fixing it in 4.2.2.

OK, I will backport it to 4.x line too

Also as per Hazelcast docs ReliableTopic Name and RingBuffer Name should be same. But that's not the case either. >Hazelcast docs should be updated to include that hz_rb in RingBuffer name to get the correct stats.

The documentation is correct, the names of ReliableTopic and RingBuffer must be the same. _hz_rb_ is added under the hood to prevent users accidentally retrieving the ringbuffer.

@rksharma1972
Copy link

rksharma1972 commented Sep 28, 2021 via email

promanenko added a commit to promanenko/hazelcast that referenced this pull request Sep 28, 2021
Previously ReliableTopic stats (published/received messages) were reported only when ReliableTopicProxy was used. No stats were reported If ClientReliableTopicProxy was used. Since ReliableTopic is always backed by a RingBuffer, we can report ReliableTopic stats from the RB operations if the RB name has a prefix "_hz_rb_"

Fixes hazelcast#19555
promanenko added a commit that referenced this pull request Sep 29, 2021
Previously ReliableTopic stats (published/received messages) were reported only when ReliableTopicProxy was used. No stats were reported If ClientReliableTopicProxy was used. Since ReliableTopic is always backed by a RingBuffer, we can report ReliableTopic stats from the RB operations if the RB name has a prefix "_hz_rb_"

Fixes #19555
promanenko added a commit that referenced this pull request Sep 29, 2021
Previously ReliableTopic stats (published/received messages) were reported only when ReliableTopicProxy was used. No stats were reported If ClientReliableTopicProxy was used. Since ReliableTopic is always backed by a RingBuffer, we can report ReliableTopic stats from the RB operations if the RB name has a prefix "_hz_rb_"

Fixes #19555
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.

ReliableTopic totalPublishes metric is always 0
5 participants