[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314588811
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperSaslClient.java
 ##
 @@ -77,7 +77,7 @@
  */
 @Deprecated
 public static boolean isEnabled() {
-return 
Boolean.valueOf(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
+return 
Boolean.getBoolean(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
 
 Review comment:
   Should never do refactor in this pr then.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314588811
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperSaslClient.java
 ##
 @@ -77,7 +77,7 @@
  */
 @Deprecated
 public static boolean isEnabled() {
-return 
Boolean.valueOf(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
+return 
Boolean.getBoolean(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
 
 Review comment:
   Should never do refactor then.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314588414
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperSaslClient.java
 ##
 @@ -77,7 +77,7 @@
  */
 @Deprecated
 public static boolean isEnabled() {
-return 
Boolean.valueOf(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
+return 
Boolean.getBoolean(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
 
 Review comment:
   Reverted as `Boolean.parseBoolean`. It should never be 
`Boolean.getBoolean("true"|"false")` which would nearly always returns false. 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314588183
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
 ##
 @@ -99,15 +99,16 @@ public FileTxnSnapLog(File dataDir, File snapDir) throws 
IOException {
 
 // by default create snap/log dirs, but otherwise complain instead
 // See ZOOKEEPER-1161 for more details
-boolean enableAutocreate = 
Boolean.valueOf(System.getProperty(ZOOKEEPER_DATADIR_AUTOCREATE, 
ZOOKEEPER_DATADIR_AUTOCREATE_DEFAULT));
+boolean enableAutocreate = Boolean.getBoolean(
 
 Review comment:
   This is a buggy change. Revert.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314580611
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -1268,11 +1256,7 @@ public void run() {
 } else if (e instanceof SocketException) {
 LOG.info("Socket error occurred: {}: {}", 
serverAddress, e.getMessage());
 } else {
-LOG.warn("Session 0x{} for server {}, unexpected 
error{}",
-Long.toHexString(getSessionId()),
-serverAddress,
-RETRY_CONN_MSG,
-e);
+LOG.warn("Session 0x{} for server {}, unexpected 
error{}", Long.toHexString(getSessionId()), serverAddress, RETRY_CONN_MSG, e);
 
 Review comment:
   It is possible that we enable `LineLength` rule on zookeeper-server that 
forbid every line exceed a preconfigure value. See ZOOKEEPER-3508 for it.
   
   We can tell an auto-formatter to break a line but it would introduce awful 
breaks as Enrico request to change.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314581508
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -925,15 +926,11 @@ else if (serverPath.length() > chrootPath.length())
  */
 try {
 if (packet.requestHeader.getXid() != replyHdr.getXid()) {
-packet.replyHeader.setErr(
-KeeperException.Code.CONNECTIONLOSS.intValue());
-throw new IOException("Xid out of order. Got Xid "
-+ replyHdr.getXid() + " with err " +
-+ replyHdr.getErr() +
-" expected Xid "
-+ packet.requestHeader.getXid()
-+ " for a packet with details: "
-+ packet );
+
packet.replyHeader.setErr(KeeperException.Code.CONNECTIONLOSS.intValue());
+throw new IOException("Xid out of order. Got Xid " + 
replyHdr.getXid()
+  + " with err " + +replyHdr.getErr()
 
 Review comment:
   Nice catch. 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314581323
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/Watcher.java
 ##
 @@ -35,34 +35,33 @@
  * This interface defines the possible states an Event may represent
  */
 @InterfaceAudience.Public
-public interface Event {
+interface Event {
+
 /**
  * Enumeration of states the ZooKeeper may be at the event
  */
 @InterfaceAudience.Public
-public enum KeeperState {
+enum KeeperState {
 /** Unused, this state is never generated by the server */
-@Deprecated
-Unknown (-1),
+@Deprecated Unknown(-1),
 
 Review comment:
   Nope. Reverted.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314580886
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java
 ##
 @@ -109,45 +111,43 @@
 int ADMIN = 1 << 4;
 
 int ALL = READ | WRITE | CREATE | DELETE | ADMIN;
+
 }
 
 @InterfaceAudience.Public
 public interface Ids {
+
 /**
  * This Id represents anyone.
  */
-public final Id ANYONE_ID_UNSAFE = new Id("world", "anyone");
+Id ANYONE_ID_UNSAFE = new Id("world", "anyone");
 
 Review comment:
   Any field in an interface must be `public static final`. There is a rule 
`redundant modifier` check this.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314580742
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
 ##
 @@ -200,9 +195,7 @@ public static void visitSubTreeDFS(ZooKeeper zk, final 
String path, boolean watc
 }
 
 @SuppressWarnings("unchecked")
-private static void visitSubTreeDFSHelper(ZooKeeper zk, final String path,
-boolean watch, StringCallback cb)
-throws KeeperException, InterruptedException {
+private static void visitSubTreeDFSHelper(ZooKeeper zk, final String path, 
boolean watch, StringCallback cb) throws KeeperException, InterruptedException {
 
 Review comment:
   This pattern can be resolve easier. Since chop down method declaration is a 
fixed pattern.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314580611
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -1268,11 +1256,7 @@ public void run() {
 } else if (e instanceof SocketException) {
 LOG.info("Socket error occurred: {}: {}", 
serverAddress, e.getMessage());
 } else {
-LOG.warn("Session 0x{} for server {}, unexpected 
error{}",
-Long.toHexString(getSessionId()),
-serverAddress,
-RETRY_CONN_MSG,
-e);
+LOG.warn("Session 0x{} for server {}, unexpected 
error{}", Long.toHexString(getSessionId()), serverAddress, RETRY_CONN_MSG, e);
 
 Review comment:
   It is possible that we enable `LineBreak` rule on zookeeper-server that 
forbid every line exceed a preconfigure value. See ZOOKEEPER-3508 for it.
   
   We can tell an auto-formatter to break a line but it would introduce awful 
breaks as Enrico request to change.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314554375
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -433,22 +417,22 @@ public WatcherSetEventPair(Set watchers, 
WatchedEvent event) {
  */
 private static String makeThreadName(String suffix) {
 String name = Thread.currentThread().getName().
-replaceAll("-EventThread", "");
+  
replaceAll("-EventThread", "");
 
 Review comment:
   There is no checkstyle rule guard this and I accidentally introduce it when 
auto-formatting...
   
   I will check these changes for now and try to find a rule to forbid this 
pattern. However, given how to break lines is nearly arbitrary by the meaning 
of the codes, it is hard to express such a rule.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314416189
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314416189
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314412010
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314411673
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java
 ##
 @@ -112,15 +109,29 @@ private PortForwarder setUp(final int omProxyPort) 
throws IOException {
 
 OM_PORT = PortAssignment.unique();
 
-String quorumCfgSection =
-"server.1=127.0.0.1:" + (PORT_QP1)
-+ ":" + (PORT_QP_LE1) + ";" +  CLIENT_PORT_QP1
-+ "\nserver.2=127.0.0.1:" + (PORT_QP2)
-+ ":" + (PORT_QP_LE2) + ";" + CLIENT_PORT_QP2
-+ "\nserver.3=127.0.0.1:"
-+ (PORT_OBS)+ ":" + (PORT_OBS_LE) + ":observer" + ";" 
+ CLIENT_PORT_OBS;
+String quorumCfgSection = "server.1=127.0.0.1:"
 
 Review comment:
   Yes. It requires another pass about line break things. I will take care of 
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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314405124
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperSaslClient.java
 ##
 @@ -442,11 +428,9 @@ public boolean clientTunneledAuthenticationInProgress() {
 }
 
 // 2. SASL authentication has succeeded or failed..
-if (!gotLastPacket) {
-// ..but still in progress, because there is a final SASL
-// message from server which must be received.
-return true;
-}
+// ..but still in progress, because there is a final SASL
+// message from server which must be received.
+return !gotLastPacket;
 
 Review comment:
   Reverting...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314404402
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/ZookeeperBanner.java
 ##
 @@ -26,21 +26,12 @@
  */
 public class ZookeeperBanner {
 
-private static final String[] BANNER = {"",
-"  __  _   
   ",
-" |___  / | |  
   ",
-"/ /___ ___   | | __   ______   _ __ ___   _ 
__   ",
-"   / // _ \\   / _ \\  | |/ /  / _ \\  / _ \\ | '_ \\   / _ 
\\ | '__|",
-"  / /__  | (_) | | (_) | |   <  |  __/ |  __/ | |_) | |  __/ | |  
  " ,
-" /_|  \\___/   \\___/  |_|\\_\\  \\___|  \\___| | .__/   
\\___| |_|",
-"  | | 
" ,
-"  |_| 
",
-""
-};
+private static final String[] BANNER = {"", "  __  _   
   ", " |___  / | | 
", "/ /___ ___   | | __   ___
___   _ __ ___   _ __   ", "   / // _ \\   / _ \\  | |/ /  / _ \\  / _ 
\\ | '_ \\   / _ \\ | '__|", "  / /__  | (_) | | (_) | |   <  |  __/ |  __/ | 
|_) | |  __/ | |", " /_|  \\___/   \\___/  |_|\\_\\  \\___|  \\___| | 
.__/   \\___| |_|", "  | |  
   ", "  |_|
 ", ""};
 
 Review comment:
   Yes of course.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314404109
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/ServerAdminClient.java
 ##
 @@ -192,9 +192,12 @@ public static void setTraceMask(String host, int port, 
String traceMaskStr) {
 int rc = is.read(resBytes);
 ByteBuffer res = ByteBuffer.wrap(resBytes);
 long retv = res.getLong();
-System.out.println("rc=" + rc + " retv=0"
-+ Long.toOctalString(retv) + " masks=0"
-+ Long.toOctalString(traceMask));
+System.out.println("rc="
+   + rc
 
 Review comment:
   Reformatted


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314365190
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
 ##
 @@ -131,12 +128,7 @@ private Bootstrap configureBootstrapAllocator(Bootstrap 
bootstrap) {
 void connect(InetSocketAddress addr) throws IOException {
 firstConnect = new CountDownLatch(1);
 
-Bootstrap bootstrap = new Bootstrap()
-.group(eventLoopGroup)
-.channel(NettyUtils.nioOrEpollSocketChannel())
-.option(ChannelOption.SO_LINGER, -1)
-.option(ChannelOption.TCP_NODELAY, true)
-.handler(new ZKClientPipelineFactory(addr.getHostString(), 
addr.getPort()));
+Bootstrap bootstrap = new 
Bootstrap().group(eventLoopGroup).channel(NettyUtils.nioOrEpollSocketChannel()).option(ChannelOption.SO_LINGER,
 -1).option(ChannelOption.TCP_NODELAY, true).handler(new 
ZKClientPipelineFactory(addr.getHostString(), addr.getPort()));
 
 Review comment:
   Reverting...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314364487
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -232,17 +232,7 @@ public String toString() {
 
 SocketAddress local = 
sendThread.getClientCnxnSocket().getLocalSocketAddress();
 SocketAddress remote = 
sendThread.getClientCnxnSocket().getRemoteSocketAddress();
-sb
-.append("sessionid:0x").append(Long.toHexString(getSessionId()))
-.append(" local:").append(local)
-.append(" remoteserver:").append(remote)
-.append(" lastZxid:").append(lastZxid)
-.append(" xid:").append(xid)
-.append(" 
sent:").append(sendThread.getClientCnxnSocket().getSentCount())
-.append(" 
recv:").append(sendThread.getClientCnxnSocket().getRecvCount())
-.append(" queuedpkts:").append(outgoingQueue.size())
-.append(" pendingresp:").append(pendingQueue.size())
-.append(" queuedevents:").append(eventThread.waitingEvents.size());
+
sb.append("sessionid:0x").append(Long.toHexString(getSessionId())).append(" 
local:").append(local).append(" remoteserver:").append(remote).append(" 
lastZxid:").append(lastZxid).append(" xid:").append(xid).append(" 
sent:").append(sendThread.getClientCnxnSocket().getSentCount()).append(" 
recv:").append(sendThread.getClientCnxnSocket().getRecvCount()).append(" 
queuedpkts:").append(outgoingQueue.size()).append(" 
pendingresp:").append(pendingQueue.size()).append(" 
queuedevents:").append(eventThread.waitingEvents.size());
 
 Review comment:
   Reverting...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312900302
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/HostProvider.java
 ##
 @@ -18,58 +18,60 @@
 
 package org.apache.zookeeper.client;
 
-import org.apache.yetus.audience.InterfaceAudience;
-
 import java.net.InetSocketAddress;
 import java.util.Collection;
+import org.apache.yetus.audience.InterfaceAudience;
 
 /**
  * A set of hosts a ZooKeeper client should connect to.
- * 
- * Classes implementing this interface must guarantee the following:
- * 
- * * Every call to next() returns an InetSocketAddress. So the iterator never
- * ends.
- * 
- * * The size() of a HostProvider may never be zero.
- * 
- * A HostProvider must return resolved InetSocketAddress instances on next() 
if the next address is resolvable.
+ *
+ * Classes implementing this interface must guarantee the following:
+ *
+ * 
+ * Every call to next() returns an InetSocketAddress. So the iterator 
never ends.
+ * The size() of a HostProvider may never be zero.
+ * 
+ *
+ * A HostProvider must return resolved InetSocketAddress instances on 
next() if the next address is resolvable.
  * In that case, it's up to the HostProvider, whether it returns the next 
resolvable address in the list or return
  * the next one as UnResolved.
- * 
- * Different HostProvider could be imagined:
- * 
- * * A HostProvider that loads the list of Hosts from an URL or from DNS 
- * * A HostProvider that re-resolves the InetSocketAddress after a timeout. 
- * * A HostProvider that prefers nearby hosts.
+ *
+ * ifferent HostProvider could be imagined:
 
 Review comment:
   Fixed.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312868387
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
 ##
 @@ -188,24 +179,27 @@ public static EphemeralType get(long ephemeralOwner) {
 }
 
 /**
- * Make sure the given server ID is compatible with the current extended 
ephemeral setting
+ * Make sure the given server ID is compatible with the current extended 
ephemeral setting.
  *
  * @param serverId Server ID
  * @throws RuntimeException extendedTypesEnabled is true but Server ID is 
too large
  */
 public static void validateServerId(long serverId) {
-// TODO: in the future, serverId should be validated for all cases, 
not just the extendedEphemeralTypesEnabled case
+// TODO: in the future, serverId should be validated for all cases,
 
 Review comment:
   Support. This is also how akka community tracked TODOs and it was a good 
practice yet.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312868023
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
 ##
 @@ -89,8 +78,9 @@ public long toEphemeralOwner(long ttl) {
 if ((ttl > TTL.maxValue()) || (ttl <= 0)) {
 throw new IllegalArgumentException("ttl must be positive and 
cannot be larger than: " + TTL.maxValue());
 }
-//noinspection PointlessBitwiseExpression
 
 Review comment:
   Invalid any more.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312866880
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/HostProvider.java
 ##
 @@ -18,58 +18,60 @@
 
 package org.apache.zookeeper.client;
 
-import org.apache.yetus.audience.InterfaceAudience;
-
 import java.net.InetSocketAddress;
 import java.util.Collection;
+import org.apache.yetus.audience.InterfaceAudience;
 
 /**
  * A set of hosts a ZooKeeper client should connect to.
- * 
- * Classes implementing this interface must guarantee the following:
- * 
- * * Every call to next() returns an InetSocketAddress. So the iterator never
- * ends.
- * 
- * * The size() of a HostProvider may never be zero.
- * 
- * A HostProvider must return resolved InetSocketAddress instances on next() 
if the next address is resolvable.
+ *
+ * Classes implementing this interface must guarantee the following:
+ *
+ * 
+ * Every call to next() returns an InetSocketAddress. So the iterator 
never ends.
+ * The size() of a HostProvider may never be zero.
+ * 
+ *
+ * A HostProvider must return resolved InetSocketAddress instances on 
next() if the next address is resolvable.
  * In that case, it's up to the HostProvider, whether it returns the next 
resolvable address in the list or return
  * the next one as UnResolved.
- * 
- * Different HostProvider could be imagined:
- * 
- * * A HostProvider that loads the list of Hosts from an URL or from DNS 
- * * A HostProvider that re-resolves the InetSocketAddress after a timeout. 
- * * A HostProvider that prefers nearby hosts.
+ *
+ * ifferent HostProvider could be imagined:
 
 Review comment:
   Nice catch. Fixing...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312865164
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
 ##
 @@ -184,25 +191,31 @@ public static String validateFileInput(String filePath) {
 /**
  * Visits the subtree with root as given path and calls the passed 
callback with each znode
  * found during the search. It performs a depth-first, pre-order traversal 
of the tree.
- * 
- * Important: This is not an atomic snapshot of the tree 
ever, but the
+ *
+ * Important: This is not an atomic snapshot of the tree 
ever, but the
  * state as it exists across multiple RPCs from zkClient to the ensemble.
  * For practical purposes, it is suggested to bring the clients to the 
ensemble
  * down (i.e. prevent writes to pathRoot) to 'simulate' a snapshot 
behavior.
  */
-public static void visitSubTreeDFS(ZooKeeper zk, final String path, 
boolean watch,
-StringCallback cb) throws KeeperException, InterruptedException {
+public static void visitSubTreeDFS(
+ZooKeeper zk,
+final String path,
+boolean watch,
+StringCallback cb
+) throws KeeperException, InterruptedException {
 PathUtils.validatePath(path);
 
 zk.getData(path, watch, null);
 cb.processResult(Code.OK.intValue(), path, null, path);
 visitSubTreeDFSHelper(zk, path, watch, cb);
 }
 
-@SuppressWarnings("unchecked")
 
 Review comment:
   Yes. This is validated by the build cycle as we enable `-Werror`.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312864700
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -928,12 +925,12 @@ else if (serverPath.length() > chrootPath.length())
 packet.replyHeader.setErr(
 KeeperException.Code.CONNECTIONLOSS.intValue());
 throw new IOException("Xid out of order. Got Xid "
-+ replyHdr.getXid() + " with err " +
-+ replyHdr.getErr() +
-" expected Xid "
++ replyHdr.getXid() + " with err "
++ +replyHdr.getErr()
 
 Review comment:
   Here is a rule `+' should be on a new line`. For cleanup I guess you mean 
replace it with `String.format`? It would be a follow up to unify the 
log/Exception format and generation like that for javadoc(ZOOKEEPER-3469). This 
pull request would be a clean start point for all of this request but we should 
prevent it from too complex IMHO.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312863142
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -160,7 +157,7 @@
 
 private long sessionId;
 
-private byte sessionPasswd[] = new byte[16];
+private byte[] sessionPasswd;
 
 Review comment:
   Right. Always be initialized in constructor.


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:
us...@infra.apache.org


With regards,
Apache Git Services