You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by "Patrick Hunt (JIRA)" <ji...@apache.org> on 2010/07/19 23:57:52 UTC

[jira] Created: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

update ZooKeeper java client to optionally use Netty for connections
--------------------------------------------------------------------

                 Key: ZOOKEEPER-823
                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
             Project: Zookeeper
          Issue Type: New Feature
          Components: java client
            Reporter: Patrick Hunt
            Assignee: Patrick Hunt
             Fix For: 3.4.0


This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12909636#action_12909636 ] 

Hadoop QA commented on ZOOKEEPER-823:
-------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12453672/ZOOKEEPER-823.patch
  against trunk revision 997192.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 21 new or modified tests.

    -1 patch.  The patch command could not apply the patch.

Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/113/console

This message is automatically generated.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Flavio Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12914822#action_12914822 ] 

Flavio Junqueira commented on ZOOKEEPER-823:
--------------------------------------------

Here is another instance:

{noformat}
Testcase: testPathValidation took 1.865 sec
	Caused an ERROR
KeeperErrorCode = ConnectionLoss for /chrootclienttest
org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss for /chrootclienttest
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:90)
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:42)
	at org.apache.zookeeper.ZooKeeper.create(ZooKeeper.java:640)
	at org.apache.zookeeper.test.ChrootClientTest.setUp(ChrootClientTest.java:42)
{noformat}

I'm on Mac OS X 1.5.8, java build 1.6.0_20-b02-279-9M3165.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Open  (was: Patch Available)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890942#action_12890942 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

Hm... seems to be an intermittent issue with NIO now:

a test got stuck in zookeeper client waiting for close to return - which it never did. The send thread is already gone. No logs available as junit timed out eventually and didn't write the logs as a result. 

Ben can you take a look at this?

"main" prio=10 tid=0x0000000041397800 nid=0x48c2 in Object.wait() [0x00007f17f5afd000]
   java.lang.Thread.State: WAITING (on object monitor)
        at java.lang.Object.wait(Native Method)
        - waiting on <0x00007f17d2125fd0> (a org.apache.zookeeper.ClientCnxn$Packet)
        at java.lang.Object.wait(Object.java:485)
        at org.apache.zookeeper.ClientCnxn.submitRequest(ClientCnxn.java:1007)
        - locked <0x00007f17d2125fd0> (a org.apache.zookeeper.ClientCnxn$Packet)
        at org.apache.zookeeper.ClientCnxn.close(ClientCnxn.java:985)
        at org.apache.zookeeper.ZooKeeper.close(ZooKeeper.java:534)
        - locked <0x00007f17d1ca5858> (a org.apache.zookeeper.test.DisconnectableZooKeeper)
        at org.apache.zookeeper.test.QuorumTest.testLeaderShutdown(QuorumTest.java:168)


> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

This is a refactoring of phunt's patch to separate the socket specific code into subclasses of ClientCnxnSocket and make ClientCnxn concrete again. IMHO this makes the code much more comprehendable.
I'm sorry, that eclipse made the patch a little larger then necessary. I've turned the auto-reformatting off now, but I don't know any easy way to undo the formatting changes.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920734#action_12920734 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

Sorry man, but I checked that, a few things indicate that it's nio and not netty:

1) The log would have something like this in it:
     [junit] Running org.apache.zookeeper.test.NettyNettySuiteHammerTest

which I don't see prior to this failure (running ... *Netty*

2) I can see log messages from NIO and not from netty in the log (NIO class loggers I mean)

3) I'm pretty sure (don't have one on hand) that if the *Netty*Test fails you see that in the stack trace.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

I did run the tests, but had some failures. But as far as I could see, these failures were in the server code, unrelated to my changes.
I did a basic test to create, delete and list nodes with this jar in the zk shell and this did work with NIO and Netty.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12913095#action_12913095 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

Saw an additional test failure on another machine, it failed with:

    [junit] Running org.apache.zookeeper.test.NettyNettySuiteTest
    [junit] Tests run: 1, Failures: 0, Errors: 1, Time elapsed: 0 sec
    [junit] Test org.apache.zookeeper.test.NettyNettySuiteTest FAILED (timeout)

