poorbarcode commented on code in PR #17751:
URL: https://github.com/apache/pulsar/pull/17751#discussion_r990613777


##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java:
##########
@@ -116,6 +116,22 @@ public void readEntriesFailed(ManagedLedgerException 
exception, Object ctx) {
                 recycle();
                 return;
             }
+
+            int toAckEntryNum = 0;
+            List<Entry> skippedEntries = new ArrayList<>();
+            PositionImpl startPosition = readPosition;
+            PositionImpl endPosition = (PositionImpl) nexReadPosition;
+            while (startPosition.compareTo(endPosition) < 0) {
+                
skippedEntries.add(EntryImpl.createSkippedEntry(startPosition.ledgerId, 
startPosition.entryId));
+                startPosition = ledger.getNextValidPosition(startPosition);
+                toAckEntryNum++;
+                if (toAckEntryNum > 
cursor.getConfig().getMaxAckEntryNumForAutoSkipNonRecoverableData()) {

Review Comment:
   If the state of the cursor is like this:
   
   ```
   read position {1:0}
   individual deleted messages [ {1:0}, {1:50001} ]
   ```
   
   After the entry filter, the `nexReadPosition` will be {1,10000} and  
`filteredEntries` will be {1,0}, then 
`maxAckEntryNumForAutoSkipNonRecoverableData` could not work perfect. Can we 
let `cursor` do the filtering?
   



##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java:
##########
@@ -116,6 +116,22 @@ public void readEntriesFailed(ManagedLedgerException 
exception, Object ctx) {
                 recycle();
                 return;
             }
+
+            int toAckEntryNum = 0;
+            List<Entry> skippedEntries = new ArrayList<>();
+            PositionImpl startPosition = readPosition;
+            PositionImpl endPosition = (PositionImpl) nexReadPosition;
+            while (startPosition.compareTo(endPosition) < 0) {
+                
skippedEntries.add(EntryImpl.createSkippedEntry(startPosition.ledgerId, 
startPosition.entryId));
+                startPosition = ledger.getNextValidPosition(startPosition);
+                toAckEntryNum++;
+                if (toAckEntryNum > 
cursor.getConfig().getMaxAckEntryNumForAutoSkipNonRecoverableData()) {
+                    nexReadPosition = startPosition;
+                    break;
+                }
+            }
+            List<Entry> filteredEntries = 
cursor.filterReadEntries(skippedEntries);

Review Comment:
   Can we just drop the data with the instruction `cursor.delete(positions)`? 
This saves the memory of `entries` and is easier to understand.



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