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 ttl expiration block due to no-recoverable exception even if autoSkipNonRecoverableData=true #19132

Merged
merged 1 commit into from
Jan 8, 2023

Conversation

aloyszhang
Copy link
Contributor

@aloyszhang aloyszhang commented Jan 4, 2023

fix #19131

Motivation

fix TTL expiration block due to no-recoverable exception even if autoSkipNonRecoverableData=true

Modifications

In the logic of message expiration, if met NonRecoverableLedgerException when reading the oldest msg, just skip these msgs if autoSkipNonRecoverableData is set to true.

Documentation

  • doc-not-needed

Matching PR in forked repository

PR in forked repository:[ ]
aloyszhang#10

@aloyszhang aloyszhang requested a review from AnonHxy January 6, 2023 05:26
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

@aloyszhang aloyszhang self-assigned this Jan 7, 2023
@aloyszhang
Copy link
Contributor Author

@AnonHxy PTAL again, thx.

@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2023

Codecov Report

Merging #19132 (bd6966e) into master (9ec1d07) will decrease coverage by 0.37%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19132      +/-   ##
============================================
- Coverage     47.73%   47.35%   -0.38%     
+ Complexity    10819    10735      -84     
============================================
  Files           712      713       +1     
  Lines         69645    69674      +29     
  Branches       7481     7483       +2     
============================================
- Hits          33242    32995     -247     
- Misses        32699    32960     +261     
- Partials       3704     3719      +15     
Flag Coverage Δ
unittests 47.35% <50.00%> (-0.38%) ⬇️

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

Impacted Files Coverage Δ
...sar/broker/service/persistent/PersistentTopic.java 61.64% <50.00%> (-1.03%) ⬇️
.../pulsar/broker/service/SharedConsumerAssignor.java 3.70% <0.00%> (-64.82%) ⬇️
...java/org/apache/pulsar/proxy/stats/TopicStats.java 58.82% <0.00%> (-41.18%) ⬇️
...apache/pulsar/broker/service/EntryAndMetadata.java 0.00% <0.00%> (-40.75%) ⬇️
...ar/broker/loadbalance/impl/BundleSplitterTask.java 54.00% <0.00%> (-28.00%) ⬇️
...e/pulsar/broker/service/EntryBatchIndexesAcks.java 82.14% <0.00%> (-10.72%) ⬇️
...roker/service/persistent/MessageDeduplication.java 43.23% <0.00%> (-7.43%) ⬇️
...sistent/PersistentDispatcherMultipleConsumers.java 52.47% <0.00%> (-6.67%) ⬇️
...er/loadbalance/impl/ModularLoadManagerWrapper.java 73.91% <0.00%> (-6.53%) ⬇️
...roker/loadbalance/impl/ModularLoadManagerImpl.java 63.06% <0.00%> (-5.83%) ⬇️
... and 36 more

@AnonHxy
Copy link
Contributor

AnonHxy commented Jan 8, 2023

LGTM

@aloyszhang aloyszhang merged commit badd69b into apache:master Jan 8, 2023
@Technoboy- Technoboy- added this to the 2.12.0 milestone Jan 9, 2023
@Technoboy- Technoboy- added type/bug The PR fixed a bug or issue reported a bug area/broker labels Jan 9, 2023
@aloyszhang aloyszhang deleted the ttl-no-recoverable branch August 17, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker doc-not-needed Your PR changes do not impact docs type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] ttl expiration block due to no-recoverable exception even if autoSkipNonRecoverableData=true
5 participants