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 PersistentReplicator.expireMessages logic compute backlog twice #23957

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

hanmz
Copy link
Contributor

@hanmz hanmz commented Feb 11, 2025

Motivation

Function cursor.getNumberOfEntriesInBacklog(false) maybe called twice in PersistentReplicator.expireMessages.

This PR #20416 optimizes similar problems in PersistentSubscription.expireMessages.

Modifications

Save backlog to variable.

Documentation

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 11, 2025
@lhotari lhotari changed the title [improve] [broker] Avoid PersistentReplicator.expireMessages logic compute backlog twice [improve][broker] Avoid PersistentReplicator.expireMessages logic compute backlog twice Feb 11, 2025
@lhotari lhotari added this to the 4.1.0 milestone Feb 11, 2025
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.21%. Comparing base (bbc6224) to head (9a06489).
Report is 900 commits behind head on master.

Files with missing lines Patch % Lines
...roker/service/persistent/PersistentReplicator.java 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23957      +/-   ##
============================================
+ Coverage     73.57%   74.21%   +0.64%     
+ Complexity    32624    31888     -736     
============================================
  Files          1877     1853      -24     
  Lines        139502   143827    +4325     
  Branches      15299    16344    +1045     
============================================
+ Hits         102638   106741    +4103     
+ Misses        28908    28684     -224     
- Partials       7956     8402     +446     
Flag Coverage Δ
inttests 26.81% <0.00%> (+2.22%) ⬆️
systests 23.23% <0.00%> (-1.10%) ⬇️
unittests 73.73% <50.00%> (+0.88%) ⬆️

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

Files with missing lines Coverage Δ
...roker/service/persistent/PersistentReplicator.java 67.94% <50.00%> (-0.93%) ⬇️

... and 1041 files with indirect coverage changes

@lhotari lhotari merged commit 6db275c into apache:master Feb 11, 2025
59 of 62 checks passed
lhotari pushed a commit that referenced this pull request Feb 11, 2025
lhotari pushed a commit that referenced this pull request Feb 11, 2025
lhotari pushed a commit that referenced this pull request Feb 11, 2025
hanmz added a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Feb 19, 2025
…pute backlog twice (apache#23957)

(cherry picked from commit 6db275c)
(cherry picked from commit 921b75e)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Feb 20, 2025
…pute backlog twice (apache#23957)

(cherry picked from commit 6db275c)
(cherry picked from commit 127ae84)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Feb 24, 2025
…pute backlog twice (apache#23957)

(cherry picked from commit 6db275c)
(cherry picked from commit 921b75e)
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