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][client] Fix race-condition causing doReconsumeLater to hang when creating retryLetterProducer has failed #23560

Merged
merged 3 commits into from
Nov 29, 2024

Conversation

hanmz
Copy link
Contributor

@hanmz hanmz commented Nov 5, 2024

Motivation

When retryLetterProducer creation fails, retryLetterProducer will be set to null. So retryLetterProducer==null may occur when race-condition is present, at this time, the results cannot be completed.

Only handle the situation when retryLetterProducer is not equal to null:

Modifications

When retryLetterProducer==null, mark the result as an completeExceptionally.

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 Nov 5, 2024
@lhotari
Copy link
Member

lhotari commented Nov 5, 2024

Do you have a chance to add a test case?

@hanmz
Copy link
Contributor Author

hanmz commented Nov 11, 2024

Do you have a chance to add a test case?

I've added a test case.

@lhotari lhotari requested a review from poorbarcode November 11, 2024 18:39
@lhotari lhotari changed the title [fix][client] Fix race-condition causing doReconsumeLater hang up when create retryLetterProducer is failed [fix][client] Fix race-condition causing doReconsumeLater to hang when creating retryLetterProducer has failed Nov 11, 2024
@lhotari lhotari added this to the 4.1.0 milestone Nov 11, 2024
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, thanks for the contribution @hanmz

@lhotari lhotari closed this Nov 27, 2024
@lhotari lhotari reopened this Nov 27, 2024
@lhotari lhotari closed this Nov 29, 2024
@lhotari lhotari reopened this Nov 29, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.04%. Comparing base (bbc6224) to head (0797828).
Report is 767 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23560      +/-   ##
============================================
+ Coverage     73.57%   74.04%   +0.47%     
- Complexity    32624    34650    +2026     
============================================
  Files          1877     1945      +68     
  Lines        139502   151644   +12142     
  Branches      15299    17232    +1933     
============================================
+ Hits         102638   112289    +9651     
- Misses        28908    30618    +1710     
- Partials       7956     8737     +781     
Flag Coverage Δ
inttests 27.25% <50.00%> (+2.67%) ⬆️
systests 25.17% <0.00%> (+0.85%) ⬆️
unittests 73.42% <100.00%> (+0.58%) ⬆️

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

Files with missing lines Coverage Δ
...va/org/apache/pulsar/client/impl/ConsumerImpl.java 74.95% <100.00%> (-2.62%) ⬇️

... and 663 files with indirect coverage changes

@lhotari lhotari merged commit bf1f677 into apache:master Nov 29, 2024
122 of 129 checks passed
lhotari pushed a commit that referenced this pull request Nov 29, 2024
…n creating retryLetterProducer has failed (#23560)

(cherry picked from commit bf1f677)
lhotari pushed a commit that referenced this pull request Nov 29, 2024
…n creating retryLetterProducer has failed (#23560)

(cherry picked from commit bf1f677)
lhotari pushed a commit that referenced this pull request Nov 29, 2024
…n creating retryLetterProducer has failed (#23560)

(cherry picked from commit bf1f677)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 6, 2024
…n creating retryLetterProducer has failed (apache#23560)

(cherry picked from commit bf1f677)
(cherry picked from commit 5f25367)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 9, 2024
…n creating retryLetterProducer has failed (apache#23560)

(cherry picked from commit bf1f677)
(cherry picked from commit 5f25367)
hanmz added 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants