You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/12/16 08:48:23 UTC

[GitHub] [bookkeeper] StevenLuMT opened a new pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

StevenLuMT opened a new pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946


   Descriptions of the changes in this PR:
   
   
   
   ### Motivation
   
   bug fix for EntryLocationIndex.getLastEntryInLedger
   
   ### Changes
   
   1.if the ledger has been deleted,run default path for NoEntryException
   
   Master Issue: #2927 


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-997306786


   > 
   
   thanks for reviewing, I have updated this description


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002849062


   @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-997403044


   @nicoloboschi  @dlg99  @eolivelli 
   if this pr has no problem , help me merge it ,thanks 


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-997797056


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks.


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-997797056


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1003642440


   @dlg99
   Happy new year,  if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1001104032


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1003083263


   @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1006313615


   @dlg99 @Vanlightly 
   Happy new year, if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002356104


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-995563685


   @nicoloboschi @dlg99 @eolivelli @Shoothzj
   If you have time, please help me review it, thank you


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] pkumar-singh commented on a change in pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
pkumar-singh commented on a change in pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#discussion_r774774479



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java
##########
@@ -93,9 +93,16 @@ public long getLocation(long ledgerId, long entryId) throws IOException {
     public long getLastEntryInLedger(long ledgerId) throws IOException {
         if (deletedLedgers.contains(ledgerId)) {
             // Ledger already deleted
-            return -1;
+            if (log.isDebugEnabled()) {

Review comment:
       If you want to change semantics of this method you may have to change where and how this method is being used.




-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1001104032


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002915150


   @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-997384169


   `rerun failure checks`


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1000091762


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] pkumar-singh commented on a change in pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
pkumar-singh commented on a change in pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#discussion_r774774180



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java
##########
@@ -93,9 +93,16 @@ public long getLocation(long ledgerId, long entryId) throws IOException {
     public long getLastEntryInLedger(long ledgerId) throws IOException {
         if (deletedLedgers.contains(ledgerId)) {
             // Ledger already deleted
-            return -1;
+            if (log.isDebugEnabled()) {

Review comment:
       I think you are changing the  semantics of this method.
   As far as I could see. 
   Last Entry ID -1 means ledger is deleted. 
   ```
   for (entryId = firstEntryId; entryId < lastEntryId; entryId++) {
      // do somehting.
   }
   ```
   
   Since Last Entry ID is -1 it will not go in the for loop.  Throwing Bookie.NoEntryException might have undesirable effect in this case.
   
   However, Bookie.NoEntryException is thrown when EntryID is valid but entry does not exist. 
   




-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1001360053


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1006313615


   @dlg99 @Vanlightly 
   Happy new year, if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1000813187


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1001285741


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-999347075


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002849062


   @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1003365178


   @dlg99 
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1003718551


   @dlg99
   Happy new year, if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1003718551


   @dlg99
   Happy new year, if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002529794


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002576562


    @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002576562


    @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1000813187


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-997384169


   `rerun failure checks`


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1001285741


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1001360053


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002356104


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1004482558


   @dlg99
   Happy new year, if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1003365178


   @dlg99 
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT edited a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT edited a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-997797056


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1004482558


   @dlg99
   Happy new year, if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1008478999


   @dlg99 @Vanlightly
   Happy new year, if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] pkumar-singh commented on a change in pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
pkumar-singh commented on a change in pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#discussion_r774774180



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java
##########
@@ -93,9 +93,16 @@ public long getLocation(long ledgerId, long entryId) throws IOException {
     public long getLastEntryInLedger(long ledgerId) throws IOException {
         if (deletedLedgers.contains(ledgerId)) {
             // Ledger already deleted
-            return -1;
+            if (log.isDebugEnabled()) {

Review comment:
       I think you are changing the  semantics of this method.
   As far as I could see. 
   Last Entry ID -1 means ledger is deleted. 
   ```
   for (entryId = firstEntryId; entryId < lastEntryId; entryId++) {
      // do somehting.
   }
   ```
   
   Since Last Entry ID is -1 it will not go in the for loop.  Throwing Bookie.NoEntryException might have undesirable effect in this case.
   
   However, Bookie.NoEntryException is thrown when EntryID is valid but it does not exist. 
   




-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-999347075


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-997403044


   @nicoloboschi  @dlg99  @eolivelli 
   if this pr has no problem , help me merge it ,thanks 


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] zymap merged pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
zymap merged pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946


   


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002529794


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1002915150


   @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT removed a comment on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT removed a comment on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1000091762


   @nicoloboschi @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1003642440


   @dlg99
   Happy new year,  if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] StevenLuMT commented on pull request #2946: bug fix for EntryLocationIndex.getLastEntryInLedger

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on pull request #2946:
URL: https://github.com/apache/bookkeeper/pull/2946#issuecomment-1003083263


   @dlg99 @eolivelli
   if this pr has no problem , help me merge it ,thanks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org