That was the entire content of the log output for the test
 
However the prior test failed with a ton of warnings, including this which seems suspicious, it needs to be tracked down why this could happen:

    [junit] 2010-09-21 09:42:25,890 [myid:] - INFO  [New I/O server worker #32-3:ZooKeeperServer@801] - Client attempting to establish new session at /127.0.0.1:48780
    [junit] 2010-09-21 09:42:25,891 [myid:] - INFO  [SyncThread:0:ZooKeeperServer@577] - Established session 0x12b352cbcb80001 with negotiated timeout 30000 for client /127.0.0.1:48780
    [junit] 2010-09-21 09:42:25,900 [myid:] - INFO  [New I/O client worker #41-1:ClientCnxn$SendThread@904] - Session establishment complete on server localhost/127.0.0.1:11250, sessionid = 0x12b352cbcb80001, negotiated timeout = 30000
    [junit] 2010-09-21 09:42:26,205 [myid:] - WARN  [New I/O client worker #41-1:ClientCnxnSocketNetty$ZKClientHandler@289] - Exception caught [id: 0x66bb1ead, /127.0.0.1:48780 => localhost/127.0.0.1:11250] EXCEPTION: java.io.IOException: Nothing in the queue, but got 294
    [junit] java.io.IOException: Nothing in the queue, but got 294
    [junit] 	at org.apache.zookeeper.ClientCnxn$SendThread.readResponse(ClientCnxn.java:968)
    [junit] 	at org.apache.zookeeper.ClientCnxnSocketNetty$ZKClientHandler.messageReceived(ClientCnxnSocketNetty.java:270)
    [junit] 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:274)
    [junit] 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:261)
    [junit] 	at org.jboss.netty.channel.socket.nio.NioWorker.read(NioWorker.java:349)
    [junit] 	at org.jboss.netty.channel.socket.nio.NioWorker.processSelectedKeys(NioWorker.java:281)
    [junit] 	at org.jboss.netty.channel.socket.nio.NioWorker.run(NioWorker.java:201)
    [junit] 	at org.jboss.netty.util.internal.IoWorkerRunnable.run(IoWorkerRunnable.java:46)
    [junit] 	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
    [junit] 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
    [junit] 	at java.lang.Thread.run(Thread.java:619)


> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Open  (was: Patch Available)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

I changed the following things:

- call the correct cleanup() method. I f..ed up to refactor the call hierarchy
- combine doReads() and doWrites() again into doIO() in NIO as it was before
- all access to selector is synchronized on ClientCnxnSocket

I'm having problems to run the testsuite and believe that I'm somehow bitten by ZOOKEEPER-126. Some Tests just block and when I debug them, they always wait indefinitely on packet.wait().

Something like the following helped, however with every different attempt to solve ZOOKEEPER-126 other tests failed respectively blocked.

src/java/main/org/apache/zookeeper/ClientCnxn.java
@@ -1088,6 +1088,10 @@ public class ClientCnxn {
         ReplyHeader r = new ReplyHeader();
         Packet packet = queuePacket(h, r, request, response, null, null, null,
                 null, watchRegistration);
+        if(h.getType() == ZooDefs.OpCode.closeSession){
+            Thread.sleep((long) (sessionTimeout*1.1));
+            return r;
+        }
         synchronized (packet) {
             while (!packet.finished) {
                 packet.wait();

My tests still run now with the above code. Some take 10 minutes and more.

I also had to reduce the number of hammerthreads in org.apache.zookeeper.test.AsyncHammerTest to 10 to make it work.

I'm running openjdk 
java version "1.6.0_18"
OpenJDK Runtime Environment (IcedTea6 1.8.1) (6b18-1.8.1-1)
OpenJDK 64-Bit Server VM (build 16.0-b13, mixed mode)


> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: testDisconnectedAddAuth_FAILURE

I've built this patch 4 times now and had only ones a Failure in testDisconnectedAddAuth instead of multiple failures on every build. - Please commit to trunk and we'll also catch these last few issues.
Anyway, all observed failures were only seen with Netty and we could mark them as Experimental as long as we don't fully trust this code.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920715#action_12920715 ] 

Thomas Koch commented on ZOOKEEPER-823:
---------------------------------------

Hi Patrick,

I saw the same exceptions as you saw. They are not _thrown_ inside the netty code, but they occurred in my cases every time during the NettyNettySuiteTest. It isn't very obvious from the junit log, whether a test was run as part of the NettyNettySuiteTest or not. I do a forward search for "FAILED" and from this position a backwards search for "running" to find the name of the actual test suite.
Are you perfecty sure, the failed tests were not run as part of the NettyNettySuiteTest?

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920750#action_12920750 ] 

Thomas Koch commented on ZOOKEEPER-823:
---------------------------------------

The problem with the failing testWatchAutoResetWithPending might be the following. The test uses testableZooKeeper.pauseCnxn($milliSeconds) 
PauseCnxn tries to block the ClientCnxn by holding a lock on the ClientCnxn instance, interrupting the connection and not releasing the lock for $milliSeconds.
Actually I wonder how this could have ever worked reliable in the first place. The only thing I see that also synchronizes on ClientCnxn is the getXid() method.
The solution might be to find a better implementation for testableZooKeeper.pauseCnxn

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

updated patch to newest trunk state

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: testWatchAutoResetWithPending_FAILURE

I did 10 builds now in total with the last patch. 8 builds were succesful, 2 failed with different test cases, see the *_FAILURE files. Both failures happened in the NettyNettySuiteTest.
It's hard to debug this kind of Heisenbugs that happen only very seldom. It would make things easier, if I could start cleaning up the code to better identify spots where things could go wrong.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Attachment: TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz

log of test failure, all other tests passed though (incl nionetty) so it seems this is related to the netty client.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment:     (was: ZOOKEEPER-823.patch)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12914087#action_12914087 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

Unfortunately this change is one that touches some code that's very complex. As a result it's going to take some time to work out all the issues. Also given that it's refactoring classes into new classes it's hard to refactor other stuff before we get this in. This is not typical of our changes, but it does happen. Hopefully the other refactorings you are suggesting (Thomas) will help with this going fwd.

bq. phunt: NettyNettySuiteTest - ACLTest.testDisconnectedAddAuth(ACLTest.java:67):

You read that right. The conditions it tests are that the auth is handled properly when the connection is eventually established. There's another issue though, thats handling flow control on the session. Notice that during session establishment the NIO code disables recv after getting the session create request, then handles the auth request. This was a big problem (and might still be an issue with netty refactoring) with netty and may still have a bug or two. Notice that netty reads the entire tcp queue, not just a packet at a time as NIO does. as a result there is no way to do flow control (as we do with nio) at the socket level with netty. In the netty case notice that we have an additional layer on top where we queue the pending requests (ones that have been read from netty socket but not yet accepted by our clientcnxn code). I'd look at this for clues.

bq. phunt: NettyNettySuiteTest - AsyncHammerTest.testObserversHammer(AsyncHammerTest.java:213)

this fails for me every time I run it. I suspect that the join will never complete. This is probably due to the closing=true change for ZOOKEEPER-846. We must not be doing the right thing with that patch (846) naively merged into this patch. (I didn't see this error until I applied 846 to this patch)

Here are my machine specs for a machine where I see failures:
 2.6.18-164.15.1.el5 #1 SMP Wed Mar 17 11:30:06 EDT 2010 x86_64 x86_64 x86_64 GNU/Linux
16 cores:  model name      : Intel(R) Xeon(R) CPU           E5530  @ 2.40GHz


There's also this issue I reported:
https://issues.apache.org/jira/browse/ZOOKEEPER-823?focusedCommentId=12913095&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12913095


> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12913446#action_12913446 ] 

Thomas Koch commented on ZOOKEEPER-823:
---------------------------------------

With the latest trunk merged in, works on my machine. :-) BUILD SUCCESSFUL

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12907270#action_12907270 ] 

Thomas Koch commented on ZOOKEEPER-823:
---------------------------------------

Hi Ivan,

thank you for looking over the patch. I agree with all your comments but the last one on src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO:L110. Yes, the separate try-catches seem necessary to me.
I've added a link to issue ZOOKEEPER-702 about the incompatibility.
In general, it's easier to review patches that do small incremental changes. So I'd propose that you open separate jira issues for your observations so that we can work on them after this patch has been committed.
I already opened ZOOKEEPER-835 as an umbrella issue for several clean ups in the ZK java client code. Your issues would fit in there.



> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Open  (was: Patch Available)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Open  (was: Patch Available)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

wrapped access to ClientCnxn.authInfo in synchonized(authInfo). Did 13 builds with 6 failed builds.
Failed Tests:
org.apache.zookeeper.test.ACLTest.testAcls
org.apache.zookeeper.test.ChrootClientTest.testClientCleanup
org.apache.zookeeper.test.WatcherTest.testWatchAutoResetWithPending
org.apache.zookeeper.test.FLERestartTest.testLERestart
org.apache.zookeeper.test.WatcherTest.testWatchAutoResetWithPending
org.apache.zookeeper.test.QuorumTest.testFollowersStartAfterLeader
org.apache.zookeeper.test.QuorumTest.testNoLogBeforeLeaderEstablishment
org.apache.zookeeper.test.QuorumTest.testNull
org.apache.zookeeper.test.QuorumZxidSyncTest.testLateLogs

What puzzles me, is that also tests totally unrelated to the java client failed. My test machine is also a development server for the whole team, so its load varies heavily.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12904961#action_12904961 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

FYI: I tried acltest on the trunk w/o this patch and it passed.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Open  (was: Patch Available)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Open  (was: Patch Available)

Now this test is hanging for me (with most recent patch):

    [junit] Running org.apache.zookeeper.server.InvalidSnapshotTest

ZOOKEEPER-126 has never really been much of an issue - don't remember seeing it in tests. Could be that timing shifted, however I was not seeing any of this with my original patch.


> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

changes:

- call ClientCnxn.cleanup() from ClientCnxnSocketNIO.cleanup(), was lost during the refactoring
- cleaned the formatting changes to make the patch smaller

Now there are only three failures left:
NettyNettySuiteTest - ACLTest.testAcls
KeeperErrorCode = ConnectionLoss for /0
org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss for /0
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:90)
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:42)
	at org.apache.zookeeper.ZooKeeper.create(ZooKeeper.java:640)
	at org.apache.zookeeper.test.ACLTest.testAcls(ACLTest.java:104)
	at org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:51)

When I run the whole suite in eclipse as JUnit test, it does not fail.

NettyNettySuiteHammerTest - The log doesn't tell me anything, I assume it's just the same as in NettyNettySuiteTest

NioNettySuiteTest - ClientTest.testClientCleanup
open fds after test are not significantly higher than before
junit.framework.AssertionFailedError: open fds after test are not significantly higher than before
	at org.apache.zookeeper.test.ClientTest.testClientCleanup(ClientTest.java:731)
	at org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:51)

