From commits-return-8134-archive-asf-public=cust-asf.ponee.io@zookeeper.apache.org Tue Jan 28 14:22:39 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id C620E180658 for ; Tue, 28 Jan 2020 15:22:38 +0100 (CET) Received: (qmail 88793 invoked by uid 500); 28 Jan 2020 14:22:38 -0000 Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zookeeper.apache.org Delivered-To: mailing list commits@zookeeper.apache.org Received: (qmail 88780 invoked by uid 99); 28 Jan 2020 14:22:37 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 28 Jan 2020 14:22:37 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id B19CA8D845; Tue, 28 Jan 2020 14:22:37 +0000 (UTC) Date: Tue, 28 Jan 2020 14:22:37 +0000 To: "commits@zookeeper.apache.org" Subject: [zookeeper] branch master updated: ZOOKEEPER-1105: wait for server response in C client zookeeper_close MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <158022135757.22973.8577828698260350938@gitbox.apache.org> From: nkalmar@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: zookeeper X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: e4758ba70b35dd38ff0bba9e534790391f53b050 X-Git-Newrev: 57be7aedd698cb824ade7373aaa4d8264e1b4eb7 X-Git-Rev: 57be7aedd698cb824ade7373aaa4d8264e1b4eb7 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. nkalmar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/zookeeper.git The following commit(s) were added to refs/heads/master by this push: new 57be7ae ZOOKEEPER-1105: wait for server response in C client zookeeper_close 57be7ae is described below commit 57be7aedd698cb824ade7373aaa4d8264e1b4eb7 Author: Mate Szalay-Beko AuthorDate: Tue Jan 28 15:22:30 2020 +0100 ZOOKEEPER-1105: wait for server response in C client zookeeper_close **Thanks for Lincoln Lee for the original fix!** In the current implementation, we always get a WARN in server side ("EndOfStreamException: Unable to read additional data from client") whenever we close a zookeeper handler from the C client. This also happens in the end of every execution of the command line C client. The reason is that currently we don't wait for the response from the server when we initiate the closing of the client connection, and we terminate the socket on the client side too early. I tested the patch both on linux and windows. I also tested it both with NIO and Netty server side socket implementations. Author: Mate Szalay-Beko Reviewers: Andor Molnar , Norbert Kalmar Closes #1176 from symat/ZOOKEEPER-1105 --- .../zookeeper-client-c/src/zookeeper.c | 70 ++++++++++++++++++---- 1 file changed, 57 insertions(+), 13 deletions(-) diff --git a/zookeeper-client/zookeeper-client-c/src/zookeeper.c b/zookeeper-client/zookeeper-client-c/src/zookeeper.c index db42d6c..5f3cb5d 100644 --- a/zookeeper-client/zookeeper-client-c/src/zookeeper.c +++ b/zookeeper-client/zookeeper-client-c/src/zookeeper.c @@ -3644,6 +3644,49 @@ static int add_multi_completion(zhandle_t *zh, int xid, void_completion_t dc, return add_completion(zh, xid, COMPLETION_MULTI, dc, data, 0,0, clist); } +/** + * After sending the close request, we are waiting for a given millisecs for + * getting the answer and/or for the socket to be closed by the server. + * + * This function should not be called while we still want to process + * any response from the server. It must be called after adaptor_finish called, + * in order not to mess with the I/O receiver thread in multi-threaded mode. + */ +int wait_for_session_to_be_closed(zhandle_t *zh, int timeout_ms) +{ + int ret = 0; +#ifndef WIN32 + struct pollfd fd_s[1]; +#else + fd_set rfds; + struct timeval waittime = {timeout_ms / 1000, (timeout_ms % 1000) * 1000}; +#endif + + if (zh == NULL) { + return ZBADARGUMENTS; + } + +#ifndef WIN32 + fd_s[0].fd = zh->fd->sock; + fd_s[0].events = POLLIN; + ret = poll(fd_s, 1, timeout_ms); +#else + FD_ZERO(&rfds); + FD_SET(zh->fd->sock , &rfds); + ret = select(zh->fd->sock + 1, &rfds, NULL, NULL, &waittime); +#endif + + if (ret == 0){ + LOG_WARN(LOGCALLBACK(zh), "Timed out (%dms) during waiting for server's reply after sending a close request, sessionId=%#llx\n", + timeout_ms, zh->client_id.client_id); + } else if (ret < 0) { + LOG_WARN(LOGCALLBACK(zh), "System error (%d) happened while waiting for server's reply, sessionId=%#llx\n", + ret, zh->client_id.client_id); + } + + return ZOK; +} + int zookeeper_close(zhandle_t *zh) { int rc=ZOK; @@ -3669,32 +3712,33 @@ int zookeeper_close(zhandle_t *zh) } /* No need to decrement the counter since we're just going to * destroy the handle later. */ - if (is_connected(zh)){ + if (is_connected(zh)) { struct oarchive *oa; struct RequestHeader h = {get_xid(), ZOO_CLOSE_OP}; LOG_INFO(LOGCALLBACK(zh), "Closing zookeeper sessionId=%#llx to %s\n", - zh->client_id.client_id,zoo_get_current_server(zh)); + zh->client_id.client_id, zoo_get_current_server(zh)); oa = create_buffer_oarchive(); rc = serialize_RequestHeader(oa, "header", &h); - rc = rc < 0 ? rc : queue_buffer_bytes(&zh->to_send, get_buffer(oa), - get_buffer_len(oa)); + rc = rc < 0 ? rc : queue_buffer_bytes(&zh->to_send, get_buffer(oa), get_buffer_len(oa)); /* We queued the buffer, so don't free it */ close_buffer_oarchive(&oa, 0); if (rc < 0) { + LOG_DEBUG(LOGCALLBACK(zh), "Error during closing zookeeper session, sessionId=%#llx to %s (error: %d)\n", + zh->client_id.client_id, zoo_get_current_server(zh), rc); rc = ZMARSHALLINGERROR; - goto finish; - } + } else { + /* make sure the close request is sent; we set timeout to an arbitrary + * (but reasonable) number of milliseconds since we want the call to block*/ + rc = adaptor_send_queue(zh, 3000); - /* make sure the close request is sent; we set timeout to an arbitrary - * (but reasonable) number of milliseconds since we want the call to block*/ - rc=adaptor_send_queue(zh, 3000); - }else{ - LOG_INFO(LOGCALLBACK(zh), "Freeing zookeeper resources for sessionId=%#llx\n", - zh->client_id.client_id); + /* give some time to the server to process the session close request properly */ + rc = rc < 0 ? rc : wait_for_session_to_be_closed(zh, 1500); + } + } else { rc = ZOK; } -finish: + LOG_INFO(LOGCALLBACK(zh), "Freeing zookeeper resources for sessionId=%#llx\n", zh->client_id.client_id); destroy(zh); adaptor_destroy(zh); free(zh->fd);