Jacob Levy commented on ZOOKEEPER-909:

Some questions after reviewing your diff (comments from Jacob and Michi):

In ClientCnxn.java:

* Calling the member variable 'socket' is somewhat confusing. Maybe rename to 
* I am not sure about the motivation for moving the state member variable out 
of class ZooKeeper into ClientConnection -- can you explain?
* Does the move of the state member have to be implemented at the same time as 
the NIO related changes? If not, perhaps split the JIRA?
* Why are the inner classes SessionExpiredException and EndOfStreamException 
now package static? Was there a scoping issue with getting the wrong type 
(there are already identically named classes in KeeperException.java).
* Please add a detailed comment on the purpose and implementation of 

> Extract NIO specific code from ClientCnxn
> -----------------------------------------
>                 Key: ZOOKEEPER-909
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-909
>             Project: Zookeeper
>          Issue Type: Sub-task
>          Components: java client
>            Reporter: Thomas Koch
>            Assignee: Thomas Koch
>             Fix For: 3.4.0
>         Attachments: ZOOKEEPER-909.patch, ZOOKEEPER-909.patch, 
> ZOOKEEPER-909.patch
> This patch is mostly the same patch as my last one for ZOOKEEPER-823 minus 
> everything Netty related. This means this patch only extract all NIO specific 
> code in the class ClientCnxnSocketNIO which extends ClientCnxnSocket.
> I've redone this patch from current trunk step by step now and couldn't find 
> any logical error. I've already done a couple of successful test runs and 
> will continue to do so this night.
> It would be nice, if we could apply this patch as soon as possible to trunk. 
> This allows us to continue to work on the netty integration without blocking 
> the ClientCnxn class. Adding Netty after this patch should be only a matter 
> of adding the ClientCnxnSocketNetty class with the appropriate test cases.
> You could help me by reviewing the patch and by running it on whatever test 
> server you have available. Please send me any complete failure log you should 
> encounter to thomas at koch point ro. Thx!
> Update: Until now, I've collected 8 successful builds in a row!

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to