codelipenghui commented on code in PR #25283:
URL: https://github.com/apache/pulsar/pull/25283#discussion_r2887026837
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -4225,7 +4225,7 @@ public Position getFirstPosition() {
}
if (ledgerId > lastConfirmedEntry.getLedgerId()) {
checkState(ledgers.get(ledgerId).getEntries() == 0);
- ledgerId = lastConfirmedEntry.getLedgerId();
+ return PositionFactory.create(lastConfirmedEntry.getLedgerId(),
lastConfirmedEntry.getEntryId());
Review Comment:
I don't think it's a correct fix since it changed the semantics from
`getFirstPosition` method. Now if there is now entries from the first ledger,
returned the first ledger id and -1 as the entry id is expected.
Now this PR changed to the cached lastConfirmedEntry which already been
deleted. From the caller perspective, it can't know if this entry is a valid
entry.
But the line:4230 seems incorrect. It should use 0 as the entry ID if the
first ledger has entries?
--
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]
