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][broker] Avoid object create in RedeliveryTracker.getRedeliveryCount #18352

Merged

Conversation

lifepuzzlefun
Copy link
Contributor

@lifepuzzlefun lifepuzzlefun commented Nov 6, 2022

Fixes #18353

Motivation

PositionImpl object is just use to pass parameter which can be avoid by direct pass.

Modifications

change method signature RedeliveryTracker.getRedeliveryCount

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.

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

…tion)` to `RedeliveryTracker.getRedeliveryCount(long ledgerId, long entryId)`

with this replace we can remove the unnessary PositionImpl object creation.
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 6, 2022
@Technoboy- Technoboy- changed the title [cleanup] avoid object create in RedeliveryTracker.getRedeliveryCount [cleanup][broker] Avoid object create in RedeliveryTracker.getRedeliveryCount Nov 8, 2022
@Technoboy- Technoboy- added this to the 2.12.0 milestone Nov 8, 2022
@Technoboy- Technoboy- added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/broker ready-to-test labels Nov 8, 2022
@Technoboy- Technoboy- changed the title [cleanup][broker] Avoid object create in RedeliveryTracker.getRedeliveryCount [improve][broker] Avoid object create in RedeliveryTracker.getRedeliveryCount Nov 8, 2022
@Technoboy- Technoboy- closed this Nov 8, 2022
@Technoboy- Technoboy- reopened this Nov 8, 2022
@poorbarcode
Copy link
Contributor

@WJL3333 You can trigger CI through close PR then reopen PR

@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2022

Codecov Report

Merging #18352 (d513374) into master (67d9d63) will increase coverage by 6.84%.
The diff coverage is 22.22%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18352      +/-   ##
============================================
+ Coverage     40.29%   47.13%   +6.84%     
- Complexity     8685    10385    +1700     
============================================
  Files           687      692       +5     
  Lines         67441    67765     +324     
  Branches       7225     7258      +33     
============================================
+ Hits          27175    31943    +4768     
+ Misses        37257    32253    -5004     
- Partials       3009     3569     +560     
Flag Coverage Δ
unittests 47.13% <22.22%> (+6.84%) ⬆️

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

Impacted Files Coverage Δ
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java 36.81% <0.00%> (-0.09%) ⬇️
...rg/apache/bookkeeper/mledger/impl/OpReadEntry.java 65.55% <ø> (+0.72%) ⬆️
...lsar/broker/service/RedeliveryTrackerDisabled.java 50.00% <ø> (ø)
.../transaction/TransactionCoordinatorClientImpl.java 0.00% <0.00%> (ø)
...lsar/broker/service/InMemoryRedeliveryTracker.java 94.11% <100.00%> (-0.33%) ⬇️
...pulsar/broker/service/PulsarCommandSenderImpl.java 71.72% <100.00%> (-6.29%) ⬇️
.../pulsar/broker/service/SharedConsumerAssignor.java 3.70% <0.00%> (-74.08%) ⬇️
...apache/pulsar/broker/service/EntryAndMetadata.java 0.00% <0.00%> (-40.75%) ⬇️
.../apache/pulsar/broker/loadbalance/LoadManager.java 61.11% <0.00%> (-16.67%) ⬇️
...tent/NonPersistentDispatcherMultipleConsumers.java 40.74% <0.00%> (-12.35%) ⬇️
... and 219 more

@codelipenghui codelipenghui merged commit 6373180 into apache:master Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker doc-not-needed Your PR changes do not impact docs ready-to-test type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cleanup] avoid object create in RedeliveryTracker.getRedeliveryCount
6 participants