Denovo1998 commented on code in PR #25955:
URL: https://github.com/apache/pulsar/pull/25955#discussion_r3380631350
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -3682,23 +3692,27 @@ public void asyncOffloadPrefix(Position pos,
OffloadCallback callback, Object ct
}
long current = ledgers.lastKey();
+ boolean includeLastLedger = STATE_UPDATER.get(this) ==
State.Terminated
Review Comment:
I think this needs one more guard for the in-progress termination window.
asyncTerminate() sets state = State.Terminated before the BookKeeper close
callback refreshes the final LedgerInfo. If asyncOffloadPrefix() runs in that
window, includeLastLedger becomes true, but the current ledger still has the
active-ledger placeholder metadata (entries/size are still 0 and timestamp is
not refreshed). The loop can then find no ledgers to offload and return
lastConfirmedEntry.getNext(), which reports the prefix as fully offloaded even
though the final ledger was never offloaded.
Could we gate includeLastLedger on the final LedgerInfo actually being
closed/refreshed, or introduce a separate terminating state so manual offload
does not treat an in-flight termination as completed? A regression test can
block the BK close with PulsarMockBookKeeper.promiseAfter(0), call
asyncTerminate(), call offloadPrefix() before releasing the close, and assert
it does not report lastConfirmedEntry.getNext() without offloading the final
ledger.
--
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]
