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

[fix][broker] Cancel possible pending replay read in cancelPendingRead #23384

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Oct 1, 2024

Motivation

There's an inconsistency in the org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers#cancelPendingRead method:

@Override
protected void cancelPendingRead() {
if (havePendingRead && cursor.cancelPendingReadRequest()) {
havePendingRead = false;
}
}

In addition to havePendingRead, there's a separate field havePendingReplayRead that tracks a pending read too. It should be taken into account in cancelPendingRead method.

Modifications

  • handle havePendingReplayRead in cancelPendingRead method.

Documentation

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

@lhotari lhotari added ready-to-test category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost release/3.0.7 release/3.3.2 labels Oct 1, 2024
@lhotari lhotari added this to the 4.0.0 milestone Oct 1, 2024
@lhotari lhotari self-assigned this Oct 1, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 1, 2024
@codecov-commenter
Copy link

codecov-commenter commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.54%. Comparing base (bbc6224) to head (1138fbe).
Report is 617 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23384      +/-   ##
============================================
+ Coverage     73.57%   74.54%   +0.96%     
- Complexity    32624    33967    +1343     
============================================
  Files          1877     1934      +57     
  Lines        139502   145140    +5638     
  Branches      15299    15870     +571     
============================================
+ Hits         102638   108189    +5551     
+ Misses        28908    28661     -247     
- Partials       7956     8290     +334     
Flag Coverage Δ
inttests 27.45% <50.00%> (+2.87%) ⬆️
systests 24.50% <50.00%> (+0.18%) ⬆️
unittests 73.90% <100.00%> (+1.05%) ⬆️

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

Files with missing lines Coverage Δ
...sistent/PersistentDispatcherMultipleConsumers.java 75.68% <100.00%> (+1.35%) ⬆️

... and 603 files with indirect coverage changes

@rdhabalia rdhabalia merged commit d2c91b1 into apache:master Oct 1, 2024
54 of 55 checks passed
lhotari added a commit that referenced this pull request Oct 4, 2024
lhotari added a commit that referenced this pull request Oct 4, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Oct 15, 2024
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Oct 16, 2024
lhotari added a commit to lhotari/pulsar that referenced this pull request Jan 16, 2025
poorbarcode pushed a commit that referenced this pull request Jan 17, 2025
lhotari added a commit that referenced this pull request Jan 17, 2025
…ad in cancelPendingRead (#23384)" (#23855)

(cherry picked from commit ea56ada)
lhotari added a commit that referenced this pull request Jan 17, 2025
…ad in cancelPendingRead (#23384)" (#23855)

(cherry picked from commit ea56ada)
lhotari added a commit that referenced this pull request Jan 17, 2025
…ad in cancelPendingRead (#23384)" (#23855)

(cherry picked from commit ea56ada)
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request Jan 23, 2025
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 31, 2025
…ad in cancelPendingRead (apache#23384)" (apache#23855)

(cherry picked from commit ea56ada)
(cherry picked from commit 7387653)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Feb 3, 2025
…ad in cancelPendingRead (apache#23384)" (apache#23855)

(cherry picked from commit ea56ada)
(cherry picked from commit 7387653)
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Feb 10, 2025
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost cherry-picked/branch-3.0 cherry-picked/branch-3.3 cherry-picked/branch-4.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.7 release/3.3.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants