You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2021/03/11 18:37:58 UTC

[GitHub] [zookeeper] MuktiKrishnan opened a new pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

MuktiKrishnan opened a new pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626


   -waitforconnection option will make zk client wait for -timeout time to connect to zk server.  timeout time is 30ms by default but can be specified explicitly for a session using -timeout option in command line.


----------------------------------------------------------------
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] [zookeeper] arshadmohammad commented on pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#issuecomment-799484387


   Branch-3.6 changes are merged from https://github.com/apache/zookeeper/pull/1621 PR


----------------------------------------------------------------
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] [zookeeper] MuktiKrishnan commented on a change in pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
MuktiKrishnan commented on a change in pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#discussion_r591390180



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeperMain.java
##########
@@ -249,6 +261,17 @@ protected void connectToZK(String newHost) throws InterruptedException, IOExcept
             System.setProperty(ZKClientConfig.SECURE_CLIENT, "true");
             System.out.println("Secure connection is enabled");
         }
+        if (cl.getOption("waitforconnection") != null) {

Review comment:
       added documentation for waitforconnection




----------------------------------------------------------------
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] [zookeeper] arshadmohammad commented on a change in pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on a change in pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#discussion_r591332735



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeperMain.java
##########
@@ -249,6 +261,17 @@ protected void connectToZK(String newHost) throws InterruptedException, IOExcept
             System.setProperty(ZKClientConfig.SECURE_CLIENT, "true");
             System.out.println("Secure connection is enabled");
         }
+        if (cl.getOption("waitforconnection") != null) {

Review comment:
       Pls add documentation for waitforconnection in zookeeperCLI.md




----------------------------------------------------------------
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] [zookeeper] MuktiKrishnan commented on a change in pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
MuktiKrishnan commented on a change in pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#discussion_r591389625



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeperMain.java
##########
@@ -249,6 +261,17 @@ protected void connectToZK(String newHost) throws InterruptedException, IOExcept
             System.setProperty(ZKClientConfig.SECURE_CLIENT, "true");
             System.out.println("Secure connection is enabled");
         }
+        if (cl.getOption("waitforconnection") != null) {
+            connectLatch = new CountDownLatch(1);
+        }
+        int timeout = Integer.parseInt(cl.getOption("timeout"));
+        zk = new ZooKeeperAdmin(host, timeout, new MyWatcher(), readOnly);

Review comment:
       Removed the statement which was creating Zookeeper admin again




----------------------------------------------------------------
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] [zookeeper] arshadmohammad commented on a change in pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on a change in pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#discussion_r591291428



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeperMain.java
##########
@@ -249,6 +261,17 @@ protected void connectToZK(String newHost) throws InterruptedException, IOExcept
             System.setProperty(ZKClientConfig.SECURE_CLIENT, "true");
             System.out.println("Secure connection is enabled");
         }
+        if (cl.getOption("waitforconnection") != null) {
+            connectLatch = new CountDownLatch(1);
+        }
+        int timeout = Integer.parseInt(cl.getOption("timeout"));
+        zk = new ZooKeeperAdmin(host, timeout, new MyWatcher(), readOnly);

Review comment:
       No need to create ZooKeeperAdmin twice.
   Also move all these changes after line number 289




----------------------------------------------------------------
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] [zookeeper] arshadmohammad commented on pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#issuecomment-796954805


   @MuktiKrishnan  please raise another PR for branch-3.6


----------------------------------------------------------------
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] [zookeeper] asfgit closed pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626


   


----------------------------------------------------------------
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] [zookeeper] arshadmohammad commented on a change in pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on a change in pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#discussion_r591332735



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeperMain.java
##########
@@ -249,6 +261,17 @@ protected void connectToZK(String newHost) throws InterruptedException, IOExcept
             System.setProperty(ZKClientConfig.SECURE_CLIENT, "true");
             System.out.println("Secure connection is enabled");
         }
+        if (cl.getOption("waitforconnection") != null) {

Review comment:
       Pls add documentation in zookeeperCLI.md




----------------------------------------------------------------
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] [zookeeper] arshadmohammad closed pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
arshadmohammad closed pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626


   


----------------------------------------------------------------
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] [zookeeper] arshadmohammad commented on pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#issuecomment-796946445


   Comments given by @maoling in https://github.com/apache/zookeeper/pull/1621 are addressed in this PR.
   Test failures are not related to this PR.
   Merging it now.
   


----------------------------------------------------------------
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] [zookeeper] arshadmohammad commented on pull request #1626: ZOOKEEPER-1871: Add an option to zkCli to wait for connection before executing commands

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on pull request #1626:
URL: https://github.com/apache/zookeeper/pull/1626#issuecomment-796459166


   ping @maoling 


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