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

2019-08-15 Thread GitBox
eolivelli 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_r314591200
 
 

 ##
 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:
   @hanm just to be clear, checkstyle does not format the code, it is only a 
set of rules.
   @TisonKun is using a bunch of tools like scripts/IDEs to reformat the code 
in a way that makes checkstyle happy.
   Actually we decided to disable the only rule about line length, so 
overflowing lines could be left as they were originally.
   I guess that @TisonKun's tools anyway want to restrict the max line length


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

 ##
 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:
   In fact I locally turn on `LineLength` and see that there is about 200 files 
break the rules, a pass to take care of it is ok for this pr. But not all of 
them have a reasonable break down strategy to apply on. Some of them are just 
because too much indent level so break line length 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_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] hanm commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
hanm 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_r314570228
 
 

 ##
 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:
   Remove redundant `+`


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] hanm commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
hanm 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_r314571465
 
 

 ##
 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:
   This line after reformatting looks awfully long. Is it possible to teach 
check style so it insert line breaks when a single line exceeds certain 
preconfigured width? 


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] hanm commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
hanm 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_r314574017
 
 

 ##
 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:
   very long line 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:
us...@infra.apache.org


With regards,
Apache Git Services


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

2019-08-15 Thread GitBox
hanm 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_r314574170
 
 

 ##
 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:
   this removed `final`.. intended?


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] hanm commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
hanm 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_r314573903
 
 

 ##
 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:
   Is this intended that we want to put annotation and annotated in the same 
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:
us...@infra.apache.org


With regards,
Apache Git Services


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

2019-08-15 Thread GitBox
hanm 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_r314555258
 
 

 ##
 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:
   ok. was asking just in case this was a systematic problem instead of an 
accident.


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] hanm commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
hanm commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration 
on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521849325
 
 
   reviewing, feedback on the way.


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] hanm commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
hanm 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_r314553232
 
 

 ##
 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:
   So, check style format the line to make it look worse (comparing to old 
code)? Was it a check style bug or a by design?


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] eolivelli commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle 
configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521763584
 
 
   I think we are on our way.
   Ping me when you are done
   
   If the other guys (@hanm @enixon) that started a review approve this change 
I will be happy to commit it as soon as possible


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314458370
 
 

 ##
 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 removed a comment on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun removed a comment on issue #1049: ZOOKEEPER-3475 Enable Checkstyle 
configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521730229
 
 
   Also it is a pain to handle multiple log formats which also make formatting 
harder. I would propose a best effort(i.e. no rule to enforce this) on the fly 
reformatting on log style(string concat style) that when the line is too long 
and variable interception matters, use the pattern below.
   
   ```
   log.info("...{} ...{}", arg1, arg2)
   ```
   
   instead of string concat. This also works on `String.format`.


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] mayawang commented on issue #984: ZOOKEEPER-3427: Introduce SnapshotComparer that assists debugging with snapshots.

2019-08-15 Thread GitBox
mayawang commented on issue #984: ZOOKEEPER-3427: Introduce SnapshotComparer 
that assists debugging with snapshots.
URL: https://github.com/apache/zookeeper/pull/984#issuecomment-521741636
 
 
   > @hanm Pull requests ownership cannot be changed. You can grant write 
access for your fork and he'll able to push changes on this branch. Once the 
patch is committed, you can revoke access.
   @anmolnar Great idea. @hanm did exactly this yesterday I guess that's what 
he meant by "other ways of collaborating on PR." Thanks a lot. :)


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] mayawang commented on a change in pull request #984: ZOOKEEPER-3427: Introduce SnapshotComparer that assists debugging with snapshots.

2019-08-15 Thread GitBox
mayawang commented on a change in pull request #984: ZOOKEEPER-3427: Introduce 
SnapshotComparer that assists debugging with snapshots.
URL: https://github.com/apache/zookeeper/pull/984#discussion_r314432989
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/SnapshotComparer.java
 ##
 @@ -0,0 +1,461 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Scanner;
