Skip to content

[fix] Failed read entries after multiple decommissioning #4613

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.

Sign up for GitHub

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

Jump to bottom
Merged
zymap merged 27 commits into from
Oct 29, 2025
Merged

[fix] Failed read entries after multiple decommissioning #4613

zymap merged 27 commits into from
Oct 29, 2025
+352 −13

Conversation

@poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Jun 5, 2025 •
edited
Loading

Motivation

Background

  • ReadOnlyLedgerHandle opened without doRecovery, in other words, it has not been closed yet, its metadata in memory will be updated once modified.
  • ReadOnlyLedgerHandle opened with doRecovery, in other words, it has been closed or it will be closed; its metadata in memory will never be updated

Issue

  • Background: The ledger's metadata can also be modified by the auto-recovery component.
  • There is a scenario in which a ledger handle always gets the error org.apache.bookkeeper.client.BKException$BKBookieHandleNotAvailableException: Bookie handle is not available` after multiple decommissions. The reproduction steps are as follows
    • Client service opens a read-only ledger handle, which has been closed.
    • All BKs that relate to the ledger have been decommissioned.
    • Auto recovery component moved the data into other BK instances who is alive.
    • The ledger handle in the client memory keeps connecting to the BKs who are in the original ensemble set, and the connection will always fail.
  • You can reproduce the issue with the new test testOpenedLedgerHandleStillWorkAfterDecommissioning

Changes

Let Bookie LedgerHandle always accept the metadata updates, no matter "doRecovery" or not.

Provide a new API to open a read-only ledger handle with keepUpdateMetadata, the ledger's metadata in memory will be updated automatically after the auto-recovery component updates it.

@codelipenghui
Copy link
Contributor

codelipenghui commented Jun 5, 2025

I'm curious about is there any cases that the ReadOnlyLedgerHandle cannot accept the updates from metadata server. As I understand, it should always accept the metadata updates no matter "doRecovery" or not. If the Ledger metadata updates to the Ledger which need to be recovered is not expected, we should fix the updates parts.

@poorbarcode
Copy link
Contributor Author

poorbarcode commented Jun 6, 2025

@codelipenghui

I'm curious about is there any cases that the ReadOnlyLedgerHandle cannot accept the updates from metadata server. As I understand, it should always accept the metadata updates no matter "doRecovery" or not.

Changed the implementation.

@poorbarcode poorbarcode closed this Jun 6, 2025
@poorbarcode poorbarcode reopened this Jun 6, 2025
@poorbarcode poorbarcode requested a review from zymap June 11, 2025 02:52
@poorbarcode poorbarcode requested a review from zymap July 10, 2025 06:40
@zymap zymap requested a review from hangc0276 July 18, 2025 02:21
@zymap zymap added this to the 4.18.0 milestone Jul 18, 2025
@poorbarcode poorbarcode force-pushed the fix/ledger_handle_inavaliable branch from aeb118b to abb7ec5 Compare July 30, 2025 09:15
@StevenLuMT
Copy link
Member

StevenLuMT commented Aug 6, 2025

rerun failure checks

@poorbarcode poorbarcode closed this Aug 6, 2025
@poorbarcode poorbarcode reopened this Aug 6, 2025
StevenLuMT
StevenLuMT approved these changes Aug 10, 2025
View reviewed changes
Copy link
Member

@StevenLuMT StevenLuMT left a comment

Choose a reason for hiding this comment

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

Good jobs

@lhotari lhotari mentioned this pull request Aug 27, 2025
Open
14 tasks
@poorbarcode poorbarcode force-pushed the fix/ledger_handle_inavaliable branch from 5b5294a to fd48774 Compare September 9, 2025 09:18
@poorbarcode poorbarcode reopened this Sep 10, 2025
@poorbarcode poorbarcode reopened this Sep 11, 2025
@poorbarcode poorbarcode reopened this Sep 11, 2025
@poorbarcode poorbarcode force-pushed the fix/ledger_handle_inavaliable branch from e125401 to b1b453c Compare October 23, 2025 03:08
@poorbarcode poorbarcode reopened this Oct 27, 2025
@poorbarcode
Copy link
Contributor Author

poorbarcode commented Oct 27, 2025

rerun failure checks

@poorbarcode poorbarcode reopened this Oct 27, 2025
@poorbarcode
Copy link
Contributor Author

poorbarcode commented Oct 27, 2025 •
edited
Loading

rerun failure checks

https://github.com/apache/bookkeeper/actions/runs/18828128247/job/53714715505?pr=4672

@poorbarcode poorbarcode reopened this Oct 27, 2025
@poorbarcode poorbarcode reopened this Oct 27, 2025
@poorbarcode poorbarcode reopened this Oct 27, 2025
@poorbarcode
Copy link
Contributor Author

poorbarcode commented Oct 27, 2025

rerun failure checks

@poorbarcode poorbarcode reopened this Oct 27, 2025
@poorbarcode poorbarcode reopened this Oct 27, 2025
@zymap zymap merged commit bae9e49 into apache:master Oct 29, 2025
148 of 287 checks passed
lhotari pushed a commit that referenced this pull request Nov 28, 2025
@poorbarcode @lhotari
[fix] Failed read entries after multiple decommissioning (#4613)
b6d8b0f 
* -

* checkstyle

* let all ledger handle enable watcher

* let all ledger handle enable watcher

* fix tests

* fix tests

* fix tests

* add test logs for debug

* add test logs for debug

* add test logs for debug

* -

* add a new param keepUpdateMetadata when open a read-only ledger handle

* address comments

* address comment

* address comment

* test CI

* test CI

* test CI

* test CI

* test CI

* test CI

* test CI

* remove logs for CI

* test CI

* remove logs for CI

* address comment

* fix test

(cherry picked from commit bae9e49)
priyanshu-ctds pushed a commit to datastax/bookkeeper that referenced this pull request Dec 2, 2025
@poorbarcode @priyanshu-ctds
[fix] Failed read entries after multiple decommissioning (apache#4613)
3faaeee 
* -

* checkstyle

* let all ledger handle enable watcher

* let all ledger handle enable watcher

* fix tests

* fix tests

* fix tests

* add test logs for debug

* add test logs for debug

* add test logs for debug

* -

* add a new param keepUpdateMetadata when open a read-only ledger handle

* address comments

* address comment

* address comment

* test CI

* test CI

* test CI

* test CI

* test CI

* test CI

* test CI

* remove logs for CI

* test CI

* remove logs for CI

* address comment

* fix test

(cherry picked from commit bae9e49)
(cherry picked from commit b6d8b0f)
srinath-ctds pushed a commit to datastax/bookkeeper that referenced this pull request Dec 4, 2025
@poorbarcode @srinath-ctds
[fix] Failed read entries after multiple decommissioning (apache#4613)
d359715 
* -

* checkstyle

* let all ledger handle enable watcher

* let all ledger handle enable watcher

* fix tests

* fix tests

* fix tests

* add test logs for debug

* add test logs for debug

* add test logs for debug

* -

* add a new param keepUpdateMetadata when open a read-only ledger handle

* address comments

* address comment

* address comment

* test CI

* test CI

* test CI

* test CI

* test CI

* test CI

* test CI

* remove logs for CI

* test CI

* remove logs for CI

* address comment

* fix test

(cherry picked from commit bae9e49)
(cherry picked from commit b6d8b0f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zymap zymap zymap approved these changes

@StevenLuMT StevenLuMT StevenLuMT approved these changes

@hangc0276 hangc0276 Awaiting requested review from hangc0276

Assignees

@poorbarcode poorbarcode

Labels

cherry-picked/branch-4.17 release/4.17.3 type/improvement

Projects

None yet

Milestone

4.18.0

Development

Successfully merging this pull request may close these issues.

6 participants

@poorbarcode @codelipenghui @StevenLuMT @hangc0276 @zymap @lhotari