oneby-wang commented on code in PR #25101:
URL: https://github.com/apache/pulsar/pull/25101#discussion_r2638730082
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -2713,26 +2713,29 @@ public void
maybeUpdateCursorBeforeTrimmingConsumedLedger() {
for (ManagedCursor cursor : cursors) {
Position lastAckedPosition =
cursor.getPersistentMarkDeletedPosition() != null
? cursor.getPersistentMarkDeletedPosition() :
cursor.getMarkDeletedPosition();
- LedgerInfo currPointedLedger =
ledgers.get(lastAckedPosition.getLedgerId());
+ LedgerInfo curPointedLedger =
ledgers.get(lastAckedPosition.getLedgerId());
LedgerInfo nextPointedLedger =
Optional.ofNullable(ledgers.higherEntry(lastAckedPosition.getLedgerId()))
.map(Map.Entry::getValue).orElse(null);
- if (currPointedLedger != null) {
+ if (curPointedLedger != null) {
if (nextPointedLedger != null) {
if (lastAckedPosition.getEntryId() != -1
- && lastAckedPosition.getEntryId() + 1 >=
currPointedLedger.getEntries()) {
+ && lastAckedPosition.getEntryId() + 1 >=
curPointedLedger.getEntries()) {
lastAckedPosition =
PositionFactory.create(nextPointedLedger.getLedgerId(), -1);
}
} else {
log.debug("No need to reset cursor: {}, current ledger is
the last ledger.", cursor);
}
} else {
+ // TODO no ledger exists, should we move cursor mark deleted
position to nextPointedLedger:-1 ?
log.warn("Cursor: {} does not exist in the managed-ledger.",
cursor);
}
- if (!lastAckedPosition.equals(cursor.getMarkDeletedPosition())) {
+ if (lastAckedPosition.compareTo(cursor.getMarkDeletedPosition()) >
0) {
Position finalPosition = lastAckedPosition;
log.info("Reset cursor:{} to {} since ledger consumed
completely", cursor, lastAckedPosition);
+ // TODO If PR https://github.com/apache/pulsar/pull/25087 is
needed, maybe we should make
Review Comment:
Call stack trace: `OpaddEntry.closeComplete()` -->
`ManagedLedgerImpl.ledgerClosed()`, which doesn't even get a chance to receive
the callback.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -2713,26 +2713,29 @@ public void
maybeUpdateCursorBeforeTrimmingConsumedLedger() {
for (ManagedCursor cursor : cursors) {
Position lastAckedPosition =
cursor.getPersistentMarkDeletedPosition() != null
? cursor.getPersistentMarkDeletedPosition() :
cursor.getMarkDeletedPosition();
- LedgerInfo currPointedLedger =
ledgers.get(lastAckedPosition.getLedgerId());
+ LedgerInfo curPointedLedger =
ledgers.get(lastAckedPosition.getLedgerId());
LedgerInfo nextPointedLedger =
Optional.ofNullable(ledgers.higherEntry(lastAckedPosition.getLedgerId()))
.map(Map.Entry::getValue).orElse(null);
- if (currPointedLedger != null) {
+ if (curPointedLedger != null) {
if (nextPointedLedger != null) {
if (lastAckedPosition.getEntryId() != -1
- && lastAckedPosition.getEntryId() + 1 >=
currPointedLedger.getEntries()) {
+ && lastAckedPosition.getEntryId() + 1 >=
curPointedLedger.getEntries()) {
lastAckedPosition =
PositionFactory.create(nextPointedLedger.getLedgerId(), -1);
}
} else {
log.debug("No need to reset cursor: {}, current ledger is
the last ledger.", cursor);
}
} else {
+ // TODO no ledger exists, should we move cursor mark deleted
position to nextPointedLedger:-1 ?
Review Comment:
I think we should move `markDeletePosition` to `nextPointedLedger:-1` here.
If `ManagedLedger` has only one empty ledger, and `markDeletePosition` is
`ledgerId:-1`, recover read will create a new ledger `ledgerId+1:-1`. If we
don't move the cursor to `ledgerId+1:-1`, the `markDeletePosition` is still
confusion.
--
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]
