From: Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org Subject: [Tarantool-patches] [PATCH 2/2] iproto: clear request::header for client requests Date: Mon, 9 Aug 2021 13:40:28 +0300 [thread overview] Message-ID: <5850df69f9f8f9fadc7cb85d53b75c9d79c7ff46.1628505014.git.vdavydov@tarantool.org> (raw) In-Reply-To: <2d5889e419f7c6a35d1c4394a42b0f4b5ff62a68.1628505014.git.vdavydov@tarantool.org> To apply a client request, we only need to know its type and body. All the meta information, such as LSN, TSN, or replica id, must be set by WAL. Currently, however, it isn't necessarily true: iproto leaves a request header received over iproto as is, and tx will reuse the header instead of allocating a new one in this case, which is needed to process replication requests, see txn_add_redo(). Unless a client actually sets one of those meta fields, this causes no problems. However, if we added transaction support to the replication protocol, reusing the header would result in broken xlog, because currently, all requests received over iproto have the is_commit field set in xrow_header for the lack of TSN, while is_commit must only be set for the final statement in a transaction. One way to fix it would be clearing is_commit explicitly in iproto, but ignoring the whole header received over iproto looks more logical and error-proof. Needed for #5860 --- https://github.com/tarantool/tarantool/tree/vdavydov/iproto-clear-request-header src/box/iproto.cc | 6 ++++++ src/box/xrow.h | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/box/iproto.cc b/src/box/iproto.cc index 5cc69b77ff93..dcf60e1be099 100644 --- a/src/box/iproto.cc +++ b/src/box/iproto.cc @@ -1264,6 +1264,12 @@ iproto_msg_decode(struct iproto_msg *msg, const char **pos, const char *reqend, if (xrow_decode_dml(&msg->header, &msg->dml, dml_request_key_map(type))) goto error; + /* + * In contrast to replication requests, for a client request + * the xrow header is set by WAL, which generates LSNs and sets + * replica id. Ignore the header received over network. + */ + msg->dml.header = NULL; assert(type < sizeof(iproto_thread->dml_route) / sizeof(*(iproto_thread->dml_route))); cmsg_init(&msg->base, iproto_thread->dml_route[type]); diff --git a/src/box/xrow.h b/src/box/xrow.h index 0f2fcf94acfc..48b8b55f5e9b 100644 --- a/src/box/xrow.h +++ b/src/box/xrow.h @@ -163,7 +163,7 @@ struct request { /* * Either log row, or network header, or NULL, depending * on where this packet originated from: the write ahead - * log/snapshot, client request, or a Lua request. + * log/snapshot, repliation, or a client request. */ struct xrow_header *header; /** -- 2.25.1
next prev parent reply other threads:[~2021-08-09 10:41 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-05 18:17 [Tarantool-patches] [PATCH 0/7] implement iproto streams mechanik20051988 via Tarantool-patches 2021-08-05 18:17 ` [Tarantool-patches] [PATCH 1/7] iproto: implement stream id in binary iproto protocol mechanik20051988 via Tarantool-patches 2021-08-06 8:20 ` Vladimir Davydov via Tarantool-patches 2021-08-05 18:17 ` [Tarantool-patches] [PATCH 2/7] salad: fix segfault in case when mhash table allocation failure mechanik20051988 via Tarantool-patches 2021-08-06 8:33 ` Vladimir Davydov via Tarantool-patches 2021-08-05 18:17 ` [Tarantool-patches] [PATCH 3/7] txn: detach transaction from fiber mechanik20051988 via Tarantool-patches 2021-08-06 8:51 ` Vladimir Davydov via Tarantool-patches 2021-08-05 18:17 ` [Tarantool-patches] [PATCH 4/7] iproto: implement streams in iproto mechanik20051988 via Tarantool-patches 2021-08-06 10:30 ` Vladimir Davydov via Tarantool-patches 2021-08-05 18:17 ` [Tarantool-patches] [PATCH 5/7] net.box: add stream support to net.box mechanik20051988 via Tarantool-patches 2021-08-06 12:03 ` Vladimir Davydov via Tarantool-patches 2021-08-05 18:17 ` [Tarantool-patches] [PATCH 6/7] iproto: implement interactive transactions over iproto streams mechanik20051988 via Tarantool-patches 2021-08-06 12:59 ` Vladimir Davydov via Tarantool-patches 2021-08-09 10:39 ` Vladimir Davydov via Tarantool-patches 2021-08-09 10:40 ` [Tarantool-patches] [PATCH 1/2] xrow: remove unused call_request::header Vladimir Davydov via Tarantool-patches 2021-08-09 10:40 ` Vladimir Davydov via Tarantool-patches [this message] 2021-08-09 11:27 ` [Tarantool-patches] [PATCH 2/2] iproto: clear request::header for client requests Evgeny Mekhanik via Tarantool-patches 2021-08-09 11:26 ` [Tarantool-patches] [PATCH 1/2] xrow: remove unused call_request::header Evgeny Mekhanik via Tarantool-patches 2021-08-05 18:17 ` [Tarantool-patches] [PATCH 7/7] net.box: add interactive transaction support in net.box mechanik20051988 via Tarantool-patches 2021-08-06 14:04 ` Vladimir Davydov via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5850df69f9f8f9fadc7cb85d53b75c9d79c7ff46.1628505014.git.vdavydov@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --cc=vdavydov@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 2/2] iproto: clear request::header for client requests' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox