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/26 13:22:53 UTC

[GitHub] [bookkeeper] StevenLuMT opened a new pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   Descriptions of the changes in this PR:
   
   
   
   ### Motivation
   
   1. the bookie's machine is broken, when the ledger's status is IN_RECOVERY, the autorecovery and old recover tool (you can run with this command: bin/bookkeeper shell recover) couldn't solve these ledgers in IN_RECOVERY.
   2. I develop these code in branch-4.14 to recover these ledgers in IN_RECOVERY
   3. then I find a same pr #2870 ,but I think mine codes are more **accurate** to solve this problem 
   
   ### Changes
   
   1.add RecoverCommand's function skipRemoveBookieStatus (just skip remove bookie's status to revover the others bookie's data)
   2.add param(named skipRemoveBookieStatus) in client API in LedgerOpenOp/LedgerRecoveryOp/PendingReadLacOp/PendingReadOp/ReadLastConfirmedOp
   
   


-- 
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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] StevenLuMT commented on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @hangc0276 @zymap 
   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] StevenLuMT removed a comment on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @hangc0276 @zymap 
   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] StevenLuMT commented on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] StevenLuMT removed a comment on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] StevenLuMT removed a comment on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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] zymap commented on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @hangc0276 Could you please take a look as well? Because it looks like is an improvement of the skip unrecoverable ledger.


-- 
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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] StevenLuMT commented on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] StevenLuMT commented on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] StevenLuMT commented on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] StevenLuMT commented on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] StevenLuMT removed a comment on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @hangc0276 @zymap
   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] StevenLuMT commented on pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   > Could you please give me more details of this PR?
   
   @zymap 
   case when a bookie node is down and the machine cannot be repaired:
   
   The ledger in IN_RECOVERY status cann't be auto recovery successfully,
   so we add this function to recover this ledger.


-- 
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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @zymap
   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] zymap commented on a change in pull request #2966: Add skip remove bookie option for bookkeeper shell recover command

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



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java
##########
@@ -93,12 +96,17 @@ LedgerRecoveryOp setEntryListener(ReadEntryListener entryListener) {
     }
 
     public CompletableFuture<LedgerHandle> initiate() {
+        return initiate(null);
+    }
+
+    public CompletableFuture<LedgerHandle> initiate(Set<BookieId> skipStatusRemoveBookies) {
+        this.skipStatusRemoveBookies = skipStatusRemoveBookies;
         ReadLastConfirmedOp rlcop = new ReadLastConfirmedOp(clientCtx.getBookieClient(),
-                                                            lh.distributionSchedule,
-                                                            lh.macManager,
-                                                            lh.ledgerId,
-                                                            lh.getCurrentEnsemble(),
-                                                            lh.ledgerKey,
+                lh.distributionSchedule,

Review comment:
       Please avoid formatting code in the PR, that confuses the review.

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadOnlyLedgerHandle.java
##########
@@ -261,13 +265,23 @@ void recover(GenericCallback<Void> finalCb) {
     void recover(GenericCallback<Void> finalCb,
                  final @VisibleForTesting ReadEntryListener listener,
                  final boolean forceRecovery) {
+        recover(finalCb,
+                listener,
+                forceRecovery,
+                null);
+    }
+
+    void recover(GenericCallback<Void> finalCb,
+                 final @VisibleForTesting ReadEntryListener listener,
+                 final boolean forceRecovery,
+                 final Set<BookieId> skipStatusRemoveBookies) {
         final GenericCallback<Void> cb = new TimedGenericCallback<Void>(
-            finalCb,
-            BKException.Code.OK,
-            clientCtx.getClientStats().getRecoverOpLogger());
+                finalCb,

Review comment:
       same above.

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java
##########
@@ -530,7 +549,7 @@ void initiate() {
             entry.read();
             if (!parallelRead && clientCtx.getConf().readSpeculativeRequestPolicy.isPresent()) {
                 speculativeTask = clientCtx.getConf().readSpeculativeRequestPolicy.get()
-                    .initiateSpeculativeRequest(clientCtx.getScheduler(), entry);
+                        .initiateSpeculativeRequest(clientCtx.getScheduler(), entry);

Review comment:
       same above.




-- 
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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   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 #2966: Add skip remove bookie option for bookkeeper shell recover command

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


   @nicoloboschi @dlg99 @eolivelli @hangc0276 @zymap
   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