When I run the whole suite in eclipse, the test still fails, however when I run only ClientTest.testClientCleanup alone, it does not fail anymore.

I would really appreciate, if you could help me from now on. I double, partly triple checked the refactoring.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920737#action_12920737 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

Thomas (also Mahadev and other committers)

Here's an idea. What if we setup a branch in SVN with these changes (netty). It would be a short lived branch. Then we can setup a new job on apache hudson, and just keep hammering on that thing until we get all the issues (intermittent bugs) resolved. 

How's that sound?

An alternative would be to use Git but apache hudson doesn't allow that. I'd create/maintain the svn branch (you need to be a committer).

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

I did another version of the patch with an example how I'd solve the deadlock mentioned in my last comment. I made the synchronized blocks in doTransport and doWrites smaller.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Attachment: ZOOKEEPER-823.patch

Updated to patch against latest trunk.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12898157#action_12898157 ] 

Thomas Koch commented on ZOOKEEPER-823:
---------------------------------------

Hi Patrick,

please excuse me, but when it comes to clean code I'm a pain in the ass.

ClientCnxnFactory.createConnection (in both variants) looks up the name of the factory from System.getProperty(ZOOKEEPER_CLIENT_CNXN_FACTORY). I think this is ugly. It should not be the concern of createConnection where the name comes from (System property, config file, ...). It should receive the name as a parameter. You can then still have a helper method to look up the name from the system property.

