Denovo1998 commented on code in PR #25795:
URL: https://github.com/apache/pulsar/pull/25795#discussion_r3304004677
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -1749,8 +1767,11 @@ public void operationComplete(Void v, Stat stat) {
synchronized (ManagedLedgerImpl.this) {
try {
State state =
STATE_UPDATER.get(ManagedLedgerImpl.this);
- if (state == State.Closed || state.isFenced()) {
- log.debug().log("skip ledger update after
create complete ledger is closed or fenced");
+ if (state == State.Closed || state ==
State.Terminated || state.isFenced()) {
Review Comment:
I think this branch still leaves one terminate-vs-rollover race unresolved.
By the time this callback reaches `operationComplete`,
`updateLedgersListAfterRollover` has already successfully written metadata that
includes `newLedger`. If `terminate()` is concurrently writing the terminated
metadata, the two `store.asyncUpdateLedgerIds(...)` calls can still race
because `asyncTerminate` is not serialized with this `metadataMutex` path.
There are two problematic outcomes: if the rollover metadata update wins
first, this branch only closes `lh` and relies on a later terminate metadata
update to remove the new ledger from metadata, but the unused BookKeeper ledger
is not deleted; if terminate wins the metadata version race first, this
rollover callback can go through `operationFailed(BadVersionException)` and
call `handleBadVersion`, fencing a ledger that should remain terminated. The
TODO here is therefore part of the correctness fix, not just cleanup.
Can we make the in-flight rollover metadata update terminal-state aware as
well, e.g. serialize terminate with the same metadata update path, or handle
`state == Terminated` in both the success and BadVersion failure callbacks as a
stale rollover completion, while ensuring the unused ledger is removed from
metadata and deleted if it was already written?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
