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

[fix][broker] Fix incorrect unack msk count when dup ack a message #20990

Merged
merged 3 commits into from
Aug 18, 2023

Conversation

Technoboy-
Copy link
Contributor

@Technoboy- Technoboy- commented Aug 15, 2023

Fixes #19268

Modifications

When pendingAck not contains the message, it means the msg has been removed, so it can't update the unack msg count..

Documentation

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

@Technoboy- Technoboy- changed the title Fix incorrect unack msk count when dup ack a message [fix][broker] Fix incorrect unack msk count when dup ack a message Aug 15, 2023
@Technoboy- Technoboy- self-assigned this Aug 15, 2023
@Technoboy- Technoboy- added this to the 3.2.0 milestone Aug 15, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 15, 2023
@codelipenghui codelipenghui added the type/bug The PR fixed a bug or issue reported a bug label Aug 15, 2023
@Technoboy- Technoboy- closed this Aug 16, 2023
@Technoboy- Technoboy- reopened this Aug 16, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2023

Codecov Report

Merging #20990 (fdac917) into master (63d9eaf) will increase coverage by 0.02%.
Report is 5 commits behind head on master.
The diff coverage is 65.38%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #20990      +/-   ##
============================================
+ Coverage     36.92%   36.95%   +0.02%     
- Complexity    12187    12229      +42     
============================================
  Files          1698     1698              
  Lines        129846   129857      +11     
  Branches      14163    14163              
============================================
+ Hits          47947    47986      +39     
+ Misses        75570    75548      -22     
+ Partials       6329     6323       -6     
Flag Coverage Δ
inttests 24.29% <38.46%> (+0.14%) ⬆️
systests 25.18% <42.30%> (+0.02%) ⬆️
unittests 32.18% <61.53%> (+0.10%) ⬆️

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

Files Changed Coverage Δ
...rg/apache/pulsar/compaction/TwoPhaseCompactor.java 69.95% <58.33%> (-0.23%) ⬇️
...g/apache/pulsar/client/impl/RawBatchConverter.java 75.32% <66.66%> (+1.64%) ⬆️
...ava/org/apache/pulsar/broker/service/Consumer.java 63.47% <72.72%> (-1.09%) ⬇️

... and 58 files with indirect coverage changes

@shibd
Copy link
Member

shibd commented Oct 22, 2023

@Technoboy- Can you help create a PR to cherry-pick this change to branch-2.11?

shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 25, 2023
nodece pushed a commit to nodece/pulsar that referenced this pull request Jan 11, 2024
…pache#20990)

(cherry picked from commit 4facdad)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
nodece added a commit to ascentstream/pulsar that referenced this pull request Jan 16, 2024
* [fix][broker] Fix incorrect unack msk count when dup ack a message (apache#20990)

(cherry picked from commit 4facdad)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* [fix][test] flaky test `testCanRecoverConsumptionWhenLiftMaxUnAckedMessagesRestriction` (apache#18726)

(cherry picked from commit 2d205c9)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

---------

Co-authored-by: Jiwei Guo <technoboy@apache.org>
Co-authored-by: labuladong <labuladong@foxmail.com>
nodece pushed a commit to nodece/pulsar that referenced this pull request Mar 8, 2024
nodece pushed a commit to nodece/pulsar that referenced this pull request Mar 8, 2024
nodece pushed a commit to nodece/pulsar that referenced this pull request Mar 8, 2024
nodece added a commit to ascentstream/pulsar that referenced this pull request Mar 15, 2024
* [fix][broker] Fix incorrect unack msk count when dup ack a message (apache#20990)

(cherry picked from commit 4facdad)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* [fix][test] flaky test `testCanRecoverConsumptionWhenLiftMaxUnAckedMessagesRestriction` (apache#18726)

(cherry picked from commit 2d205c9)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

---------

Co-authored-by: Jiwei Guo <technoboy@apache.org>
Co-authored-by: labuladong <labuladong@foxmail.com>
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.

[Bug] Acknowledge the same message-id causes inconsistency unackedMessages number.
7 participants