From da1892a9f053200947468364d0b89464cf1e8d0a Mon Sep 17 00:00:00 2001
From: Nikita Pettik <kitnerh@gmail.com>
Date: Thu, 28 Apr 2022 16:55:17 +0300
Subject: [PATCH] box/iproto: introduce reqstart to iproto_msg
Previously we decided to pass `msg->p_inbuf->rpos` to flight recorder in
order to dump request. However, this is wrong way to do it since `rpos`
is moved only request has been processed, i.e. at the same moment several
requests may have the same `rpos` value. To fix this let's save the start
of unparsed request to `iproto_msg`. There's one detail that should be
clarified to understand that in this case we'll always get valid
pointer to ibuf. Imagine following state of ibuf:
```
+_____________________
| | | |
+---------------------
^ ^ ^
RPOS R2 WPOS
R1
```
R1 is the first request in the buffer (i.e. rpos points to it);
R2 is the second. One can argue that if R2 is processed faster than R1
than during the dump of R1 its `reqstart` may point to the garbage.
However, dump of request takes place at the bery beginning of request
execution in TX thread. As far as messages are started to be processed
exactly in the same way as they were received, then R1 will be always
dumped before R2 and its further processing.
Follow-up 247515e98
NO_DOC=<No user visible changes>
NO_TEST=<No functional changes>
NO_CHANGELOG=<No functional changes>
---
src/box/flightrec.h | 4 ++--
src/box/iproto.cc | 12 +++++++++++-
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/box/flightrec.h b/src/box/flightrec.h
index 0544e071e7..259c9a7446 100644
--- a/src/box/flightrec.h
+++ b/src/box/flightrec.h
@@ -68,9 +68,9 @@ flightrec_check_cfg(int64_t logs_size, int64_t logs_max_msg_size,
/** No-op in OS version. */
static inline void
-flightrec_write_request(char *request_mspack, size_t len)
+flightrec_write_request(const char *request_msgpack, size_t len)
{
- (void)request_mspack;
+ (void)request_msgpack;
(void)len;
}
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 8a457239ad..1b7ee72cf2 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -356,6 +356,15 @@ struct iproto_msg
* ibuf object.
*/
size_t len;
+ /**
+ * Pointer to the start of unparsed request stored in @a p_ibuf.
+ * It is used to dump request to flight recorder (if available) in
+ * TX thread. It is guaranteed that @a reqstart points to the valid
+ * position: rpos of input buffer is moved after processing the message;
+ * meanwhile requests are handled in the order they are stored in
+ * the buffer.
+ */
+ const char *reqstart;
/**
* Position in the connection output buffer. When sending a
* message to the tx thread, iproto sets it to its current
@@ -1140,6 +1149,7 @@ iproto_enqueue_batch(struct iproto_connection *con, struct ibuf *in)
return 0;
}
msg->p_ibuf = con->p_ibuf;
+ msg->reqstart = reqstart;
msg->wpos = con->wpos;
msg->len = reqend - reqstart; /* total request length */
@@ -1884,7 +1894,7 @@ tx_accept_msg(struct cmsg *m)
msg->connection->iproto_thread->tx.requests_in_progress++;
rmean_collect(msg->connection->iproto_thread->tx.rmean,
REQUESTS_IN_PROGRESS, 1);
- flightrec_write_request(msg->p_ibuf->rpos, msg->len);
+ flightrec_write_request(msg->reqstart, msg->len);
return msg;
}
--
GitLab