Are the classes NIOClientCnxnFactory and NettyClientCnxnFactory really necessary? I'm not fluent enough in java, but this seems very verbose and unnecessarily confusing to me.

The two methods ClientCnxnFactory.createConnection are mostly cut&paste code. One should call the other with null values for long sessionId, byte[] sessionPasswd.

In NIOClientCnxn.cleanup() you check for LOG.isDebugEnabled(). This would be warranted only for log messages that are expensive to build and inside of code that's called often. Both conditions are not true in these cases.

Now that you've put all the socket related code in the two XXXClientCnxn packages it becomes obvious, that ClientCnxn actually has at least two responsibilities that could be separated in different classes. ClientCnxn could be split up in one class working with Packets, handling sockets and another class preparing and handling finished Packets. The first may be called abstract ClientCnxnSocket and the second remains ClientCnxn.
ClientCnxn could work with two types of ClientCnxnSocket: NioClientCnxnSocket and NettyClientCnxnSocket. The ClientCnxnSocket classes would probably not even be aware of that they are working for ZooKeeper. They just happily send and receive Packets and handle a Thread for that issue. I don't know, how much work it would mean for you to refactor your patch in this way, but I think it would clean up the code a lot. (And make all of us happy in the long run.)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12913660#action_12913660 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

I was also seeing some intermittent nettynetty failures previously. However now I'm seeing this each time (3 out of 3) I run on one of my beefier server boxes (8core)

