[ https://issues.apache.org/jira/browse/ZOOKEEPER-2140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14599039#comment-14599039 ]
Hadoop QA commented on ZOOKEEPER-2140: -------------------------------------- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12704961/ZOOKEEPER-2140-2.patch against trunk revision 1686767. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2778//console This message is automatically generated. > NettyServerCnxn and NIOServerCnxn code should be improved > --------------------------------------------------------- > > Key: ZOOKEEPER-2140 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2140 > Project: ZooKeeper > Issue Type: Improvement > Reporter: Arshad Mohammad > Fix For: 3.6.0 > > Attachments: ZOOKEEPER-2140-1.patch, ZOOKEEPER-2140-2.patch > > > Classes org.apache.zookeeper.server.NIOServerCnxn and > org.apache.zookeeper.server.NettyServerCnxn have following need and scope for > improvement > 1) Duplicate code. > These two classes have around 250 line duplicate code. All the command > code is duplicated > 2) Many improvement/bugFix done in one class but not done in other class. > These changes should be synced > For example > In NettyServerCnxn > {code} > // clone should be faster than iteration > // ie give up the cnxns lock faster > AbstractSet<ServerCnxn> cnxns; > synchronized (factory.cnxns) { > cnxns = new HashSet<ServerCnxn>(factory.cnxns); > } > for (ServerCnxn c : cnxns) { > c.dumpConnectionInfo(pw, false); > pw.println(); > } > {code} > In NIOServerCnxn > {code} > for (ServerCnxn c : factory.cnxns) { > c.dumpConnectionInfo(pw, false); > pw.println(); > } > {code} > 3) NettyServerCnxn and NIOServerCnxn classes are bulky unnecessarily. > Command classes have altogether different functionality, the command classes > should go in different class files. > If this done it will be easy to add new command with minimal change to > existing classes. -- This message was sent by Atlassian JIRA (v6.3.4#6332)