+import java.util.zip.Adler32;
+import java.util.zip.CheckedInputStream;
+
+import org.apache.commons.cli.BasicParser;
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.OptionBuilder;
+import org.apache.commons.cli.ParseException;
+import org.apache.jute.BinaryInputArchive;
+import org.apache.jute.InputArchive;
+import org.apache.zookeeper.server.persistence.FileSnap;
+
+/**
+ * Compare two data tree snapshots, output information about the delta.
+ * Only outputs information about permanent nodes, ignoring both sessions
+ * and ephemeral nodes.
+ */
+public class SnapshotComparer {
+  private final Options options;
+  private static final String leftOption = "left";
+  private static final String rightOption = "right";
+  private static final String byteThresholdOption = "bytes";
+  private static final String nodeThresholdOption = "nodes";
+  private static final String debugOption = "debug";
+  private static final String interactiveOption = "interactive";
+
+  @SuppressWarnings("static")
+  private SnapshotComparer() {
+options = new Options();
+options.addOption(
+OptionBuilder
+.hasArg()
+.isRequired(true)
+.withLongOpt(leftOption)
+.withDescription("the left snapshot file")
+.withArgName("LEFT")
+.withType(File.class)
+.create("l"));
+options.addOption(
+OptionBuilder
+.hasArg()
+.isRequired(true)
+.withLongOpt(rightOption)
+.withDescription("the right snapshot file")
+.withArgName("RIGHT")
+.withType(File.class)
+.create("r"));
+options.addOption(
+OptionBuilder
+.hasArg()
+.isRequired(true)
+.withLongOpt(byteThresholdOption)
+.withDescription("the node data delta size threshold, in bytes, 
for printing the node")
+.withArgName("BYTETHRESHOLD")
+.withType(File.class)
+.create("b"));
+options.addOption(
+OptionBuilder
+.hasArg()
+.isRequired(true)
+.withLongOpt(nodeThresholdOption)
+.withDescription("the descendant node delta size threshold, in 
nodes, for printing the node")
+.withArgName("NODETHRESHOLD")
+.withType(String.class)
+.create("n"));
+options.addOption(
+OptionBuilder
+.hasArg()
+.withLongOpt(debugOption)
+.withDescription("use debug output")
+.withArgName("DEBUG")
+.withType(String.class)
+.create("d"));
+options.addOption(
+OptionBuilder
+.hasArg()
+.withLongOpt(interactiveOption)
+.withDescription("interactive mode")
+.withArgName("INTERACTIVE")
+.withType(String.class)
+.create("i"));
+  }
+
+  private void usage() {
+HelpFormatter help = new HelpFormatter();
+
+help.printHelp(
+120,
+"java -cp  " + SnapshotFormatter.class.getName(),
+"",
+options,
 
 Review comment:
   @maoling Thanks! I'll work with @hanm to continue improving it.


This is an automated message from the Apache Git Service.
To 

[GitHub] [zookeeper] TisonKun edited a comment on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun edited a comment on issue #1049: ZOOKEEPER-3475 Enable Checkstyle 
configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521730229
 
 
   Also it is a pain to handle multiple log formats which also make formatting 
harder. I would propose a best effort(i.e. no rule to enforce this) on the fly 
reformatting on log style(string concat style) that when the line is too long 
and variable interception matters, use the pattern below.
   
   ```
   log.info("...{} ...{}", arg1, arg2)
   ```
   
   instead of string concat. This also works on `String.format`.


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 issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle 
configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521730229
 
 
   Also there is a pain multiple log format which also make formatting harder. 
I would propose a best effort on the fly reformatting on log style(string 
concat style) that when the line is too long and variable interception matters, 
use the pattern below.
   
   ```
   log.info("...{} ...{}", arg1, arg2)
   ```
   
   instead of string concat. This also works on `String.format`.


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 edited a comment on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun edited a comment on issue #1049: ZOOKEEPER-3475 Enable Checkstyle 
configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521730229
 
 
   Also it is a pain to handle multiple log formats which also make formatting 
harder. I would propose a best effort on the fly reformatting on log 
style(string concat style) that when the line is too long and variable 
interception matters, use the pattern below.
   
   ```
   log.info("...{} ...{}", arg1, arg2)
   ```
   
   instead of string concat. This also works on `String.format`.


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 issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle 
configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521729375
 
 
   Line breaks, i.e., confusing line formats request changed above, suffer from 
lack usage of local variables, which causes quite a lot long line without 
semantic awareness. I would like to add some local variables to help 
formatting. Since it is limited in local variables, it would not conflict 
outside world.


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_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] eolivelli commented on issue #993: Enhance Mavenized Make C client

2019-08-15 Thread GitBox
eolivelli commented on issue #993: Enhance Mavenized Make C client
URL: https://github.com/apache/zookeeper/pull/993#issuecomment-521726503
 
 
   @phunt 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] eolivelli commented on a change in pull request #1056: ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+

2019-08-15 Thread GitBox
eolivelli commented on a change in pull request #1056: ZOOKEEPER-3495: fix 
SnapshotDigestTest to work with JDK12+
URL: https://github.com/apache/zookeeper/pull/1056#discussion_r314412180
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/util/DigestCalculator.java
 ##
 @@ -37,19 +37,28 @@
 
 // The hardcoded digest version, should bump up this version whenever
 // we changed the digest method or fields.
-//
-// Defined it as Integer to make it able to be changed in test via 
reflection
-public static final Integer DIGEST_VERSION = 2;
+private static final int DIGEST_VERSION = 2;
 
+public static final DigestCalculator DIGEST_CALCULATOR;
 public static final String ZOOKEEPER_DIGEST_ENABLED = 
"zookeeper.digest.enabled";
-private static boolean digestEnabled;
 
 static {
-digestEnabled = Boolean.parseBoolean(
+boolean digestEnabled = Boolean.parseBoolean(
 System.getProperty(ZOOKEEPER_DIGEST_ENABLED, "true"));
 LOG.info("{} = {}", ZOOKEEPER_DIGEST_ENABLED, digestEnabled);
+DIGEST_CALCULATOR = new DigestCalculator(digestEnabled, 
DIGEST_VERSION);
 }
 
+
 
 Review comment:
   This is not final, why don't you add a setDigestEnabled method?
   Make it 'volatile'
   
   We won't lose much in performances


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] anmolnar commented on issue #984: ZOOKEEPER-3427: Introduce SnapshotComparer that assists debugging with snapshots.

2019-08-15 Thread GitBox
anmolnar commented on issue #984: ZOOKEEPER-3427: Introduce SnapshotComparer 
that assists debugging with snapshots.
URL: https://github.com/apache/zookeeper/pull/984#issuecomment-521724747
 
 
   @hanm Pull requests ownership cannot be changed. You can grant write access 
for your fork and he'll able to push changes on this branch. Once the patch is 
committed, you can revoke access.


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_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] anmolnar commented on a change in pull request #1048: ZOOKEEPER-3188: Improve resilience to network

2019-08-15 Thread GitBox
anmolnar commented on a change in pull request #1048: ZOOKEEPER-3188: Improve 
resilience to network
URL: https://github.com/apache/zookeeper/pull/1048#discussion_r314410325
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java
 ##
 @@ -657,11 +647,31 @@ public CommandResponse run(ZooKeeperServer zkServer, 
Map kwargs)
 TreeMap::new));
 }
 
