Author: phunt Date: Thu Nov 19 21:19:44 2009 New Revision: 882300 URL: http://svn.apache.org/viewvc?rev=882300&view=rev Log: ZOOKEEPER-562. c client can flood server with pings if tcp send queue filled.
Modified: hadoop/zookeeper/branches/branch-3.1/CHANGES.txt hadoop/zookeeper/branches/branch-3.1/src/c/src/zookeeper.c hadoop/zookeeper/branches/branch-3.1/src/c/tests/TestOperations.cc Modified: hadoop/zookeeper/branches/branch-3.1/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/zookeeper/branches/branch-3.1/CHANGES.txt?rev=882300&r1=882299&r2=882300&view=diff ============================================================================== --- hadoop/zookeeper/branches/branch-3.1/CHANGES.txt (original) +++ hadoop/zookeeper/branches/branch-3.1/CHANGES.txt Thu Nov 19 21:19:44 2009 @@ -1,3 +1,13 @@ +Release 3.1.2 - 2009-11-24 + +Backward compatibile changes: + +BUGFIXES: + + ZOOKEEPER-562. c client can flood server with pings if tcp send queue + filled. (ben reed via mahadev) + + Release 3.1.1 - 2009-03-17 Backward compatibile changes: Modified: hadoop/zookeeper/branches/branch-3.1/src/c/src/zookeeper.c URL: http://svn.apache.org/viewvc/hadoop/zookeeper/branches/branch-3.1/src/c/src/zookeeper.c?rev=882300&r1=882299&r2=882300&view=diff ============================================================================== --- hadoop/zookeeper/branches/branch-3.1/src/c/src/zookeeper.c (original) +++ hadoop/zookeeper/branches/branch-3.1/src/c/src/zookeeper.c Thu Nov 19 21:19:44 2009 @@ -1181,7 +1181,7 @@ // a PING if (zh->state==ZOO_CONNECTED_STATE) { send_to = zh->recv_timeout/3 - idle_send; - if (send_to <= 0) { + if (send_to <= 0 && zh->sent_requests.head==0) { // LOG_DEBUG(("Sending PING to %s (exceeded idle by %dms)", // format_current_endpoint_info(zh),-send_to)); int rc=send_ping(zh); Modified: hadoop/zookeeper/branches/branch-3.1/src/c/tests/TestOperations.cc URL: http://svn.apache.org/viewvc/hadoop/zookeeper/branches/branch-3.1/src/c/tests/TestOperations.cc?rev=882300&r1=882299&r2=882300&view=diff ============================================================================== --- hadoop/zookeeper/branches/branch-3.1/src/c/tests/TestOperations.cc (original) +++ hadoop/zookeeper/branches/branch-3.1/src/c/tests/TestOperations.cc Thu Nov 19 21:19:44 2009 @@ -265,6 +265,27 @@ // only one ping so far? CPPUNIT_ASSERT_EQUAL(1,zkServer.pingCount_); CPPUNIT_ASSERT(timeMock==zh->last_recv); + + // Round 4 + // make sure that a ping is not sent if something is outstanding + AsyncGetOperationCompletion res1; + rc=zoo_aget(zh,"/x/y/1",0,asyncCompletion,&res1); + CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); + rc=zookeeper_interest(zh,&fd,&interest,&tv); + CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); + timeMock.tick(tv); + rc=zookeeper_interest(zh,&fd,&interest,&tv); + CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); + rc=zookeeper_process(zh,interest); + CPPUNIT_ASSERT_EQUAL((int)ZNOTHING,rc); + rc=zookeeper_interest(zh,&fd,&interest,&tv); + CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); + // pseudo-sleep for a short while (10 ms) + timeMock.millitick(10); + rc=zookeeper_process(zh,interest); + CPPUNIT_ASSERT_EQUAL((int)ZNOTHING,rc); + // only one ping so far? + CPPUNIT_ASSERT_EQUAL(1,zkServer.pingCount_); } // simulate a watch arriving right before a ping is due @@ -332,6 +353,7 @@ // queue up a request; keep it pending (as if the server is busy or has died) AsyncGetOperationCompletion res1; + zkServer.addOperationResponse(new ZooGetResponse("2",1)); int rc=zoo_aget(zh,"/x/y/1",0,asyncCompletion,&res1); int fd=0; @@ -344,7 +366,7 @@ CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); // no delay -- the socket is writable rc=zookeeper_process(zh,interest); - CPPUNIT_ASSERT_EQUAL((int)ZNOTHING,rc); // ZNOTHING -- no response yet + CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); // Round 2. // what's next?