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 2022/02/19 02:04:33 UTC

[GitHub] [bookkeeper] pkumar-singh opened a new pull request #3067: Recover only those entries which as supposed to be on dead bookie

pkumar-singh opened a new pull request #3067:
URL: https://github.com/apache/bookkeeper/pull/3067


   **Motivation**
   
   When WQ < E there would be case when some entries of a fragment will be on Bookie B1 and not on B2 while some will on B2 but not B1. Currently If B1 is dead and being recovered, the process will not only recover entries which were on B1 but also those entries which were not on B1 but were on B2.
   
   **Change**
   From the fragment recover only those entries which were supposed to be on dead bookie


-- 
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] eolivelli commented on a change in pull request #3067: Recover only those entries which as supposed to be on dead bookie

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #3067:
URL: https://github.com/apache/bookkeeper/pull/3067#discussion_r832973062



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerFragmentReplicator.java
##########
@@ -133,6 +133,16 @@ public LedgerFragmentReplicator(BookKeeper bkc, ClientConfiguration conf) {
     private static final Logger LOG = LoggerFactory
             .getLogger(LedgerFragmentReplicator.class);
 
+    private boolean isEntryExpectedOnDeadBookie(final LedgerHandle lh, Long eId, Set<Integer> deadBookieIndex) {
+        DistributionSchedule.WriteSet writeSet = lh.getWriteSetForReadOperation(eId);

Review comment:
       we should call `writeSet .recycle()` in a finally block in order to reduce allocations

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerFragmentReplicator.java
##########
@@ -133,6 +133,16 @@ public LedgerFragmentReplicator(BookKeeper bkc, ClientConfiguration conf) {
     private static final Logger LOG = LoggerFactory
             .getLogger(LedgerFragmentReplicator.class);
 
+    private boolean isEntryExpectedOnDeadBookie(final LedgerHandle lh, Long eId, Set<Integer> deadBookieIndex) {

Review comment:
       why the name is "deadBookieIndex" ? you are passing "lf.getBookiesIndexes()"




-- 
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 commented on pull request #3067: Recover only those entries which as supposed to be on dead bookie

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


   Could you please add a test to cover this?


-- 
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] mauricebarnum commented on a change in pull request #3067: Recover only those entries which as supposed to be on dead bookie

Posted by GitBox <gi...@apache.org>.
mauricebarnum commented on a change in pull request #3067:
URL: https://github.com/apache/bookkeeper/pull/3067#discussion_r810671246



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerFragmentReplicator.java
##########
@@ -133,6 +133,16 @@ public LedgerFragmentReplicator(BookKeeper bkc, ClientConfiguration conf) {
     private static final Logger LOG = LoggerFactory
             .getLogger(LedgerFragmentReplicator.class);
 
+    private boolean isEntryExpectedOnDeadBookie(final LedgerHandle lh, Long eId, Set<Integer> deadBookieIndex) {

Review comment:
       looks like this would work and might be the shortest path from where you are to where you want to get.
   
   it's somewhat unfortunate you can't just configure the LedgerHandle to avoid the dead bookies when creating the write set.  there's almost support in there for doing that, but it isn't too flexible and ultimately optional at both the LedgerHandle and DistributionSchedule implementation levels.  see how bookie health info is almost useful.




-- 
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 pull request #3067: Recover only those entries which as supposed to be on dead bookie

Posted by GitBox <gi...@apache.org>.
pkumar-singh commented on pull request #3067:
URL: https://github.com/apache/bookkeeper/pull/3067#issuecomment-1048396351


   > Could you please add a test to cover this?
   
   Done.


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