+private String getMultiAddressString(QuorumPeer.QuorumServer qs) {
+return qs.addr.getAllAddresses().stream()
+.map(address -> getSingleAddressString(qs, address))
+.collect(Collectors.joining(","));
+}
+
+private String getSingleAddressString(QuorumPeer.QuorumServer qs, 
InetSocketAddress address) {
 
 Review comment:
   Perfect!
   


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314401708
 
 

 ##
 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314399707
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslTestUtil.java
 ##
 @@ -21,41 +21,49 @@
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
-
 import org.junit.Assert;
 
 public class SaslTestUtil extends ClientBase {
-  // The maximum time (in milliseconds) a client should take to observe
-  // a disconnect event of the same client from server.
-  static Integer CLIENT_DISCONNECT_TIMEOUT = 3000;
-  static String requireSASLAuthProperty = 
"zookeeper.sessionRequireClientSASLAuth";
-  static String authProviderProperty = "zookeeper.authProvider.1";
-  static String authProvider = 
"org.apache.zookeeper.server.auth.SASLAuthenticationProvider";
-  static String digestLoginModule = 
"org.apache.zookeeper.server.auth.DigestLoginModule";
-  static String jaasConfig = "java.security.auth.login.config";
 
-  static String createJAASConfigFile(String fileName, String password) {
-String ret = null;
-try {
-  File tmpDir = createTmpDir();
-  File jaasFile = new File(tmpDir, fileName);
-  FileWriter fwriter = new FileWriter(jaasFile);
-  fwriter.write("" +
-  "Server {\n" +
-  "  " + digestLoginModule + " required\n" +
-  "  user_super=\"test\";\n" +
-  "};\n" +
-  "Client {\n" +
-  "   " + digestLoginModule + " required\n" +
-  "   username=\"super\"\n" +
-  "   password=\"" + password + "\";\n" +
-  "};" + "\n");
-  fwriter.close();
-  ret = jaasFile.getAbsolutePath();
-} catch (IOException e) {
-  Assert.fail("Unable to create JaaS configuration file!");
+// The maximum time (in milliseconds) a client should take to observe
+// a disconnect event of the same client from server.
+static Integer CLIENT_DISCONNECT_TIMEOUT = 3000;
+static String requireSASLAuthProperty = 
"zookeeper.sessionRequireClientSASLAuth";
+static String authProviderProperty = "zookeeper.authProvider.1";
+static String authProvider = 
"org.apache.zookeeper.server.auth.SASLAuthenticationProvider";
+static String digestLoginModule = 
"org.apache.zookeeper.server.auth.DigestLoginModule";
+static String jaasConfig = "java.security.auth.login.config";
+
+static String createJAASConfigFile(String fileName, String password) {
+String ret = null;
+try {
+File tmpDir = createTmpDir();
+File jaasFile = new File(tmpDir, fileName);
+FileWriter fwriter = new FileWriter(jaasFile);
+fwriter.write(""
+  + "Server {\n"
 
 Review comment:
   revert this string


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314397411
 
 

 ##
 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:
   it is worth reverting
   can you take a look to all this similar cases in tests ?
   I won't add any other comments on this kind of issue


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] symat opened a new pull request #1056: ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+

2019-08-15 Thread GitBox
symat opened a new pull request #1056: ZOOKEEPER-3495: fix SnapshotDigestTest 
to work with JDK12+
URL: https://github.com/apache/zookeeper/pull/1056
 
 
   The problem with the test `SnapshotDigestTest.testDifferentDigestVersion` 
was that it used reflection to change a final static value in 
`DigestCalculator`, what is no longer supported with JDK 12+ (see 
[JDK-8210522](https://bugs.openjdk.java.net/browse/JDK-8210522))
   
   I think there are still some hacky solutions to go behind these limitations 
(with [PowerMock](https://github.com/powermock/powermock) maybe or using 
[VarHandle as suggested 
here](https://stackoverflow.com/questions/56039341/get-declared-fields-of-java-lang-reflect-fields-in-jdk12)),
 but I think the best is to avoid the situation when we need to poke the static 
final field.
   
   I changed the fully static `DigestCalculator`, converting it to a static 
singleton object with non-static methods / fields. The fields of the singleton 
object can not be modified from production code, but we can change them from 
the tests even in JDK 12 / 13 (as they are not static final fields).
   
   I tested it locally using openjdk, I hope it will also work in jenkins.


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314396111
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java
 ##
 @@ -78,10 +75,15 @@ public void 
testIncrementalReconfigInvokedOnHiearchicalQS() throws Exception {
 members.add("weight.5=1");
 
 for (int i = 1; i <= 5; i++) {
-members.add("server." + i + "=127.0.0.1:"
-+ qu.getPeer(i).peer.getQuorumAddress().getPort() + ":"
-+ qu.getPeer(i).peer.getElectionAddress().getPort() + ";"
-+ "127.0.0.1:" + qu.getPeer(i).peer.getClientPort());
+members.add("server."
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314395460
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
 ##
 @@ -375,17 +347,30 @@ private String generateQuorumConfiguration() {
 int portLe2 = PortAssignment.unique();
 int portLe3 = PortAssignment.unique();
 
-
-
-return "server.1=127.0.0.1:" + (portQp1) + ":" + (portLe1) + ";" +  
clientPortQp1 + "\n" +
-   "server.2=127.0.0.1:" + (portQp2) + ":" + (portLe2) + ";" + 
clientPortQp2 + "\n" +
-   "server.3=127.0.0.1:" + (portQp3) + ":" + (portLe3) + ";" + 
clientPortQp3;
+return "server.1=127.0.0.1:"
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314395801
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigBackupTest.java
 ##
 @@ -137,9 +137,14 @@ public void testReconfigCreateNewVersionFile() throws 
Exception {
 clientPorts[i] = PortAssignment.unique();
 quorumPorts[i] = PortAssignment.unique();
 electionPorts[i] = PortAssignment.unique();
-servers[i] = "server." + i + "=localhost:" + quorumPorts[i]
-+ ":" + electionPorts[i] + ":participant;localhost:"
-+ clientPorts[i];
+servers[i] = "server."
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314392789
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java
 ##
 @@ -299,40 +271,31 @@ public void 
processAllFollowingUncommittedAfterFirstCommitTest()
  * in the last iteration, and all lists are empty.
  */
 @Test
-public void processAllWritesMaxBatchSize()
-throws Exception {
+public void processAllWritesMaxBatchSize() throws Exception {
 final String path = "/processAllWritesMaxBatchSize";
 HashSet shouldBeProcessedAfterPending = new 
HashSet();
 
-Request writeReq = newRequest(
-new CreateRequest(path + "_1", new byte[0], 
Ids.OPEN_ACL_UNSAFE,
-CreateMode.PERSISTENT_SEQUENTIAL.toFlag()),
-OpCode.create, 0x1, 1);
+Request writeReq = newRequest(new CreateRequest(path
++ "_1", new 
byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL.toFlag()), 
OpCode.create, 0x1, 1);
 processor.queuedRequests.add(writeReq);
 processor.queuedWriteRequests.add(writeReq);
 
-Request writeReq2 = newRequest(
-new CreateRequest(path + "_2", new byte[0], 
Ids.OPEN_ACL_UNSAFE,
-CreateMode.PERSISTENT_SEQUENTIAL.toFlag()),
-OpCode.create, 0x2, 1);
+Request writeReq2 = newRequest(new CreateRequest(path
+ + "_2", new 
byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL.toFlag()), 
OpCode.create, 0x2, 1);
 processor.queuedRequests.add(writeReq2);
 processor.queuedWriteRequests.add(writeReq2);
 
 for (int readReqId = 2; readReqId <= 5; ++readReqId) {
-Request readReq = newRequest(new GetDataRequest(path, false),
-OpCode.getData, 0x1, readReqId);
-Request readReq2 = newRequest(new GetDataRequest(path, false),
-OpCode.getData, 0x2, readReqId);
+Request readReq = newRequest(new GetDataRequest(path, false), 
OpCode.getData, 0x1, readReqId);
+Request readReq2 = newRequest(new GetDataRequest(path, false), 
OpCode.getData, 0x2, readReqId);
 processor.queuedRequests.add(readReq);
 shouldBeProcessedAfterPending.add(readReq);
 processor.queuedRequests.add(readReq2);
 shouldBeProcessedAfterPending.add(readReq2);
 }
 
-Request writeReq3 = newRequest(
-new CreateRequest(path + "_3", new byte[0], 
Ids.OPEN_ACL_UNSAFE,
-CreateMode.PERSISTENT_SEQUENTIAL.toFlag()),
-OpCode.create, 0x2, 6);
+Request writeReq3 = newRequest(new CreateRequest(path
+ + "_3", new 
byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL.toFlag()), 
OpCode.create, 0x2, 6);
 
 Review comment:
   nit: 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314396014
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigBackupTest.java
 ##
 @@ -256,18 +254,22 @@ public void testVersionOfDynamicFilename() throws 
Exception {
 final int SERVER_COUNT = 5;
 final int oldServerCount = 3;
 final int lagOffServerId = 0;
-final int clientPorts[] = new int[SERVER_COUNT];
+final int[] clientPorts = new int[SERVER_COUNT];
 StringBuilder sb = new StringBuilder();
 String server;
 StringBuilder oldSb = new StringBuilder();
 ArrayList allServers = new ArrayList();
 
-
 for (int i = 0; i < SERVER_COUNT; i++) {
 clientPorts[i] = PortAssignment.unique();
-server = "server." + i + "=localhost:" + PortAssignment.unique()
-+ ":" + PortAssignment.unique() + ":participant;localhost:"
-+ clientPorts[i];
+server = "server."
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314394619
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java
 ##
 @@ -80,63 +78,50 @@ public void testQuorumInternal(String addr) throws 
Exception {
 final int CLIENT_PORT_QP1 = PortAssignment.unique();
 final int CLIENT_PORT_QP2 = PortAssignment.unique();
 
-String quorumCfgSection = String.format("server.1=%1$s:%2$s:%3$s;%4$s",
-addr,
-PortAssignment.unique(),
-PortAssignment.unique(),
-CLIENT_PORT_QP1) + "\n" +
-String.format("server.2=%1$s:%2$s:%3$s;%4$s",
-addr,
-PortAssignment.unique(),
-PortAssignment.unique(),
-CLIENT_PORT_QP2);
+String quorumCfgSection = 
String.format("server.1=%1$s:%2$s:%3$s;%4$s", addr, PortAssignment.unique(), 
PortAssignment.unique(), CLIENT_PORT_QP1)
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314395112
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
 ##
 @@ -164,60 +164,52 @@ public MainThread(int myid, String quorumCfgSection) 
throws IOException {
 this(myid, quorumCfgSection, true);
 }
 
-public MainThread(int myid, String quorumCfgSection, Integer 
secureClientPort, boolean writeDynamicConfigFile)
-throws  IOException {
-this(myid, UNSET_STATIC_CLIENTPORT, JettyAdminServer.DEFAULT_PORT, 
secureClientPort,
-quorumCfgSection, null, null, writeDynamicConfigFile, 
null);
+public MainThread(int myid, String quorumCfgSection, Integer 
secureClientPort, boolean writeDynamicConfigFile) throws IOException {
+this(myid, UNSET_STATIC_CLIENTPORT, JettyAdminServer.DEFAULT_PORT, 
secureClientPort, quorumCfgSection, null, null, writeDynamicConfigFile, null);
 }
 
-public MainThread(int myid, String quorumCfgSection, boolean 
writeDynamicConfigFile)
-throws IOException {
+public MainThread(int myid, String quorumCfgSection, boolean 
writeDynamicConfigFile) throws IOException {
 this(myid, UNSET_STATIC_CLIENTPORT, quorumCfgSection, 
writeDynamicConfigFile);
 }
 
-public MainThread(int myid, int clientPort, String quorumCfgSection, 
boolean writeDynamicConfigFile)
-throws IOException {
+public MainThread(int myid, int clientPort, String quorumCfgSection, 
boolean writeDynamicConfigFile) throws IOException {
 this(myid, clientPort, JettyAdminServer.DEFAULT_PORT, 
quorumCfgSection, null, null, writeDynamicConfigFile);
 }
-
-public MainThread(int myid, int clientPort, String quorumCfgSection, 
String peerType, boolean writeDynamicConfigFile)
-throws IOException {
+
+public MainThread(int myid, int clientPort, String quorumCfgSection, 
String peerType, boolean writeDynamicConfigFile) throws IOException {
 this(myid, clientPort, JettyAdminServer.DEFAULT_PORT, 
quorumCfgSection, null, peerType, writeDynamicConfigFile);
 }
 
-public MainThread(int myid, int clientPort, String quorumCfgSection, 
boolean writeDynamicConfigFile,
-  String version) throws IOException {
-this(myid, clientPort, JettyAdminServer.DEFAULT_PORT, 
quorumCfgSection, null,
-null, writeDynamicConfigFile, version);
+public MainThread(int myid, int clientPort, String quorumCfgSection, 
boolean writeDynamicConfigFile, String version) throws IOException {
+this(myid, clientPort, JettyAdminServer.DEFAULT_PORT, 
quorumCfgSection, null, null, writeDynamicConfigFile, version);
 }
 
-public MainThread(int myid, int clientPort, String quorumCfgSection, 
String configs)
-throws IOException {
+public MainThread(int myid, int clientPort, String quorumCfgSection, 
String configs) throws IOException {
 this(myid, clientPort, JettyAdminServer.DEFAULT_PORT, 
quorumCfgSection, configs, null, true);
 }
 
-public MainThread(int myid, int clientPort, int adminServerPort, 
String quorumCfgSection,
-String configs)  throws IOException {
+public MainThread(int myid, int clientPort, int adminServerPort, 
String quorumCfgSection, String configs) throws IOException {
 this(myid, clientPort, adminServerPort, quorumCfgSection, configs, 
null, true);
 }
 
-public MainThread(int myid, int clientPort, int adminServerPort, 
String quorumCfgSection,
-String configs, String peerType, boolean 
writeDynamicConfigFile)
-throws IOException {
+public MainThread(int myid, int clientPort, int adminServerPort, 
String quorumCfgSection, String configs, String peerType, boolean 
writeDynamicConfigFile) throws IOException {
 this(myid, clientPort, adminServerPort, quorumCfgSection, configs, 
peerType, writeDynamicConfigFile, null);
 }
 
-public MainThread(int myid, int clientPort, int adminServerPort, 
String quorumCfgSection,
-  String configs, String peerType, boolean 
writeDynamicConfigFile, String version) throws IOException {
+public MainThread(int myid, int clientPort, int adminServerPort, 
String quorumCfgSection, String configs, String peerType, boolean 
writeDynamicConfigFile, String version) throws IOException {
 this(myid, clientPort, adminServerPort, null, quorumCfgSection, 
configs, peerType, writeDynamicConfigFile, version);
 }
 
-public MainThread(int myid, int clientPort, int adminServerPort, 
Integer secureClientPort,
- 

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

2019-08-15 Thread GitBox
eolivelli 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_r314396335
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigLegacyTest.java
 ##
 @@ -252,39 +248,41 @@ public static Properties readPropertiesFromFile(File 
file) throws IOException {
  */
 @Test(timeout = 12)
 public void testRestartZooKeeperServer() throws Exception {
-final int clientPorts[] = new int[SERVER_COUNT];
+final int[] clientPorts = new int[SERVER_COUNT];
 StringBuilder sb = new StringBuilder();
 String server;
 
 for (int i = 0; i < SERVER_COUNT; i++) {
 clientPorts[i] = PortAssignment.unique();
-server = "server." + i + "=127.0.0.1:" + PortAssignment.unique()
-+ ":" + PortAssignment.unique() + ":participant;127.0.0.1:"
-+ clientPorts[i];
+server = "server."
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314394741
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java
 ##
 @@ -80,63 +78,50 @@ public void testQuorumInternal(String addr) throws 
Exception {
 final int CLIENT_PORT_QP1 = PortAssignment.unique();
 final int CLIENT_PORT_QP2 = PortAssignment.unique();
 
-String quorumCfgSection = String.format("server.1=%1$s:%2$s:%3$s;%4$s",
-addr,
-PortAssignment.unique(),
-PortAssignment.unique(),
-CLIENT_PORT_QP1) + "\n" +
-String.format("server.2=%1$s:%2$s:%3$s;%4$s",
-addr,
-PortAssignment.unique(),
-PortAssignment.unique(),
-CLIENT_PORT_QP2);
+String quorumCfgSection = 
String.format("server.1=%1$s:%2$s:%3$s;%4$s", addr, PortAssignment.unique(), 
PortAssignment.unique(), CLIENT_PORT_QP1)
+  + "\n"
+  + 
String.format("server.2=%1$s:%2$s:%3$s;%4$s", addr, PortAssignment.unique(), 
PortAssignment.unique(), CLIENT_PORT_QP2);
 
 MainThread q1 = new MainThread(1, CLIENT_PORT_QP1, quorumCfgSection);
 MainThread q2 = new MainThread(2, CLIENT_PORT_QP2, quorumCfgSection);
 q1.start();
 q2.start();
 
-Assert.assertTrue("waiting for server 1 being up",
-ClientBase.waitForServerUp(addr + ":" + CLIENT_PORT_QP1,
-CONNECTION_TIMEOUT));
-Assert.assertTrue("waiting for server 2 being up",
-ClientBase.waitForServerUp(addr + ":" + CLIENT_PORT_QP2,
-CONNECTION_TIMEOUT));
+Assert.assertTrue("waiting for server 1 being up", 
ClientBase.waitForServerUp(addr
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314396217
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigLegacyTest.java
 ##
 @@ -58,49 +55,53 @@ public void setup() {
  */
 @Test
 public void testConfigFileBackwardCompatibility() throws Exception {
-final int clientPorts[] = new int[SERVER_COUNT];
+final int[] clientPorts = new int[SERVER_COUNT];
 StringBuilder sb = new StringBuilder();
 String server;
 ArrayList allServers = new ArrayList();
 
 for (int i = 0; i < SERVER_COUNT; i++) {
 clientPorts[i] = PortAssignment.unique();
-server = "server." + i + "=localhost:" + PortAssignment.unique()
-+ ":" + PortAssignment.unique() + ":participant;localhost:"
-+ clientPorts[i];
+server = "server."
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314395631
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/RaceConditionTest.java
 ##
 @@ -105,18 +103,24 @@ public void tearDown() {
 }
 
 private MainThread[] startQuorum() throws IOException {
-final int clientPorts[] = new int[SERVER_COUNT];
+final int[] clientPorts = new int[SERVER_COUNT];
 StringBuilder sb = new StringBuilder();
 String server;
 
 for (int i = 0; i < SERVER_COUNT; i++) {
 clientPorts[i] = PortAssignment.unique();
-server = "server." + i + "=127.0.0.1:" + PortAssignment.unique() + 
":" + PortAssignment.unique()
-+ ":participant;127.0.0.1:" + clientPorts[i];
+server = "server."
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314392692
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java
 ##
 @@ -299,40 +271,31 @@ public void 
processAllFollowingUncommittedAfterFirstCommitTest()
  * in the last iteration, and all lists are empty.
  */
 @Test
-public void processAllWritesMaxBatchSize()
-throws Exception {
+public void processAllWritesMaxBatchSize() throws Exception {
 final String path = "/processAllWritesMaxBatchSize";
 HashSet shouldBeProcessedAfterPending = new 
HashSet();
 
-Request writeReq = newRequest(
-new CreateRequest(path + "_1", new byte[0], 
Ids.OPEN_ACL_UNSAFE,
-CreateMode.PERSISTENT_SEQUENTIAL.toFlag()),
-OpCode.create, 0x1, 1);
+Request writeReq = newRequest(new CreateRequest(path
++ "_1", new 
byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL.toFlag()), 
OpCode.create, 0x1, 1);
 processor.queuedRequests.add(writeReq);
 processor.queuedWriteRequests.add(writeReq);
 
-Request writeReq2 = newRequest(
-new CreateRequest(path + "_2", new byte[0], 
Ids.OPEN_ACL_UNSAFE,
-CreateMode.PERSISTENT_SEQUENTIAL.toFlag()),
-OpCode.create, 0x2, 1);
+Request writeReq2 = newRequest(new CreateRequest(path
+ + "_2", new 
byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL.toFlag()), 
OpCode.create, 0x2, 1);
 
 Review comment:
   nit: 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314390603
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/SerializationPerfTest.java
 ##
 @@ -56,70 +58,70 @@ static int createNodes(DataTree tree, String path, int 
depth,
 return count;
 }
 
-private static void serializeTree(int depth, int width, int len)
-throws InterruptedException, IOException, 
KeeperException.NodeExistsException, KeeperException.NoNodeException {
+private static void serializeTree(int depth, int width, int len) throws 
InterruptedException, IOException, KeeperException.NodeExistsException, 
KeeperException.NoNodeException {
 DataTree tree = new DataTree();
 createNodes(tree, "/", depth, width, 
tree.getNode("/").stat.getCversion(), new byte[len]);
 int count = tree.getNodeCount();
 
-BinaryOutputArchive oa =
-BinaryOutputArchive.getArchive(new NullOutputStream());
+BinaryOutputArchive oa = BinaryOutputArchive.getArchive(new 
NullOutputStream());
 System.gc();
 long start = System.nanoTime();
 tree.serialize(oa, "test");
 long end = System.nanoTime();
-long durationms = (end - start)/100L;
-long pernodeus = ((end - start)/1000L)/count;
-LOG.info("Serialized " + count + " nodes in "
-+ durationms + " ms (" + pernodeus + "us/node), depth="
-+ depth + " width=" + width + " datalen=" + len);
+long durationms = (end - start) / 100L;
+long pernodeus = ((end - start) / 1000L) / count;
+LOG.info("Serialized "
 
 Review comment:
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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314395393
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
 ##
 @@ -318,49 +303,36 @@ public X509Certificate buildEndEntityCert(KeyPair 
keyPair, X509Certificate caCer
 generalNames.add(new GeneralName(GeneralName.iPAddress, 
ipAddress));
 }
 
-SubjectPublicKeyInfo entityKeyInfo =
-
SubjectPublicKeyInfoFactory.createSubjectPublicKeyInfo(PublicKeyFactory.createKey(keyPair.getPublic()
-.getEncoded()));
+SubjectPublicKeyInfo entityKeyInfo = 
SubjectPublicKeyInfoFactory.createSubjectPublicKeyInfo(PublicKeyFactory.createKey(keyPair.getPublic().getEncoded()));
 X509ExtensionUtils extensionUtils = new BcX509ExtensionUtils();
-X509v3CertificateBuilder certificateBuilder =
-new JcaX509v3CertificateBuilder(holder.getSubject(), new 
BigInteger(128, new Random()),
-certStartTime, certEndTime, new X500Name("CN=Test End 
Entity Certificate"), keyPair.getPublic())
-.addExtension(Extension.authorityKeyIdentifier, false,
-extensionUtils.createAuthorityKeyIdentifier(holder))
-.addExtension(Extension.subjectKeyIdentifier, false,
-
extensionUtils.createSubjectKeyIdentifier(entityKeyInfo))
-.addExtension(Extension.basicConstraints, true, new 
BasicConstraints(false))
-.addExtension(Extension.keyUsage, true,
-new KeyUsage(KeyUsage.digitalSignature | 
KeyUsage.keyEncipherment));
+X509v3CertificateBuilder certificateBuilder = new 
JcaX509v3CertificateBuilder(holder.getSubject(), new BigInteger(128, new 
Random()), certStartTime, certEndTime, new X500Name("CN=Test End Entity 
Certificate"), 
keyPair.getPublic()).addExtension(Extension.authorityKeyIdentifier, false, 
extensionUtils.createAuthorityKeyIdentifier(holder)).addExtension(Extension.subjectKeyIdentifier,
 false, 
extensionUtils.createSubjectKeyIdentifier(entityKeyInfo)).addExtension(Extension.basicConstraints,
 true, new BasicConstraints(false)).addExtension(Extension.keyUsage, true, new 
KeyUsage(KeyUsage.digitalSignature
+   






 | KeyUsage.keyEncipherment));
 
 if (!generalNames.isEmpty()) {
-certificateBuilder.addExtension(Extension.subjectAlternativeName,  
true,
-new GeneralNames(generalNames.toArray(new GeneralName[] 
{})));
+certificateBuilder.addExtension(Extension.subjectAlternativeName, 
true, new GeneralNames(generalNames.toArray(new GeneralName[]{})));
 }
 
 if (crlPath != null) {
-DistributionPointName distPointOne = new DistributionPointName(new 
GeneralNames(
-new 
GeneralName(GeneralName.uniformResourceIdentifier,"file://" + crlPath)));
+DistributionPointName distPointOne = new DistributionPointName(new 
GeneralNames(new GeneralName(GeneralName.uniformResourceIdentifier,
+   
 "file://"
+   
 + crlPath)));
 
-certificateBuilder.addExtension(Extension.cRLDistributionPoints, 
false,
-new CRLDistPoint(new DistributionPoint[] {
-new DistributionPoint(distPointOne, null, null)
-}));
+certificateBuilder.addExtension(Extension.cRLDistributionPoints, 
false, new CRLDistPoint(new DistributionPoint[]{new 
DistributionPoint(distPointOne, null, null)}));
 }
 
 if (ocspPort != null) {
-certificateBuilder.addExtension(Extension.authorityInfoAccess, 
false,
-new 
AuthorityInformationAccess(X509ObjectIdentifiers.ocspAccessMethod,
-new GeneralName(GeneralName.uniformResourceIdentifier, 
"http://; + hostname + ":" + ocspPort)));
+certificateBuilder.addExtension(Extension.authorityInfoAccess, 
false, new AuthorityInformationAccess(X509ObjectIdentifiers.ocspAccessMethod, 
new 

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

2019-08-15 Thread GitBox
eolivelli 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_r314395895
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigBackupTest.java
 ##
 @@ -72,37 +69,40 @@ public void setup() {
 @Test
 public void testBackupStatic() throws Exception {
 final int SERVER_COUNT = 3;
-final int clientPorts[] = new int[SERVER_COUNT];
+final int[] clientPorts = new int[SERVER_COUNT];
 StringBuilder sb = new StringBuilder();
 String server;
 
 for (int i = 0; i < SERVER_COUNT; i++) {
 clientPorts[i] = PortAssignment.unique();
-server = "server." + i + "=localhost:" + PortAssignment.unique()
-+ ":" + PortAssignment.unique() + ":participant;localhost:"
-+ clientPorts[i];
+server = "server."
 
 Review comment:
   it is worth 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314392431
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java
 ##
 @@ -549,8 +550,12 @@ public String _verifyThreadCount(ArrayList 
peerList, long ecnt) {
 long cnt = cnxManager.getThreadCount();
 if (cnt != ecnt) {
 return new Date()
-+ " Incorrect number of Worker threads for sid=" + myid
-+ " expected " + ecnt + " found " + cnt;
+   + " Incorrect number of Worker threads for sid="
 
 Review comment:
   nit: 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_r314366835
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/SaslClientCallbackHandler.java
 ##
 @@ -50,36 +51,32 @@ public void handle(Callback[] callbacks) throws 
UnsupportedCallbackException {
 if (callback instanceof NameCallback) {
 NameCallback nc = (NameCallback) callback;
 nc.setName(nc.getDefaultName());
-}
-else {
+} else {
 if (callback instanceof PasswordCallback) {
-PasswordCallback pc = (PasswordCallback)callback;
+PasswordCallback pc = (PasswordCallback) callback;
 if (password != null) {
 pc.setPassword(this.password.toCharArray());
 } else {
-LOG.warn("Could not login: the {} is being asked for a 
password, but the ZooKeeper {}" +
-  " code does not currently support obtaining a 
password from the user." +
-  " Make sure that the {} is configured to use a 
ticket cache (using" +
-  " the JAAS configuration setting 
'useTicketCache=true)' and restart the {}. If" +
-  " you still get this message after that, the TGT in 
the ticket cache has expired and must" +
-  " be manually refreshed. To do so, first determine 
if you are using a password or a" +
-  " keytab. If the former, run kinit in a Unix shell 
in the environment of the user who" +
-  " is running this Zookeeper {} using the command" +
-  " 'kinit ' (where  is the name of the 
{}'s Kerberos principal)." +
-  " If the latter, do" +
-  " 'kinit -k -t  ' (where  is 
the name of the Kerberos principal, and" +
-  "  is the location of the keytab file). 
After manually refreshing your cache," +
-  " restart this {}. If you continue to see this 
message after manually refreshing" +
-  " your cache, ensure that your KDC host's clock is 
in sync with this host's clock.",
-  new Object[]{entity, entity, entity, entity, entity, 
entity, entity});
+LOG.warn("Could not login: the {} is being asked for a 
password, but the ZooKeeper {}"
+ + " code does not currently support 
obtaining a password from the user."
 
 Review comment:
   Reformatting...
   
   Align `+`s with the initial `"`


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314188638
 
 

 ##
 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:
   this line was more readable before


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314188240
 
 

 ##
 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:
   this line looks bad


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314292174
 
 

 ##
 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:
   The final result is the same, but the flow is changing a little.
   I would be more comfortable to keep it has it was before, if checkstyle 
would be happy 


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314190876
 
 

 ##
 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:
   can you revert this 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:
us...@infra.apache.org


With regards,
Apache Git Services


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

2019-08-15 Thread GitBox
eolivelli 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_r314189868
 
 

 ##
 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:
   nit:  indent?


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314287338
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
 ##
 @@ -3155,16 +3006,14 @@ private ClientCnxnSocket getClientCnxnSocket() throws 
IOException {
 ClientCnxnSocket clientCxnSocket = (ClientCnxnSocket) 
clientCxnConstructor.newInstance(getClientConfig());
 return clientCxnSocket;
 } catch (Exception e) {
-IOException ioe = new IOException("Couldn't instantiate "
-+ clientCnxnSocketName);
-ioe.initCause(e);
+IOException ioe = new IOException("Couldn't instantiate " + 
clientCnxnSocketName, e);
 
 Review comment:
   nit:
   throw new IOException


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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314188139
 
 

 ##
 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:
   nit: maybe this change is not needed, was it an automatic 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] eolivelli commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
eolivelli 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_r314189651
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/SaslClientCallbackHandler.java
 ##
 @@ -50,36 +51,32 @@ public void handle(Callback[] callbacks) throws 
UnsupportedCallbackException {
 if (callback instanceof NameCallback) {
 NameCallback nc = (NameCallback) callback;
 nc.setName(nc.getDefaultName());
-}
-else {
+} else {
 if (callback instanceof PasswordCallback) {
-PasswordCallback pc = (PasswordCallback)callback;
+PasswordCallback pc = (PasswordCallback) callback;
 if (password != null) {
 pc.setPassword(this.password.toCharArray());
 } else {
-LOG.warn("Could not login: the {} is being asked for a 
password, but the ZooKeeper {}" +
-  " code does not currently support obtaining a 
password from the user." +
-  " Make sure that the {} is configured to use a 
ticket cache (using" +
-  " the JAAS configuration setting 
'useTicketCache=true)' and restart the {}. If" +
-  " you still get this message after that, the TGT in 
the ticket cache has expired and must" +
-  " be manually refreshed. To do so, first determine 
if you are using a password or a" +
-  " keytab. If the former, run kinit in a Unix shell 
in the environment of the user who" +
-  " is running this Zookeeper {} using the command" +
-  " 'kinit ' (where  is the name of the 
{}'s Kerberos principal)." +
-  " If the latter, do" +
-  " 'kinit -k -t  ' (where  is 
the name of the Kerberos principal, and" +
-  "  is the location of the keytab file). 
After manually refreshing your cache," +
-  " restart this {}. If you continue to see this 
message after manually refreshing" +
-  " your cache, ensure that your KDC host's clock is 
in sync with this host's clock.",
-  new Object[]{entity, entity, entity, entity, entity, 
entity, entity});
+LOG.warn("Could not login: the {} is being asked for a 
password, but the ZooKeeper {}"
+ + " code does not currently support 
obtaining a password from the user."
 
 Review comment:
   nit: indent seems strage here


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] sonymoon commented on a change in pull request #769: ZOOKEEPER-3242: Add server side connecting throttling

2019-08-15 Thread GitBox
sonymoon commented on a change in pull request #769: ZOOKEEPER-3242: Add server 
side connecting throttling
URL: https://github.com/apache/zookeeper/pull/769#discussion_r314345629
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/BlueThrottle.java
 ##
 @@ -0,0 +1,268 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server;
+
+import java.util.Random;
+
+import org.apache.zookeeper.common.Time;
+
+/**
+ * Implements a token-bucket based rate limiting mechanism with optional
+ * probabilistic dropping inspired by the BLUE queue management algorithm [1].
+ *
+ * The throttle provides the {@link #checkLimit(int)} method which provides
+ * a binary yes/no decision.
+ *
+ * The core token bucket algorithm starts with an initial set of tokens based
+ * on the maxTokens setting. Tokens are dispensed each
+ * {@link #checkLimit(int)} call, which fails if there are not enough tokens to
+ * satisfy a given request.
+ *
+ * The token bucket refills over time, providing fillCount tokens
+ * every fillTime milliseconds, capping at maxTokens.
+ *
+ * This design allows the throttle to allow short bursts to pass, while still
+ * capping the total number of requests per time interval.
+ *
+ * One issue with a pure token bucket approach for something like request or
+ * connection throttling is that the wall clock arrival time of requests 
affects
+ * the probability of a request being allowed to pass or not. Under constant
+ * load this can lead to request starvation for requests that constantly arrive
+ * later than the majority.
+ *
+ * In an attempt to combat this, this throttle can also provide probabilistic
+ * dropping. This is enabled anytime freezeTime is set to a value
+ * other than -1.
+ *
+ * The probabilistic algorithm starts with an initial drop probability of 0, 
and
+ * adjusts this probability roughly every freezeTime milliseconds.
+ * The first request after freezeTime, the algorithm checks the
+ * token bucket. If the token bucket is empty, the drop probability is 
increased
+ * by dropIncrease up to a maximum of 1. Otherwise, 
if
+ * the bucket has a token deficit less than decreasePoint * 
maxTokens,
+ * the probability is decreased by dropDecrease.
+ *
+ * Given a call to {@link #checkLimit(int)}, requests are first dropped 
randomly
+ * based on the current drop probability, and only surviving requests are then
+ * checked against the token bucket.
+ *
+ * When under constant load, the probabilistic algorithm will adapt to a drop
+ * frequency that should keep requests within the token limit. When load drops,
+ * the drop probability will decrease, eventually returning to zero if 
possible.
+ *
+ * [1] "BLUE: A New Class of Active Queue Management Algorithms"
+ **/
+
+public class BlueThrottle {
+private int maxTokens;
+private int fillTime;
+private int fillCount;
+private int tokens;
+private long lastTime;
+
+private int freezeTime;
+private long lastFreeze;
+private double dropIncrease;
+private double dropDecrease;
+private double decreasePoint;
+private double drop;
+
+Random rng;
+
+public static final String CONNECTION_THROTTLE_TOKENS = 
"zookeeper.connection_throttle_tokens";
+public static final int DEFAULT_CONNECTION_THROTTLE_TOKENS;
+
+public static final String CONNECTION_THROTTLE_FILL_TIME = 
"zookeeper.connection_throttle_fill_time";
+public static final int DEFAULT_CONNECTION_THROTTLE_FILL_TIME;
+
+public static final String CONNECTION_THROTTLE_FILL_COUNT = 
"zookeeper.connection_throttle_fill_count";
+public static final int DEFAULT_CONNECTION_THROTTLE_FILL_COUNT;
+
+public static final String CONNECTION_THROTTLE_FREEZE_TIME = 
"zookeeper.connection_throttle_freeze_time";
+public static final int DEFAULT_CONNECTION_THROTTLE_FREEZE_TIME;
+
+public static final String CONNECTION_THROTTLE_DROP_INCREASE = 
"zookeeper.connection_throttle_drop_increase";
+public static final double DEFAULT_CONNECTION_THROTTLE_DROP_INCREASE;
+
+public static final String CONNECTION_THROTTLE_DROP_DECREASE = 
"zookeeper.connection_throttle_drop_decrease";
+public static 

[GitHub] [zookeeper] nkalmar commented on issue #1046: 2019/08/08

2019-08-15 Thread GitBox
nkalmar commented on issue #1046: 2019/08/08
URL: https://github.com/apache/zookeeper/pull/1046#issuecomment-521569551
 
 
   I'm closing this - no jira number, no description, cfg files, comments not 
in english and no clear intent. 
   Please create a jira if you would like to commit something, stick with 
english, and only commit code modifications (not cfg files). Thank you!


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] nkalmar closed pull request #1046: 2019/08/08

2019-08-15 Thread GitBox
nkalmar closed pull request #1046: 2019/08/08
URL: https://github.com/apache/zookeeper/pull/1046
 
 
   


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] ztzg opened a new pull request #1055: ZOOKEEPER-3510: Make 'zkServer.sh stop' more reliable

2019-08-15 Thread GitBox
ztzg opened a new pull request #1055: ZOOKEEPER-3510: Make 'zkServer.sh stop' 
more reliable
URL: https://github.com/apache/zookeeper/pull/1055
 
 
   As mentioned in 
https://github.com/apache/zookeeper/pull/1054#discussion_r314208678 :
   
   There is a `sleep 3` statement in `zkServer.sh restart`. I am unable to 
unearth the history of that particular line, but I believe part—if not all—of 
that sleep should be part of `zkServer.sh stop`.
   
   I frequently observe `FAILED TO START` errors in the C test suite; the logs 
consistently show that those are caused by `java.net.BindException: Address 
already in use`. Adding a simple `sleep 1` before `echo STOPPED` "fixes" it for 
me.
   
   As noted in the commit message, the `sleep` is far from optimal, an adaptive 
mechanism would be better—but I do not want to make the first iteration too 
complicated.


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] ztzg commented on a change in pull request #1054: ZOOKEEPER-1112: Add support for C client for SASL authentication

2019-08-15 Thread GitBox
ztzg commented on a change in pull request #1054: ZOOKEEPER-1112: Add support 
for C client for SASL authentication
URL: https://github.com/apache/zookeeper/pull/1054#discussion_r314209362
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/util/SecurityUtils.java
 ##
 @@ -153,6 +155,11 @@ public SaslClient run() throws SaslException {
 public static SaslServer createSaslServer(final Subject subject,
 final String protocol, final String serverName,
 final CallbackHandler callbackHandler, final Logger LOG) {
+// required by c client api - Sasl.QOP="auth" is not set
+// by default although stated in javadoc (Sun JRE 1.6.0_26-b03)
+HashMap props = new HashMap();
 
 Review comment:
   That change was part of the original patches, which I have tried to "rebase" 
with a minimum amount of changes.
   
   I don't have a good answer to this question.  I don't know if @kloni is 
still around, and if he could comment on it.
   
   Looking closer, it looks like putting anything else than `auth` in there 
might not be compliant—so I am planning to revert this part of the patch in an 
upcoming respin, and am including the preliminary commit message below.
   
   (Note that I am not against squashing some of the patches if it makes the 
series easier to ingest; just let me know if you prefer sequential changes or 
mixed authorship.)
   
   ---
   
   ZOOKEEPER-1112: Do not specify QOP for the SASL server
   
   The explicit QOP setting had been added with a comment specifying that 
`Sasl.QOP="auth"` was not set by the 1.6 JRE.
   
   More recent JREs, such as 1.8, properly set QOP to `auth` by default, and 
both Cyrus SASL and Perl's `Authen::SASL` have been verified to be okay with it.
   
   The patch also included `auth-conf` and `auth-int` in the preferences list; 
the reason for that is unclear.  It also seems incorrect, as the wire protocol 
does not provide checksums nor encryption.  (The plan is to carry everything 
over TLS anyway; perhaps QOP should be set to that triple when TLS is active?)


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] ztzg commented on a change in pull request #1054: ZOOKEEPER-1112: Add support for C client for SASL authentication

2019-08-15 Thread GitBox
ztzg commented on a change in pull request #1054: ZOOKEEPER-1112: Add support 
for C client for SASL authentication
URL: https://github.com/apache/zookeeper/pull/1054#discussion_r314208678
 
 

 ##
 File path: bin/zkServer.sh
 ##
 @@ -212,6 +212,7 @@ stop)
 else
   $KILL $(cat "$ZOOPIDFILE")
   rm "$ZOOPIDFILE"
+  sleep 1
 
 Review comment:
   Hi Enrico,
   
   Probably for the same reason that there is a `sleep 3` in `restart`.  Here 
is what I have put in the corresponding commit message:
   
   ---
   
   ZOOKEEPER-1112: Make `zkServer.sh stop` more reliable
   
   Kill is asynchronous, and without the sleep, the server's TCP port can still 
be busy when the next server is started—causing flaky runs of the C client's 
test suite.
   
   (It would probably be better to spin a few times, probing with `ps -p`.)
   
   ---
   
   This is not hypothetical: I am observing `FAILED TO START` errors in the C 
test suite; the log consistently shows that those are caused by 
`java.net.BindException: Address already in use`.
   
   As noted above, the `sleep` is far from optimal, an adaptive mechanism would 
be better—but I did not want to make the patch too complicated.
   
   Here is what I would suggest: I will drop the patch from my upcoming respin, 
create a dedicated ticket, and carry it locally in the meantime.


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