You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/05/26 10:34:59 UTC

[GitHub] [pulsar] yangl opened a new pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

yangl opened a new pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715


   ### Motivation
   
   Since the current  ZkIsolatedBookieEnsemblePlacementPolicy rack configuration metadata is stored on the zookeeper cluster where the broker is located, this causes the bookkeeper to not get the rack information correctly when using a separate zookeeper cluster (**--existing-bk-metadata-service-uri**).
   
   ```shell
   --cluster test-pulsar-1 \
   --zookeeper 10.207.128.13:2181/test-pulsar-1 \
   --configuration-store 110.207.128.13:2181/test-pulsar-1 \
   --existing-bk-metadata-service-uri "zk+hierarchical://10.206.128.154:2181;10.206.128.155:2181;10.206.128.156:2181;10.206.128.157:2181;10.206.128.158:2181/bk-ledgers" \
   --web-service-url http://pulsar.sf.com:8080 \
   --broker-service-url pulsar://pulsar.sf.com:16650
   ```
   ```xml
   ensemblePlacementPolicy=org.apache.pulsar.zookeeper.ZkIsolatedBookieEnsemblePlacementPolicy
   reppDnsResolverClass=org.apache.pulsar.zookeeper.ZkBookieRackAffinityMapping
   ```
   ```
   ./pulsar-admin --admin-url http://10.206.64.12:18080 --auth-params $PULSAR_AUTH_PARAMS --auth-plugin $PULSAR_AUTH_PLUGIN bookies racks-placement
   "g1    {bookie11:3181=BookieInfo(rack=r1, hostname=null), bookie12:3181=BookieInfo(rack=r1, hostname=null), bookie13:3181=BookieInfo(rack=r1, hostname=null), bookie14:3181=BookieInfo(rack=r1, hostname=null), bookie15:3181=BookieInfo(rack=r1, hostname=null)}"
   "g2    {bookie21:3181=BookieInfo(rack=r2, hostname=null), bookie22:3181=BookieInfo(rack=r2, hostname=null), bookie23:3181=BookieInfo(rack=r2, hostname=null), bookie24:3181=BookieInfo(rack=r2, hostname=null), bookie25:3181=BookieInfo(rack=r2, hostname=null)}"
   "g3    {bookie31:3181=BookieInfo(rack=r3, hostname=null), bookie32:3181=BookieInfo(rack=r3, hostname=null), bookie33:3181=BookieInfo(rack=r3, hostname=null), bookie34:3181=BookieInfo(rack=r3, hostname=null), bookie35:3181=BookieInfo(rack=r3, hostname=null)}"
   ```
   
   but the bookkeeper log show the rack info doesn't match the config **r1 r2 r3**
   ```
   14:19:00.139 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie14:3181
   14:19:00.139 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie32:3181
   14:19:00.139 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie35:3181
   14:19:00.140 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie11:3181
   14:19:00.140 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie22:3181
   14:19:00.140 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie12:3181
   14:19:00.141 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie25:3181
   14:19:00.141 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie33:3181
   14:19:00.141 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie23:3181
   14:19:00.142 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie15:3181
   14:19:00.142 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie31:3181
   14:19:00.142 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie24:3181
   14:19:00.143 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie21:3181
   14:19:00.143 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie34:3181
   14:19:00.143 [BookKeeperClientScheduler-OrderedScheduler-0-0] INFO  org.apache.bookkeeper.net.NetworkTopologyImpl - Adding a new node: /default-rack/bookie13:3181
   ```
   
   ### Modifications
   
   1. chang the rack config to the bookkeeper's zookeeper.
   
   
   


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

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-849402745


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl closed pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl closed pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715


   


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

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



[GitHub] [pulsar] hangc0276 commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r644867291



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -224,10 +227,21 @@ public static void main(String[] args) throws Exception {
             initializer.initializeCluster(bkMetadataServiceUri.getUri(), arguments.numStreamStorageContainers);
         }
 
