Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[improve][broker] PIP-192 Implemented ExtensibleLoadManagerWrapper.getLoadBalancingMetrics() #19440

Merged
merged 3 commits into from
Feb 11, 2023

Conversation

heesung-sn
Copy link
Contributor

Master Issue: #16691

Motivation

We will start raising PRs to implement PIP-192, #16691

Modifications

This PR implemented ExtensibleLoadManagerWrapper.getLoadBalancingMetrics()and its unit test.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • *Added unit tests.

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

We will have separate PRs to update the Doc later.

Matching PR in forked repository

PR in forked repository: heesung-sn#28

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 7, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2023

Codecov Report

Merging #19440 (624ccd7) into master (aa7af10) will increase coverage by 0.13%.
The diff coverage is 4.86%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19440      +/-   ##
============================================
+ Coverage     64.20%   64.34%   +0.13%     
- Complexity    26265    26409     +144     
============================================
  Files          1832     1836       +4     
  Lines        134067   134570     +503     
  Branches      14753    14816      +63     
============================================
+ Hits          86077    86583     +506     
- Misses        40135    40156      +21     
+ Partials       7855     7831      -24     
Flag Coverage Δ
inttests 24.85% <0.81%> (-0.07%) ⬇️
systests 25.48% <1.08%> (+<0.01%) ⬆️
unittests 61.62% <4.86%> (+0.16%) ⬆️

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

Impacted Files Coverage Δ
...dbalance/extensions/ExtensibleLoadManagerImpl.java 5.88% <0.00%> (-1.60%) ⬇️
...lance/extensions/ExtensibleLoadManagerWrapper.java 0.00% <0.00%> (ø)
...xtensions/channel/ServiceUnitStateChannelImpl.java 0.73% <0.00%> (-0.24%) ⬇️
...er/loadbalance/extensions/data/BrokerLoadData.java 0.00% <0.00%> (ø)
...r/loadbalance/extensions/models/AssignCounter.java 0.00% <0.00%> (ø)
...er/loadbalance/extensions/models/SplitCounter.java 0.00% <0.00%> (ø)
...r/loadbalance/extensions/models/SplitDecision.java 0.00% <0.00%> (ø)
...r/loadbalance/extensions/models/UnloadCounter.java 0.00% <0.00%> (ø)
.../loadbalance/extensions/models/UnloadDecision.java 0.00% <ø> (ø)
...rg/apache/pulsar/broker/service/BrokerService.java 60.59% <0.00%> (+0.21%) ⬆️
... and 132 more

Copy link
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

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

LGTM

@codelipenghui codelipenghui merged commit 5c8f929 into apache:master Feb 11, 2023
@heesung-sn heesung-sn changed the title [improve][broker] Implemented ExtensibleLoadManagerWrapper.getLoadBalancingMetrics() [improve][broker] PIP-192 Implemented ExtensibleLoadManagerWrapper.getLoadBalancingMetrics() Feb 12, 2023
@heesung-sn heesung-sn deleted the pip-192-metrics branch April 2, 2024 17:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants