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

[improve] [client] support aggregate metrics for partition topic stats #18214

Merged
merged 1 commit into from
Oct 27, 2022

Conversation

rdhabalia
Copy link
Contributor

@rdhabalia rdhabalia commented Oct 26, 2022

Motivation

Right now, Producer client stats returns incorrect rate values for partition topic instead giving aggregate value across all partitions. Right now it always returns 0 but it can be aggregated for partition topic and give to client.

Modifications

Update aggregate rate value for producer stats.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@rdhabalia rdhabalia added this to the 2.11.0 milestone Oct 26, 2022
@rdhabalia rdhabalia self-assigned this Oct 26, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 26, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2022

Codecov Report

Merging #18214 (371a754) into master (6c65ca0) will increase coverage by 15.86%.
The diff coverage is 33.19%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #18214       +/-   ##
=============================================
+ Coverage     34.91%   50.78%   +15.86%     
- Complexity     5707     8723     +3016     
=============================================
  Files           607      612        +5     
  Lines         53396    53579      +183     
  Branches       5712     5742       +30     
=============================================
+ Hits          18644    27208     +8564     
+ Misses        32119    23367     -8752     
- Partials       2633     3004      +371     
Flag Coverage Δ
unittests 50.78% <33.19%> (+15.86%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../main/java/org/apache/pulsar/PulsarStandalone.java 0.00% <0.00%> (ø)
.../apache/pulsar/broker/admin/impl/ClustersBase.java 76.66% <0.00%> (+67.62%) ⬆️
...pache/pulsar/broker/admin/impl/NamespacesBase.java 63.32% <ø> (+48.79%) ⬆️
.../pulsar/broker/service/AbstractBaseDispatcher.java 52.14% <0.00%> (+6.28%) ⬆️
...che/pulsar/broker/service/BacklogQuotaManager.java 12.39% <0.00%> (+2.91%) ⬆️
.../pulsar/broker/service/BrokerServiceException.java 41.66% <0.00%> (+16.66%) ⬆️
...ava/org/apache/pulsar/broker/service/Consumer.java 68.08% <0.00%> (+6.07%) ⬆️
...ava/org/apache/pulsar/broker/service/Producer.java 62.13% <0.00%> (+2.49%) ⬆️
...pulsar/broker/service/PulsarCommandSenderImpl.java 66.66% <0.00%> (ø)
...va/org/apache/pulsar/broker/service/ServerCnx.java 45.85% <0.00%> (+0.51%) ⬆️
... and 219 more

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