junit.framework.AssertionFailedError: thread HammerThread-0 still alive after join
	at org.apache.zookeeper.test.ClientBase.verifyThreadTerminated(ClientBase.java:308)
	at org.apache.zookeeper.test.AsyncHammerTest.testObserversHammer(AsyncHammerTest.java:213)
	at org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:51)

this only showed up after merging in ZOOKEEPER-846, so I suspect that something has changed in this patch such that more than just applying 846 is necessary.

I'm pretty swamped, Thomas can you look at these issues? (incl the ones I/flavio cited earlier?)



> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920686#action_12920686 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

I ran the latest patch 4 times on my hudson setup and saw 3 failures (1 each):

java.util.concurrent.TimeoutException: Did not connect
	at org.apache.zookeeper.test.ClientBase$CountdownWatcher.waitForConnected(ClientBase.java:124)
	at org.apache.zookeeper.test.WatcherTest.testWatchAutoResetWithPending(WatcherTest.java:199)
	at org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:51)

org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss for /
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:90)
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:42)
	at org.apache.zookeeper.ZooKeeper.setACL(ZooKeeper.java:1259)
	at org.apache.zookeeper.test.ACLTest.testDisconnectedAddAuth(ACLTest.java:67)
	at org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:51)


    [junit] 2010-10-13 09:16:42,245 [myid:] - INFO  [main:JUnit4ZKTestRunner$LoggedInvokeMethod@53] - TEST METHOD FAILED testAcls
    [junit] org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss for /0
    [junit] 	at org.apache.zookeeper.KeeperException.create(KeeperException.java:90)
    [junit] 	at org.apache.zookeeper.KeeperException.create(KeeperException.java:42)
    [junit] 	at org.apache.zookeeper.ZooKeeper.create(ZooKeeper.java:640)
    [junit] 	at org.apache.zookeeper.test.ACLTest.testAcls(ACLTest.java:104)


If this was failing in the Netty code I'd be inclined to push the code to trunk and let things settle. However this is
in the mainline NIO code and I'm reticent to break existing functionality.


Perhaps a good idea would be to have Ben/Mahadev/Flavio review the patch and see if they notice any issues.

I'd really like to get this in (esp as it's blocking other changes), so if we could review this and resolve the issues asap that would be great.



> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

