From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id F33E1289A1 for ; Mon, 2 Sep 2019 10:51:20 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hqAQAGOXYZr1 for ; Mon, 2 Sep 2019 10:51:20 -0400 (EDT) Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id A9FC72896A for ; Mon, 2 Sep 2019 10:51:20 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v3 5/6] iproto: refactor error encoding with mpstream Date: Mon, 2 Sep 2019 17:51:13 +0300 Message-Id: <2492bd7b04f74228056aa0a06ec2e5c6ae0482ae.1567435674.git.kshcherbatov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, georgy@tarantool.org Cc: alexander.turenko@tarantool.org, kostja@tarantool.org, Kirill Shcherbatov Refactor iproto_reply_error and iproto_write_error with a new mpstream-based helper mpstream_iproto_encode_error that encodes error object for iproto protocol on a given stream object. Previously each routine implemented an own error encoding, but with the increasing complexity of encode operation with following patches we need a uniform way to do it. The iproto_write_error routine starts using region location to use region-based mpstream. It is not a problem itself, because errors reporting is not really performance-critical path. Needed for #1148 --- src/box/xrow.c | 72 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 21 deletions(-) diff --git a/src/box/xrow.c b/src/box/xrow.c index 78462469a..e1e0a62be 100644 --- a/src/box/xrow.c +++ b/src/box/xrow.c @@ -36,12 +36,14 @@ #include "third_party/base64.h" #include "fiber.h" +#include "reflection.h" #include "version.h" #include "tt_static.h" #include "error.h" #include "vclock.h" #include "scramble.h" #include "iproto_constants.h" +#include "mpstream.h" static_assert(IPROTO_DATA < 0x7f && IPROTO_METADATA < 0x7f && IPROTO_SQL_INFO < 0x7f, "encoded IPROTO_BODY keys must fit into "\ @@ -381,10 +383,6 @@ static const struct iproto_body_bin iproto_body_bin = { 0x81, IPROTO_DATA, 0xdd, 0 }; -static const struct iproto_body_bin iproto_error_bin = { - 0x81, IPROTO_ERROR, 0xdb, 0 -}; - /** Return a 4-byte numeric error code, with status flags. */ static inline uint32_t iproto_encode_error(uint32_t error) @@ -475,46 +473,78 @@ iproto_reply_vote(struct obuf *out, const struct ballot *ballot, return 0; } +static void +mpstream_error_handler(void *error_ctx) +{ + *(bool *)error_ctx = true; +} + +static void +mpstream_iproto_encode_error(struct mpstream *stream, const struct error *error) +{ + mpstream_encode_map(stream, 2); + mpstream_encode_uint(stream, IPROTO_ERROR); + mpstream_encode_str(stream, error->errmsg); +} + int iproto_reply_error(struct obuf *out, const struct error *e, uint64_t sync, uint32_t schema_version) { - uint32_t msg_len = strlen(e->errmsg); - uint32_t errcode = box_error_code(e); - - struct iproto_body_bin body = iproto_error_bin; char *header = (char *)obuf_alloc(out, IPROTO_HEADER_LEN); if (header == NULL) return -1; + /* The obuf-based stream has reserved area for header. */ + bool is_error = false; + struct mpstream stream; + mpstream_init(&stream, out, obuf_reserve_cb, obuf_alloc_cb, + mpstream_error_handler, &is_error); + + uint32_t used = obuf_size(out); + mpstream_iproto_encode_error(&stream, e); + mpstream_flush(&stream); + + uint32_t errcode = box_error_code(e); iproto_header_encode(header, iproto_encode_error(errcode), sync, - schema_version, sizeof(body) + msg_len); - body.v_data_len = mp_bswap_u32(msg_len); + schema_version, obuf_size(out) - used); + /* Malformed packet appears to be a lesser evil than abort. */ - return obuf_dup(out, &body, sizeof(body)) != sizeof(body) || - obuf_dup(out, e->errmsg, msg_len) != msg_len ? -1 : 0; + return is_error ? -1 : 0; } void iproto_write_error(int fd, const struct error *e, uint32_t schema_version, uint64_t sync) { - uint32_t msg_len = strlen(e->errmsg); + bool is_error = false; + struct mpstream stream; + struct region *region = &fiber()->gc; + mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb, + mpstream_error_handler, &is_error); + + size_t region_svp = region_used(region); + mpstream_iproto_encode_error(&stream, e); + mpstream_flush(&stream); + if (is_error) + goto cleanup; + + size_t payload_size = region_used(region) - region_svp; + char *payload = region_join(region, payload_size); + if (payload == NULL) + goto cleanup; + uint32_t errcode = box_error_code(e); - char header[IPROTO_HEADER_LEN]; - struct iproto_body_bin body = iproto_error_bin; - iproto_header_encode(header, iproto_encode_error(errcode), sync, - schema_version, sizeof(body) + msg_len); - - body.v_data_len = mp_bswap_u32(msg_len); + schema_version, payload_size); ssize_t unused; unused = write(fd, header, sizeof(header)); - unused = write(fd, &body, sizeof(body)); - unused = write(fd, e->errmsg, msg_len); + unused = write(fd, payload, payload_size); (void) unused; +cleanup: + region_truncate(region, region_svp); } int -- 2.22.1