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][ml] Managed ledger should recover after open ledger failed #23368

Merged

Conversation

Demogorgon314
Copy link
Member

Motivation

When open managed ledger fails, the entryCacheManager's ml cache will be a closed managed ledger, then if we open this ledger again, it will fail to read because org.apache.bookkeeper.mledger.ManagedLedgerException: LastConfirmedEntry is null when reading ledger xxx exception.

Modifications

  • Remove the entryCacheManager's cache if open ml fails.
  • Add unit test to cover this case.

Documentation

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

@Demogorgon314 Demogorgon314 added type/bug The PR fixed a bug or issue reported a bug ready-to-test area/ML labels Sep 29, 2024
@Demogorgon314 Demogorgon314 self-assigned this Sep 29, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 29, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.59%. Comparing base (bbc6224) to head (6bd2c9b).
Report is 731 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23368      +/-   ##
============================================
+ Coverage     73.57%   74.59%   +1.02%     
- Complexity    32624    34485    +1861     
============================================
  Files          1877     1934      +57     
  Lines        139502   145132    +5630     
  Branches      15299    15870     +571     
============================================
+ Hits         102638   108267    +5629     
+ Misses        28908    28572     -336     
- Partials       7956     8293     +337     
Flag Coverage Δ
inttests 27.42% <100.00%> (+2.84%) ⬆️
systests 24.55% <0.00%> (+0.22%) ⬆️
unittests 73.94% <100.00%> (+1.10%) ⬆️

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

Files with missing lines Coverage Δ
...kkeeper/mledger/impl/ManagedLedgerFactoryImpl.java 59.23% <100.00%> (-22.16%) ⬇️

... and 602 files with indirect coverage changes

@BewareMyPower BewareMyPower added this to the 4.0.0 milestone Sep 29, 2024
@BewareMyPower BewareMyPower merged commit 77cb67a into apache:master Sep 29, 2024
53 checks passed
lhotari pushed a commit that referenced this pull request Oct 10, 2024
lhotari pushed a commit that referenced this pull request Oct 10, 2024
)

(cherry picked from commit 77cb67a)
(cherry picked from commit c80eb40)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Oct 15, 2024
…che#23368)

(cherry picked from commit 77cb67a)
(cherry picked from commit c80eb40)
(cherry picked from commit 7daad7a)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Oct 16, 2024
…che#23368)

(cherry picked from commit 77cb67a)
(cherry picked from commit c80eb40)
(cherry picked from commit 7daad7a)
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.

4 participants