Corrected another synchronization issue. 
- write.awaitUninterruptibly() may not be in any synchronization block because otherwise we get timeouts.
- Writing to the netty queue and adding the package to the pendingQueue must be in a sync(pendingQueue), otherwise we can receive the answer to the package, try to work with it in readResponse() but don't find the package in the pendingQueue.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Open  (was: Patch Available)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12904959#action_12904959 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

I only had time to apply the patch and run the tests -- the latest patch (Thomas's changes) is failing in both ACLTest and NettyNettySuiteTest. All tests were passing for me when I created the original patches, either the trunk is broken or the latest changes have introduced some problem. Both of these failed tests heavily exercise the client code (nio/netty) so I suspect it's a real issue with the latest refactoring. You'll need to look into these more deeply.


> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Flavio Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12913591#action_12913591 ] 

Flavio Junqueira commented on ZOOKEEPER-823:
--------------------------------------------

NettyNettySuiteTest is failing intermittently for me. I'm attaching logs for a run that failed.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Flavio Junqueira (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Flavio Junqueira updated ZOOKEEPER-823:
---------------------------------------

    Attachment: NettyNettySuiteTest.rtf

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12919293#action_12919293 ] 

Thomas Koch commented on ZOOKEEPER-823:
---------------------------------------

How to continue with this?

I know have a couple of Heisenbugs that do not appear with every test run. Most often

    * org.apache.zookeeper.test.ACLTest.testAcls
    * org.apache.zookeeper.test.AsyncHammerTest.testObserversHammer
    * org.apache.zookeeper.test.AsyncHammerTest.testHammer

I have the following believing:

- there are some issues with missing / misplaced synchronization blocks in the Netty java client code
- there may also be some bugs in the server netty code. Sometimes I get bugs like this, which could also be caused on the server side, but I have not yet learned the ZK server code:

Exception caught [id: 0x19509443, /127.0.0.1:53300 => localhost/127.0.0.1:11270] EXCEPTION: java.io.IOException: Xid out of order. Got Xid 32 with err 0 expected Xid 33 for a packet with details:

I'd rather like to commit the current state to trunk and to continue the bug hunting in trunk. This would allow me to do some cleanups that could help to understand the correct synchronizations. This cleanups should be ZOOKEEPER-878 and ZOOKEEPER-879

This Exception by example could be better understood by solving the above mentioned issues:

java.lang.NullPointerException
	at org.apache.zookeeper.ClientCnxn.conLossPacket(ClientCnxn.java:574)
	at org.apache.zookeeper.ClientCnxn.access$2300(ClientCnxn.java:76)
	at org.apache.zookeeper.ClientCnxn$SendThread.cleanup(ClientCnxn.java:869)
	at org.apache.zookeeper.ClientCnxnSocketNetty.cleanup(ClientCnxnSocketNetty.java:147)
	at org.apache.zookeeper.ClientCnxn$SendThread.run(ClientCnxn.java:741)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment:     (was: ZOOKEEPER-823.patch)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Attachment: ZOOKEEPER-823.patch

This patch builds on the patch for ZOOKEEPER-733

Refactored ClientCnxn into common code, nettyclientcnxn and nioclientcnxn

tests are included specifically for netty client, however you can also run the full test suite along with the zookeeper.clientCnxnFactory option in order to run all the tests using netty.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921053#action_12921053 ] 

Mahadev konar commented on ZOOKEEPER-823:
-----------------------------------------

I am +1 with creating a branch for this issue and keep working on it until we have all the issues figured out and then merge this back in.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12913089#action_12913089 ] 

Patrick Hunt commented on ZOOKEEPER-823:
----------------------------------------

I just ran this against latest trunk and it fails with:

Testcase: testDisconnectedAddAuth took 2.167 sec
	Caused an ERROR
KeeperErrorCode = ConnectionLoss for /
org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss for /
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:90)
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:42)
	at org.apache.zookeeper.ZooKeeper.setACL(ZooKeeper.java:1259)
	at org.apache.zookeeper.test.ACLTest.testDisconnectedAddAuth(ACLTest.java:67)
	at org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:51)

(log attached)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment: ZOOKEEPER-823.patch

I may have fixed another issue:

