Re: [Tarantool-patches] [PATCH 2/2] iproto: clear request::header for client requests

[Evgeny Mekhanik via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]


 
  
>Понедельник, 9 августа 2021, 13:40 +03:00 от Vladimir Davydov <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
LGTM

[-- Attachment #2: Type: text/html, Size: 3094 bytes --]