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 PersistentSubscription.expireMessages logic check backlog twice. #20416

Merged
merged 1 commit into from May 29, 2023

Conversation

lifepuzzlefun
Copy link
Contributor

@lifepuzzlefun lifepuzzlefun commented May 27, 2023

Motivation

PersistentSubscription.expireMessages will first backlog is zero and if not will check if backlog is below threshold.

those two check should be save in variable to avoid check twice.

Modifications

save backlog to variable.

Documentation

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

Matching PR in forked repository

PR in forked repository:

@lifepuzzlefun lifepuzzlefun changed the title [improv] [broker] Avoid expireMessages logic check backlog twice. [improve] [broker] Avoid expireMessages logic check backlog twice. May 27, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 27, 2023
@lifepuzzlefun lifepuzzlefun changed the title [improve] [broker] Avoid expireMessages logic check backlog twice. [improve] [broker] Avoid PersistentSubscription.expireMessages logic check backlog twice. May 27, 2023
@AnonHxy
Copy link
Contributor

AnonHxy commented May 29, 2023

LGTM

@AnonHxy
Copy link
Contributor

AnonHxy commented May 29, 2023

Please add the link of PR in forked repository, thanks

@codecov-commenter
Copy link

Codecov Report

Merging #20416 (5e1d23f) into master (aa3bfcd) will increase coverage by 0.02%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #20416      +/-   ##
============================================
+ Coverage     72.90%   72.93%   +0.02%     
- Complexity    31864    31897      +33     
============================================
  Files          1864     1864              
  Lines        138416   138416              
  Branches      15188    15188              
============================================
+ Hits         100919   100954      +35     
+ Misses        29477    29458      -19     
+ Partials       8020     8004      -16     
Flag Coverage Δ
inttests 24.26% <0.00%> (+0.09%) ⬆️
systests 24.92% <0.00%> (-0.14%) ⬇️
unittests 72.23% <50.00%> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
...ker/service/persistent/PersistentSubscription.java 75.43% <50.00%> (ø)

... and 75 files with indirect coverage changes

@Technoboy- Technoboy- merged commit fafadee into apache:master May 29, 2023
49 of 53 checks passed
Technoboy- pushed a commit to Technoboy-/pulsar that referenced this pull request May 29, 2023
Technoboy- pushed a commit that referenced this pull request May 30, 2023
@michaeljmarshall
Copy link
Member

As discussed on the mailing list https://lists.apache.org/thread/w4jzk27qhtosgsz7l9bmhf1t7o9mxjhp, there is no plan to release 2.9.6, so I am going to remove the release/2.9.6 label

Technoboy- pushed a commit that referenced this pull request Jul 4, 2023
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jul 17, 2023
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.

None yet

5 participants