zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From h...@apache.org
Subject [zookeeper] branch branch-3.4 updated: ZOOKEEPER-1636: cleanup completion list of a failed multi request
Date Thu, 01 Aug 2019 23:45:33 GMT
This is an automated email from the ASF dual-hosted git repository.

hanm pushed a commit to branch branch-3.4
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/branch-3.4 by this push:
     new 80444bc  ZOOKEEPER-1636: cleanup completion list of a failed multi request
80444bc is described below

commit 80444bc4ce1f164764afd31039aae760356eca14
Author: Alexander A. Strelets <streletsaa@gmail.com>
AuthorDate: Thu Aug 1 16:45:19 2019 -0700

    ZOOKEEPER-1636: cleanup completion list of a failed multi request
    
    This is a backport of https://github.com/apache/zookeeper/pull/717 to `branch-3.4`
    
    For some reason it was NOT merged to `branch-3.4`, but only to `branch-3.5` and `master`:
    - 4bd32bef2b75238e67b13bdfc7e6896d64c32434
    - b1fd480b2c8e0cc1429345ee04510d3849001c5c
    
    Here, it's almost a cherry-pick of the original patch, but slightly adopted to specific
of `branch-3.4`.
    Namely, `cleanup_failed_multi()` and `deserialize_multi()` do not have the first parameter
`zhandle_t *zh`.
    
    ----
    
    I wish to backport this also to use it as the base for my https://github.com/apache/zookeeper/pull/999
    
    Author: Alexander A. Strelets <streletsaa@gmail.com>
    
    Reviewers: Andor Molnár <andor@apache.org>, Michael Han <hanm@apache.org>
    
    Closes #1030 from xoiss/ZOOKEEPER-1636
---
 .../zookeeper-client-c/src/zookeeper.c             | 17 +++++-
 .../zookeeper-client-c/tests/TestMulti.cc          | 68 ++++++++++++++++++++++
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/zookeeper-client/zookeeper-client-c/src/zookeeper.c b/zookeeper-client/zookeeper-client-c/src/zookeeper.c
index 1e780ff..9479719 100644
--- a/zookeeper-client/zookeeper-client-c/src/zookeeper.c
+++ b/zookeeper-client/zookeeper-client-c/src/zookeeper.c
@@ -2021,6 +2021,17 @@ static void process_sync_completion(
     }
 }
 
+// cleanup completion list of a failed multi request
+static void cleanup_failed_multi(int xid, int rc, completion_list_t *cptr) {
+    completion_list_t *entry;
+    completion_head_t *clist = &cptr->c.clist;
+    while ((entry = dequeue_completion(clist)) != NULL) {
+        // Fake failed response for all sub-requests
+        deserialize_response(entry->c.type, xid, 1, rc, entry, NULL);
+        destroy_completion_entry(entry);
+    }
+}
+
 static int deserialize_multi(int xid, completion_list_t *cptr, struct iarchive *ia)
 {
     int rc = 0;
@@ -2138,8 +2149,12 @@ static void deserialize_response(int type, int xid, int failed, int
rc, completi
     case COMPLETION_MULTI:
         LOG_DEBUG(("Calling COMPLETION_MULTI for xid=%#x failed=%d rc=%d",
                     cptr->xid, failed, rc));
-        rc = deserialize_multi(xid, cptr, ia);
         assert(cptr->c.void_result);
+        if (failed) {
+            cleanup_failed_multi(xid, rc, cptr);
+        } else {
+            rc = deserialize_multi(xid, cptr, ia);
+        }
         cptr->c.void_result(rc, cptr->data);
         break;
     default:
diff --git a/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc b/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc
index a54e047..e9172a3 100644
--- a/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc
+++ b/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc
@@ -178,6 +178,7 @@ class Zookeeper_multi : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testCheck);
     CPPUNIT_TEST(testWatch);
     CPPUNIT_TEST(testSequentialNodeCreateInAsyncMulti);
+    CPPUNIT_TEST(testBigAsyncMulti);
 #endif
     CPPUNIT_TEST_SUITE_END();
 
@@ -249,6 +250,16 @@ public:
         count++;
     }
 
+    static void multi_completion_fn_rc(int rc, const void *data) {
+        count++;
+        *((int*) data) = rc;
+    }
+
+     static void create_completion_fn_rc(int rc, const char* value, const void *data) {
+        count++;
+        *((int*) data) = rc;
+    }
+
     static void waitForMultiCompletion(int seconds) {
         time_t expires = time(0) + seconds;
         while(count == 0 && time(0) < expires) {
@@ -656,6 +667,63 @@ public:
         waitForMultiCompletion(5);
      }
 
+    /**
+     * ZOOKEEPER-1636: If request is too large, the server will cut the
+     * connection without sending response packet. The client will try to
+     * process completion on multi request and eventually cause SIGSEGV
+     */
+    void testBigAsyncMulti() {
+        int rc;
+        int callback_rc = (int) ZOK;
+        watchctx_t ctx;
+        zhandle_t *zk = createClient(&ctx);
+
+        // The request should be more than 1MB which exceeds the default
+        // jute.maxbuffer and causes the server to drop client connection
+        const int iteration = 500;
+        const int type_count = 3;
+        const int nops = iteration * type_count;
+        char buff[1024];
+
+        zoo_op_result_t results[nops];
+        zoo_op_t ops[nops];
+        struct Stat* s[nops];
+        int index = 0;
+
+        // Test that we deliver error to 3 types of sub-request
+        for (int i = 0; i < iteration; ++i) {
+            zoo_set_op_init(&ops[index++], "/x", buff, sizeof(buff), -1, s[i]);
+            zoo_create_op_init(&ops[index++], "/x", buff, sizeof(buff),
+                               &ZOO_OPEN_ACL_UNSAFE, ZOO_SEQUENCE, NULL, 0);
+            zoo_delete_op_init(&ops[index++], "/x", -1);
+        }
+
+        rc = zoo_amulti(zk, nops, ops, results, multi_completion_fn_rc,
+                        &callback_rc);
+        CPPUNIT_ASSERT_EQUAL((int) ZOK, rc);
+
+        waitForMultiCompletion(10);
+        // With the bug, we will get SIGSEGV before reaching this point
+        CPPUNIT_ASSERT_EQUAL((int) ZCONNECTIONLOSS, callback_rc);
+
+        // Make sure that all sub-request completions get processed
+        for (int i = 0; i < nops; ++i) {
+            CPPUNIT_ASSERT_EQUAL((int) ZCONNECTIONLOSS, results[i].err);
+        }
+
+        // The handle should be able to recover itself.
+        ctx.waitForConnected(zk);
+
+        // Try to submit another async request to see if it get processed
+        // correctly
+        rc = zoo_acreate(zk, "/target", "", 0, &ZOO_OPEN_ACL_UNSAFE, 0,
+                         create_completion_fn_rc, &callback_rc);
+        CPPUNIT_ASSERT_EQUAL((int) ZOK, rc);
+
+        waitForMultiCompletion(10);
+        CPPUNIT_ASSERT_EQUAL((int) ZOK, callback_rc);
+    }
+
      /**
       * ZOOKEEPER-1624: PendingChanges of create sequential node request didn't
       * get rollbacked correctly when multi-op failed. This caused


Mime
View raw message