I wrapped sendThread.readResponse(incomingBuffer) into a synchronization on the OutgoingQueue, because it might happen otherwise, that a package is send over netty and processed by the server, but not yet added to the pendingQueue. This fix solved all the Heisenbugs I saw.
However there's still a bug with ASyncHammer and that the wait to join threads times out. I added more Debugging information. The Thread that times out hangs on ClientCnxnSocketNetty.wakeupCnxn where it waits for the synchronized(outgoingQueue).
It seems that the outgoingQueue is already owned and blocked in the doWrites method, hanging on write.awaitUninterruptibly(). doWrites is called by doTransport where the synchronized(outgoingQueue) happens.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12930251#action_12930251 ] 

Hadoop QA commented on ZOOKEEPER-823:
-------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12457168/ZOOKEEPER-823.patch
  against trunk revision 1033155.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 32 new or modified tests.

    -1 patch.  The patch command could not apply the patch.

Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/13//console

This message is automatically generated.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Open  (was: Patch Available)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Attachment:     (was: ZOOKEEPER-823.patch)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12913954#action_12913954 ] 

Thomas Koch commented on ZOOKEEPER-823:
---------------------------------------

I can not reproduce any test failure on my system. So the reported failures from phunt and Junqueira are:

- phunt: NettyNettySuiteTest - ACLTest.testDisconnectedAddAuth(ACLTest.java:67):
org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss for /
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:90)
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:42)
	at org.apache.zookeeper.ZooKeeper.setACL(ZooKeeper.java:1259)

I don't understand, what exactly this test should test. A code comment would help. However I can imagine some race condition because the test does not wait for a client connection to be established before continuing.

- phunt: NettyNettySuiteTest - AsyncHammerTest.testObserversHammer(AsyncHammerTest.java:213)

I can't confirm this neither. However the test has some non determinism because it waits only 60 seconds for a thread to join.

- Junqueira: NettyNettySuiteTest - ClientTest.testMutipleWatcherObjs(ClientTest.java:241)

org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss for /foo-89

I can't really help without access to machines where the tests fail. @Flavio What are the specs of your test machine?

While investigating these failures I already felt the urge to continue my quest for clean code and refactor: ZOOKEEPER-878, ZOOKEEPER-879 However these two issues would blow up the patch even more.



> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-823:
-----------------------------------

    Attachment: ZOOKEEPER-823.patch

fixed a timing problem with nio introduced during the refactor
fixed an issue with netty disconnect testing.

added netty-netty tests (client-server both netty)


> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Patch Available  (was: Open)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: NettyNettySuiteTest.rtf, TEST-org.apache.zookeeper.test.NettyNettySuiteTest.txt.gz, testDisconnectedAddAuth_FAILURE, testWatchAutoResetWithPending_FAILURE, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Commented: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Ivan Kelly (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12907176#action_12907176 ] 

Ivan Kelly commented on ZOOKEEPER-823:
--------------------------------------

A few comments on the current patch.

src/java/main/org/apache/zookeeper/ClientCnxn.java:L709 
There's a lot of instanceof used with exceptions. Really you should give each exception it's own catch, and then put the common code in a finally clause.

src/java/main/org/apache/zookeeper/ClientCnxn.java:L920
This method has a lot of magic numbers being used. These should be defined as consts somewhere and used as such.

src/java/main/org/apache/zookeeper/ClientCnxn.java:L755
Theres a TODO about making SendThread an implementation of Runnable. This change should be fairly simple, so why not do it before submitting? 

src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO:L110
Are all these try/catches necessary? A single try catch around the whole lot would do. If you need to see where it happened, you can look at the stacktrace.


One other thing to keep in mind is that this patch with conflict with 702, so whichever get submitted second will have to be careful not to break the other's functionality.

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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


[jira] Updated: (ZOOKEEPER-823) update ZooKeeper java client to optionally use Netty for connections

Posted by "Thomas Koch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Koch updated ZOOKEEPER-823:
----------------------------------

    Status: Open  (was: Patch Available)

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
>
>
> This jira will port the client side connection code to use netty rather than direct nio.

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