oneby-wang commented on code in PR #25101:
URL: https://github.com/apache/pulsar/pull/25101#discussion_r2639109807


##########
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) {

Review Comment:
   > I'd suppose that maybeUpdateCursorBeforeTrimmingConsumedLedger would have 
to be made asynchronous (return CompletableFuture) and the locations where it 
is being called should be evaluated.
   
   Seems not so easy to deal with these two cases, they can't bring 
CompletableFuture or Callback to their caller callbacks.
   
   
https://github.com/apache/pulsar/blob/3fb52c5d7494d4020956b7a26db5e9e721c7d555/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java#L1747-L1769
   
   
https://github.com/apache/pulsar/blob/3fb52c5d7494d4020956b7a26db5e9e721c7d555/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ShadowManagedLedgerImpl.java#L328-L346



-- 
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]

Reply via email to