[GitHub] [bookkeeper] merlimat commented on a change in pull request #1819: ISSUE-1770: Add local checker for Sorted/InterleavedLedgerStorage

[GitBox <gi...@apache.org>]

merlimat commented on a change in pull request #1819: ISSUE-1770: Add local checker for Sorted/InterleavedLedgerStorage
URL: https://github.com/apache/bookkeeper/pull/1819#discussion_r330316133
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##########
 @@ -724,12 +807,42 @@ public ByteBuf internalReadEntry(long ledgerId, long entryId, long location)
         }
         if (entrySize < MIN_SANE_ENTRY_SIZE) {
             LOG.error("Read invalid entry length {}", entrySize);
-            throw new IOException("Invalid entry length " + entrySize);
+            throw new EntryLookupException.InvalidEntryLengthException(ledgerId, entryId, entryLogId, pos);
+        }
+
+        long thisLedgerId = sizeBuff.getLong(4);
+        long thisEntryId = sizeBuff.getLong(12);
+        if (thisLedgerId != ledgerId || thisEntryId != entryId) {
+            throw new EntryLookupException.WrongEntryException(
 
 Review comment:
   This is problematic because it happens even from the `internalReadEntry()`. The purpose of `internalReadEntry()` was to avoid the sanity checks from the regular `readEntry()` and to be called when doing read-ahead. 
   
   In read-ahead mode, we don't know whether the next entry is from the same ledger or not. Therefore we keep reading until we find an entry of a different ledger. 
   
   With this change in 4.9.2 that means each read-ahead incurs in creating an exception, which is very expensive (since it will walk through the stack trace each time). 
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services