[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] 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] 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] 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 General

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

2019-08-12 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_r313081715
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
 ##
 @@ -202,78 +201,81 @@
 }
 }
 
-static public InitialMessage parse(Long protocolVersion, 
DataInputStream din)
-throws InitialMessageException, IOException {
+public static InitialMessage parse(
+Long protocolVersion,
+DataInputStream din
+) throws InitialMessageException, IOException {
 Long sid;
 
 if (protocolVersion != PROTOCOL_VERSION) {
-throw new InitialMessageException(
-"Got unrecognized protocol version %s", 
protocolVersion);
+throw new InitialMessageException("Got unrecognized protocol 
version %s", protocolVersion);
 }
 
 sid = din.readLong();
 
 int remaining = din.readInt();
 if (remaining <= 0 || remaining > maxBuffer) {
-throw new InitialMessageException(
-"Unreasonable buffer length: %s", remaining);
+throw new InitialMessageException("Unreasonable buffer length: 
%s", remaining);
 }
 
 byte[] b = new byte[remaining];
-int num_read = din.read(b);
+int numRead = din.read(b);
 
 Review comment:
   I totally agree!
   I have talked about this in the ML, we should not change variable names and 
method signatures.
   It is dangerous, especially for pending patches waiting for a merge


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

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/TestHammer.java
 ##
 @@ -18,41 +18,43 @@
 
 package org.apache.zookeeper.test;
 
-
-import org.apache.zookeeper.CreateMode;
-import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.AsyncCallback.VoidCallback;
+import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 
 public class TestHammer implements VoidCallback {
 
-static int REPS = 5;
+private static int reps = 5;
 
 Review comment:
   Yes it does some static analysis but I don't think it requests this change.
   
   @tisunkun which is the rule that fires for 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-11 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_r312754136
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
 ##
 @@ -962,14 +979,14 @@ public void dumpConnections(PrintWriter pwriter) {
 @Override
 public void resetAllConnectionStats() {
 // No need to synchronize since cnxns is backed by a ConcurrentHashMap
-for(ServerCnxn c : cnxns){
+for (ServerCnxn c : cnxns) {
 c.resetStats();
 }
 }
 
 @Override
 public Iterable> getAllConnectionInfo(boolean brief) {
-HashSet> info = new HashSet>();
+HashSet> info = new HashSet>();
 
 Review comment:
   @enixon totally agreed that we should use the interface here
   
   Personally I would prefer not to change this and the other points just 
because of checkstyle.
   
   Let's see what the others in the community think


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