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