zymap commented on code in PR #4613:
URL: https://github.com/apache/bookkeeper/pull/4613#discussion_r2196724525


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java:
##########
@@ -174,9 +195,21 @@ private void openWithMetadata(Versioned<LedgerMetadata> 
versionedMetadata) {
         }
 
         // get the ledger metadata back
+        final boolean needRecovery = !doRecovery || !metadata.isClosed();
         try {
+            // The ledger metadata may be modified even if it has been closed, 
because the auto-recovery component may
+            // rewrite the ledger's metadata. Keep receiving a notification 
from ZK to avoid the following issue: an
+            // opened ledger handle in memory still accesses to a BK instance 
who has been decommissioned. The issue
+            // that solved happens as follows:
+            // 1. Client service open a readonly ledger handle, which has been 
closed.
+            // 2. All BKs that relates to the ledger have been decommissioned.
+            // 3. Auto recovery component moved the data into other BK 
instances who is alive.
+            // 4. The ledger handle in the client memory keeps connects to the 
BKs who in the original ensemble set,
+            //    and the connection will always fail.
+            // Therefore, if a user needs to the feature that update metadata 
automatically, he will set
+            // "keepUpdateMetadata" to "true",
             lh = new ReadOnlyLedgerHandle(bk.getClientCtx(), ledgerId, 
versionedMetadata, digestType,
-                                          passwd, !doRecovery);
+                                          passwd, !needRecovery);

Review Comment:
   ```suggestion
                                             passwd, needRecovery);
   ```
   
   Should it be this? Otherwise it change the original behaviors?



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