-        if (!localStore.exists(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH).get()) {
-            createMetadataNode(localStore, ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH, "{}".getBytes());
+        // exist the existing-bk-metadata-service-uri or bookkeeper-metadata-service-uri,
+        // should create metadata on the bookkeeper side
+        if (existBk) {
+            String bkZKStr = ZkUtils.getBookieZkConnect(uriStr);
+            MetadataStoreExtended bookieStore = initMetadataStore(bkZKStr, arguments.zkSessionTimeoutMillis);

Review comment:
       the created bookieStore should close in the end.

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1022,6 +1025,28 @@ public ZooKeeper getZkClient() {
         return this.localZooKeeperConnectionProvider.getLocalZooKeeper();
     }
 
+    // get bookkeeper's zookeeper
+    public ZooKeeper getBookieZkClient() {

Review comment:
       the `bookieZkClient` will be change root to ip:port/ledgers? and the following operation such as `bookieZk().setData(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH,...)` will set data to ip:port/ledgers/bookies path?

##########
File path: pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkUtils.java
##########
@@ -121,4 +124,45 @@ public static String getParentForPath(final String path) {
         final String parentPath = sb.toString();
         return (parentPath.length() == 0) ? null : parentPath;
     }
+
+
+    /**
+     * Returns the trimmed "/ledgers" string
+     *
+     * @param path connect path sting
+     * @return the trimmed "/ledgers" string
+     */
+    public static final String trimLedgersDefaultRootPath(String path) {
+        String rs = path;
+        if (StringUtils.endsWith(path, "/")) {
+            rs = StringUtils.substringBeforeLast(path, "/");
+        }
+        // defalut "/ledgers" need to trim
+        if (StringUtils.endsWith(path, LEDGERS_DEFAULT_ROOT_PATH)) {

Review comment:
       It shouldn't assume the ledger's root path is `/ledgers`, that's just default value in bookkeeper.conf, it maybe changed by bookkeeper.conf or broker.conf. 
   
   Last but not least, `ZkBookieRackAffinityMapping` will be used for bookkeeper client and bookkeeper auto recovery, it will call  `trimLedgersDefaultRootPath`, and it shouldn't just trim the default value.

##########
File path: pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZookeeperServerTest.java
##########
@@ -41,18 +41,18 @@ public ZookeeperServerTest(int zkPort) throws IOException {
         if (!zkTmpDir.delete() || !zkTmpDir.mkdir()) {
             throw new IOException("Couldn't create zk directory " + zkTmpDir);
         }
+        // Allow all commands on ZK control port
+        System.setProperty("zookeeper.4lw.commands.whitelist", "*");

Review comment:
       +1




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

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-849492138


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r640246494



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -92,6 +96,36 @@ protected ZooKeeperCache localZkCache() {
         return pulsar().getLocalZkCache();
     }
 
+    // bookkeeper zookeeper
+    protected ZooKeeper bookieZk() {
+        ZooKeeper zkClient = null;
+
+        ServiceConfiguration config = pulsar().getConfiguration();
+
+        String bookkeeperMetadataServiceUri = config.getBookkeeperMetadataServiceUri();
+        URI uri = URI.create(bookkeeperMetadataServiceUri);
+        String bookieZkConnect = StringUtils.replace(uri.getAuthority(), ";", ",") + uri.getPath();
+
+        int zkTimeout = (int) config.getZooKeeperSessionTimeoutMillis();
+        try {
+            zkClient = ZooKeeperClient.newBuilder().connectString(bookieZkConnect)

Review comment:
       yeah, but it depends on the refactoring of the "ZkIsolatedBookieEnsemblePlacementPolicy" and "ZkBookieRackAffinityMapping" first.




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

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



[GitHub] [pulsar] eolivelli commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1022,6 +1025,28 @@ public ZooKeeper getZkClient() {
         return this.localZooKeeperConnectionProvider.getLocalZooKeeper();
     }
 
+    // get bookkeeper's zookeeper
+    public ZooKeeper getBookieZkClient() {

Review comment:
       I would name this method "createBookieZkClient" because you are actually creating it

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1022,6 +1025,28 @@ public ZooKeeper getZkClient() {
         return this.localZooKeeperConnectionProvider.getLocalZooKeeper();
     }
 
+    // get bookkeeper's zookeeper
+    public ZooKeeper getBookieZkClient() {
+        ZooKeeper zkClient = null;
+        String bookkeeperMetadataServiceUri = config.getBookkeeperMetadataServiceUri();
+        if (StringUtils.isBlank(bookkeeperMetadataServiceUri)) {
+            bookkeeperMetadataServiceUri = PulsarService.bookieMetadataServiceUri(config);
+        }
+
+        URI uri = URI.create(bookkeeperMetadataServiceUri);
+        String path = ZkUtils.trimLedgersDefaultRootPath(uri.getPath());
+        String bookieZkConnect = StringUtils.replace(uri.getAuthority(), ";", ",") + path;
+
+        int zkTimeout = (int) config.getZooKeeperSessionTimeoutMillis();
+        try {
+            zkClient = ZooKeeperClient.newBuilder().connectString(bookieZkConnect)
+                    .sessionTimeoutMs(zkTimeout).build();
+        } catch (Exception e) {
+            LOG.error("Error creating bookie zookeeper client with {} for bookie.", bookieZkConnect, e);

Review comment:
       in this case we are returning `null` 
   should we throw an exception and fail ?

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -224,10 +229,22 @@ public static void main(String[] args) throws Exception {
             initializer.initializeCluster(bkMetadataServiceUri.getUri(), arguments.numStreamStorageContainers);
         }
 
-        if (!localStore.exists(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH).get()) {
-            createMetadataNode(localStore, ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH, "{}".getBytes());
+        // exist the existing-bk-metadata-service-uri or bookkeeper-metadata-service-uri,
+        // should create metadata on the bookkeeper side
+        if (existBk) {
+            URI bkUri = URI.create(uriStr);
+            String bkZKStr = StringUtils.replace(bkUri.getAuthority(), ";", ",") + bkUri.getPath();

Review comment:
       +1

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1022,6 +1025,28 @@ public ZooKeeper getZkClient() {
         return this.localZooKeeperConnectionProvider.getLocalZooKeeper();
     }
 
+    // get bookkeeper's zookeeper
+    public ZooKeeper getBookieZkClient() {
+        ZooKeeper zkClient = null;
+        String bookkeeperMetadataServiceUri = config.getBookkeeperMetadataServiceUri();
+        if (StringUtils.isBlank(bookkeeperMetadataServiceUri)) {
+            bookkeeperMetadataServiceUri = PulsarService.bookieMetadataServiceUri(config);
+        }
+
+        URI uri = URI.create(bookkeeperMetadataServiceUri);
+        String path = ZkUtils.trimLedgersDefaultRootPath(uri.getPath());
+        String bookieZkConnect = StringUtils.replace(uri.getAuthority(), ";", ",") + path;

Review comment:
       this line requires to use a common utility method as 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.

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



[GitHub] [pulsar] rdhabalia commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r640229889



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -224,10 +229,22 @@ public static void main(String[] args) throws Exception {
             initializer.initializeCluster(bkMetadataServiceUri.getUri(), arguments.numStreamStorageContainers);
         }
 
-        if (!localStore.exists(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH).get()) {
-            createMetadataNode(localStore, ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH, "{}".getBytes());
+        // exist the existing-bk-metadata-service-uri or bookkeeper-metadata-service-uri,
+        // should create metadata on the bookkeeper side
+        if (existBk) {
+            URI bkUri = URI.create(uriStr);
+            String bkZKStr = StringUtils.replace(bkUri.getAuthority(), ";", ",") + bkUri.getPath();

Review comment:
       it should go to some util class to reuse it and we should add example and documentation on it.,




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

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



[GitHub] [pulsar] yangl commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r642972693



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1022,6 +1025,28 @@ public ZooKeeper getZkClient() {
         return this.localZooKeeperConnectionProvider.getLocalZooKeeper();
     }
 
+    // get bookkeeper's zookeeper
+    public ZooKeeper getBookieZkClient() {
+        ZooKeeper zkClient = null;
+        String bookkeeperMetadataServiceUri = config.getBookkeeperMetadataServiceUri();
+        if (StringUtils.isBlank(bookkeeperMetadataServiceUri)) {
+            bookkeeperMetadataServiceUri = PulsarService.bookieMetadataServiceUri(config);
+        }
+
+        URI uri = URI.create(bookkeeperMetadataServiceUri);
+        String path = ZkUtils.trimLedgersDefaultRootPath(uri.getPath());
+        String bookieZkConnect = StringUtils.replace(uri.getAuthority(), ";", ",") + path;
+
+        int zkTimeout = (int) config.getZooKeeperSessionTimeoutMillis();
+        try {
+            zkClient = ZooKeeperClient.newBuilder().connectString(bookieZkConnect)
+                    .sessionTimeoutMs(zkTimeout).build();
+        } catch (Exception e) {
+            LOG.error("Error creating bookie zookeeper client with {} for bookie.", bookieZkConnect, e);

Review comment:
       yeah, 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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-852848928






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

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-855334064


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-849402745


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-850145489


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-852854646


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-854498312


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r647198868



##########
File path: pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkUtils.java
##########
@@ -121,4 +124,45 @@ public static String getParentForPath(final String path) {
         final String parentPath = sb.toString();
         return (parentPath.length() == 0) ? null : parentPath;
     }
+
+
+    /**
+     * Returns the trimmed "/ledgers" string
+     *
+     * @param path connect path sting
+     * @return the trimmed "/ledgers" string
+     */
+    public static final String trimLedgersDefaultRootPath(String path) {
+        String rs = path;
+        if (StringUtils.endsWith(path, "/")) {
+            rs = StringUtils.substringBeforeLast(path, "/");
+        }
+        // defalut "/ledgers" need to trim
+        if (StringUtils.endsWith(path, LEDGERS_DEFAULT_ROOT_PATH)) {

Review comment:
       > will set data to ip:port/ledgers/bookies path?
   
    yes, this will cause incompatibility, but i think we should do this correctly. the people can move the data to the bookie's zookeeper by manual.
   
   > ZkBookieRackAffinityMapping will be used for bookkeeper client and bookkeeper auto recovery, it will call trimLedgersDefaultRootPath, and it shouldn't just trim the default value.
   
   done, now use the bookie's Metadata Service Uri.




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

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-850145489


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r642972503



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1022,6 +1025,28 @@ public ZooKeeper getZkClient() {
         return this.localZooKeeperConnectionProvider.getLocalZooKeeper();
     }
 
+    // get bookkeeper's zookeeper
+    public ZooKeeper getBookieZkClient() {
+        ZooKeeper zkClient = null;
+        String bookkeeperMetadataServiceUri = config.getBookkeeperMetadataServiceUri();
+        if (StringUtils.isBlank(bookkeeperMetadataServiceUri)) {
+            bookkeeperMetadataServiceUri = PulsarService.bookieMetadataServiceUri(config);
+        }
+
+        URI uri = URI.create(bookkeeperMetadataServiceUri);
+        String path = ZkUtils.trimLedgersDefaultRootPath(uri.getPath());
+        String bookieZkConnect = StringUtils.replace(uri.getAuthority(), ";", ",") + path;

Review comment:
       done.

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1022,6 +1025,28 @@ public ZooKeeper getZkClient() {
         return this.localZooKeeperConnectionProvider.getLocalZooKeeper();
     }
 
+    // get bookkeeper's zookeeper
+    public ZooKeeper getBookieZkClient() {

Review comment:
       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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-850291245


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl closed pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl closed pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715


   


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-854498312


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-850478349


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-850291245


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-851411609


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-849492138


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r647194759



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -224,10 +227,21 @@ public static void main(String[] args) throws Exception {
             initializer.initializeCluster(bkMetadataServiceUri.getUri(), arguments.numStreamStorageContainers);
         }
 
-        if (!localStore.exists(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH).get()) {
-            createMetadataNode(localStore, ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH, "{}".getBytes());
+        // exist the existing-bk-metadata-service-uri or bookkeeper-metadata-service-uri,
+        // should create metadata on the bookkeeper side
+        if (existBk) {
+            String bkZKStr = ZkUtils.getBookieZkConnect(uriStr);
+            MetadataStoreExtended bookieStore = initMetadataStore(bkZKStr, arguments.zkSessionTimeoutMillis);

Review comment:
       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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-851760200


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-855250688


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-855334064


   /pulsarbot run-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.

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



[GitHub] [pulsar] rdhabalia commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r640230286



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -92,6 +96,36 @@ protected ZooKeeperCache localZkCache() {
         return pulsar().getLocalZkCache();
     }
 
+    // bookkeeper zookeeper
+    protected ZooKeeper bookieZk() {
+        ZooKeeper zkClient = null;
+
+        ServiceConfiguration config = pulsar().getConfiguration();
+
+        String bookkeeperMetadataServiceUri = config.getBookkeeperMetadataServiceUri();
+        URI uri = URI.create(bookkeeperMetadataServiceUri);
+        String bookieZkConnect = StringUtils.replace(uri.getAuthority(), ";", ",") + uri.getPath();
+
+        int zkTimeout = (int) config.getZooKeeperSessionTimeoutMillis();
+        try {
+            zkClient = ZooKeeperClient.newBuilder().connectString(bookieZkConnect)

Review comment:
       umm.. we are removing zk direct dependency from the pulsar broker so, not a good idea to introduce new one. we should use `MetadataStore` instead.




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

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-849241211


   > can you please add some tests that demonstrate the problem and will prevent regressions in the future ?
   
   Okay, I'll  do this in the next few days.


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

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-855329934


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-852854646


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r643616295



##########
File path: pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZookeeperServerTest.java
##########
@@ -41,18 +41,18 @@ public ZookeeperServerTest(int zkPort) throws IOException {
         if (!zkTmpDir.delete() || !zkTmpDir.mkdir()) {
             throw new IOException("Couldn't create zk directory " + zkTmpDir);
         }
+        // Allow all commands on ZK control port

Review comment:
       Because the previous `zkport` parameter does not take effect, but this test requires the specified port 2181.




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

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-852872730


   @eolivelli  I just did a simple little refactoring of the code to fix the zk fixed port issue when testing and the ZK connection missing destruction when Pulsar is closed.
   Thanks for the review suggestion, it was great.
   Now all the CI tests have passed, thank you for taking the time to help review again.
   
   @hangc0276  @codelipenghui  PTAL


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

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



[GitHub] [pulsar] yangl commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r642972805



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -224,10 +229,22 @@ public static void main(String[] args) throws Exception {
             initializer.initializeCluster(bkMetadataServiceUri.getUri(), arguments.numStreamStorageContainers);
         }
 
-        if (!localStore.exists(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH).get()) {
-            createMetadataNode(localStore, ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH, "{}".getBytes());
+        // exist the existing-bk-metadata-service-uri or bookkeeper-metadata-service-uri,
+        // should create metadata on the bookkeeper side
+        if (existBk) {
+            URI bkUri = URI.create(uriStr);
+            String bkZKStr = StringUtils.replace(bkUri.getAuthority(), ";", ",") + bkUri.getPath();

Review comment:
       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.

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



[GitHub] [pulsar] eolivelli commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

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



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/BookiesApiTest.java
##########
@@ -30,23 +30,29 @@
 import org.apache.pulsar.common.policies.data.BookieInfo;
 import org.apache.pulsar.common.policies.data.BookiesClusterInfo;
 import org.apache.pulsar.common.policies.data.BookiesRackConfiguration;
+import org.apache.pulsar.zookeeper.ZookeeperServerTest;
+import org.apache.zookeeper.server.ZooKeeperServer;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 @Slf4j
 @Test(groups = "broker")
 public class BookiesApiTest extends MockedPulsarServiceBaseTest {
+    private ZookeeperServerTest zks;
 
     @BeforeMethod
     @Override
     public void setup() throws Exception {
+        zks = new ZookeeperServerTest(2181);

Review comment:
       we should not use a fixed port

##########
File path: pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZookeeperServerTest.java
##########
@@ -41,18 +41,18 @@ public ZookeeperServerTest(int zkPort) throws IOException {
         if (!zkTmpDir.delete() || !zkTmpDir.mkdir()) {
             throw new IOException("Couldn't create zk directory " + zkTmpDir);
         }
+        // Allow all commands on ZK control port

Review comment:
       why are we anticipating the creation of these fields ?

##########
File path: pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZookeeperServerTest.java
##########
@@ -41,18 +41,18 @@ public ZookeeperServerTest(int zkPort) throws IOException {
         if (!zkTmpDir.delete() || !zkTmpDir.mkdir()) {
             throw new IOException("Couldn't create zk directory " + zkTmpDir);
         }
+        // Allow all commands on ZK control port
+        System.setProperty("zookeeper.4lw.commands.whitelist", "*");

Review comment:
       these are System properties, it is better to set them in a "static" block

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -626,7 +628,7 @@ public void start() throws PulsarServerException {
             this.bkClientFactory = newBookKeeperClientFactory();
 
             managedLedgerClientFactory = ManagedLedgerStorage.create(
-                config, localMetadataStore, getZkClient(), bkClientFactory, ioEventLoopGroup
+                config, localMetadataStore, createBookieZkClient(), bkClientFactory, ioEventLoopGroup

Review comment:
       Sorry, one last question.
   who is going to `close` this ZooKeeper handle ?
   
   when we use `getZkClient()` the ZooKeeper handle will be closed within the PulsarService lifecycle.
   but now we are going to create instances of it without never shutting them down.
   
   probably we have to store this reference to a field in PulsarService and dispose it in the "close()" method 




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

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



[GitHub] [pulsar] yangl closed pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl closed pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715


   


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] hangc0276 commented on a change in pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#discussion_r644867291



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -224,10 +227,21 @@ public static void main(String[] args) throws Exception {
             initializer.initializeCluster(bkMetadataServiceUri.getUri(), arguments.numStreamStorageContainers);
         }
 
-        if (!localStore.exists(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH).get()) {
-            createMetadataNode(localStore, ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH, "{}".getBytes());
+        // exist the existing-bk-metadata-service-uri or bookkeeper-metadata-service-uri,
+        // should create metadata on the bookkeeper side
+        if (existBk) {
+            String bkZKStr = ZkUtils.getBookieZkConnect(uriStr);
+            MetadataStoreExtended bookieStore = initMetadataStore(bkZKStr, arguments.zkSessionTimeoutMillis);

Review comment:
       the created bookieStore should close in the end.

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1022,6 +1025,28 @@ public ZooKeeper getZkClient() {
         return this.localZooKeeperConnectionProvider.getLocalZooKeeper();
     }
 
+    // get bookkeeper's zookeeper
+    public ZooKeeper getBookieZkClient() {

Review comment:
       the `bookieZkClient` will be change root to ip:port/ledgers? and the following operation such as `bookieZk().setData(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH,...)` will set data to ip:port/ledgers/bookies path?

##########
File path: pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkUtils.java
##########
@@ -121,4 +124,45 @@ public static String getParentForPath(final String path) {
         final String parentPath = sb.toString();
         return (parentPath.length() == 0) ? null : parentPath;
     }
+
+
+    /**
+     * Returns the trimmed "/ledgers" string
+     *
+     * @param path connect path sting
+     * @return the trimmed "/ledgers" string
+     */
+    public static final String trimLedgersDefaultRootPath(String path) {
+        String rs = path;
+        if (StringUtils.endsWith(path, "/")) {
+            rs = StringUtils.substringBeforeLast(path, "/");
+        }
+        // defalut "/ledgers" need to trim
+        if (StringUtils.endsWith(path, LEDGERS_DEFAULT_ROOT_PATH)) {

Review comment:
       It shouldn't assume the ledger's root path is `/ledgers`, that's just default value in bookkeeper.conf, it maybe changed by bookkeeper.conf or broker.conf. 
   
   Last but not least, `ZkBookieRackAffinityMapping` will be used for bookkeeper client and bookkeeper auto recovery, it will call  `trimLedgersDefaultRootPath`, and it shouldn't just trim the default value.

##########
File path: pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZookeeperServerTest.java
##########
@@ -41,18 +41,18 @@ public ZookeeperServerTest(int zkPort) throws IOException {
         if (!zkTmpDir.delete() || !zkTmpDir.mkdir()) {
             throw new IOException("Couldn't create zk directory " + zkTmpDir);
         }
+        // Allow all commands on ZK control port
+        System.setProperty("zookeeper.4lw.commands.whitelist", "*");

Review comment:
       +1




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

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-855329934


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-850478349


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-851760200


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl removed a comment on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl removed a comment on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-855250688


   /pulsarbot run-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.

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



[GitHub] [pulsar] yangl commented on pull request #10715: ZkIsolatedBookieEnsemblePlacementPolicy should use bookie zookeeper

Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #10715:
URL: https://github.com/apache/pulsar/pull/10715#issuecomment-855687726


   /pulsarbot run-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.

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