sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356203
 
 

 ##########
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
 ##########
 @@ -686,21 +688,25 @@ int runCmd(CommandLine cmdLine) throws Exception {
                 predicate = replicasList -> 
!replicasList.contains(excludingBookieId);
             }
 
-            ZooKeeper zk = null;
+            LedgerManagerFactory mFactory = null;
             try {
-                zk = ZooKeeperClient.newBuilder()
-                        .connectString(bkConf.getZkServers())
-                        .sessionTimeoutMs(bkConf.getZkTimeout())
-                        .build();
-                LedgerManagerFactory mFactory = 
LedgerManagerFactory.newLedgerManagerFactory(bkConf, zk);
+                mFactory = LedgerManagerFactory.newLedgerManagerFactory(
+                    bkConf,
+                    new ZkLayoutManager(
 
 Review comment:
   I think it would be better to avoid construct ZkLayoutManager directly.
   
   A better way to do this:
   
   ```
   try (RegistrationManager rm = ...) {
       try (LedgerManagerFactory mFactory = 
LedgerManagerFactory.newLedgerManagerFactory(.., rm.getLayoutManager()) {
          ...
       }
   }
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to