[patches] [PATCH 1/1] iproto: get iproto obuf only right before usage
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Mar 15 00:31:08 MSK 2018
Branch: https://github.com/tarantool/tarantool/tree/fix-iproto-buffers
There is no issue, since the fix is simple, and does not affect
external behavior.
It is possible to discard non-sent responses using a special
sequence of requests and yields. In details: if DML requests
yield on commit too long, there are fast read requests, and a
network is saturated, then some non-sent DML responses are
discarded.
Signed-off-by: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
---
src/box/iproto.cc | 14 +++--
src/errinj.h | 1 +
test/box/errinj.result | 129 +++++++++++++++++++++++++++++++++++++++++++++++
test/box/errinj.test.lua | 60 ++++++++++++++++++++++
4 files changed, 200 insertions(+), 4 deletions(-)
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 37313fa52..d6ec62973 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -59,6 +59,7 @@
#include "replication.h" /* instance_uuid */
#include "iproto_constants.h"
#include "rmean.h"
+#include "errinj.h"
/* The number of iproto messages in flight */
enum { IPROTO_MSG_MAX = 768 };
@@ -749,6 +750,7 @@ iproto_flush(struct iproto_connection *con)
/* *Overwrite* iov_len of the last pos as it may be garbage. */
iov[iovcnt-1].iov_len = end->iov_len - begin->iov_len * (iovcnt == 1);
+ ERROR_INJECT(ERRINJ_IPROTO_FLUSH_DELAY, { usleep(100); return -1; });
ssize_t nwr = sio_writev(fd, iov, iovcnt);
/* Count statistics */
@@ -1167,7 +1169,6 @@ static void
tx_process1(struct cmsg *m)
{
struct iproto_msg *msg = tx_accept_msg(m);
- struct obuf *out = msg->connection->tx.p_obuf;
tx_fiber_init(msg->connection->session, msg->header.sync);
if (tx_check_schema(msg->header.schema_version))
@@ -1175,8 +1176,11 @@ tx_process1(struct cmsg *m)
struct tuple *tuple;
struct obuf_svp svp;
- if (box_process1(&msg->dml, &tuple) ||
- iproto_prepare_select(out, &svp))
+ struct obuf *out;
+ if (box_process1(&msg->dml, &tuple) != 0)
+ goto error;
+ out = msg->connection->tx.p_obuf;
+ if (iproto_prepare_select(out, &svp) != 0)
goto error;
if (tuple && tuple_to_obuf(tuple, out))
goto error;
@@ -1192,7 +1196,7 @@ static void
tx_process_select(struct cmsg *m)
{
struct iproto_msg *msg = tx_accept_msg(m);
- struct obuf *out = msg->connection->tx.p_obuf;
+ struct obuf *out;
struct obuf_svp svp;
struct port port;
int count;
@@ -1209,6 +1213,8 @@ tx_process_select(struct cmsg *m)
req->key, req->key_end, &port);
if (rc < 0)
goto error;
+
+ out = msg->connection->tx.p_obuf;
if (iproto_prepare_select(out, &svp) != 0) {
port_destroy(&port);
goto error;
diff --git a/src/errinj.h b/src/errinj.h
index 352a3c3c9..09da94aaf 100644
--- a/src/errinj.h
+++ b/src/errinj.h
@@ -107,6 +107,7 @@ struct errinj {
_(ERRINJ_RELAY_EXIT_DELAY, ERRINJ_DOUBLE, {.dparam = 0}) \
_(ERRINJ_VY_DELAY_PK_LOOKUP, ERRINJ_BOOL, {.bparam = false}) \
_(ERRINJ_VY_RUN_WRITE_STMT_TIMEOUT, ERRINJ_DOUBLE, {.dparam = 0}) \
+ _(ERRINJ_IPROTO_FLUSH_DELAY, ERRINJ_BOOL, {.bparam = 0}) \
ENUM0(errinj_id, ERRINJ_LIST);
extern struct errinj errinjs[];
diff --git a/test/box/errinj.result b/test/box/errinj.result
index db27dd4b3..81089baf8 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -38,6 +38,8 @@ errinj.info()
state: false
ERRINJ_PORT_DUMP:
state: false
+ ERRINJ_IPROTO_FLUSH_DELAY:
+ state: false
ERRINJ_WAL_IO:
state: false
ERRINJ_TUPLE_ALLOC:
@@ -1057,3 +1059,130 @@ box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1)
s:drop()
---
...
+--
+-- Check that IProto commands get obuf right before sending it to
+-- IProto thread. The test was failed with discarding an response
+-- before its sending to a client. A plan is the following:
+--
+-- obuf[0] obuf[1]
+-- 1) start DML, yield;
+-- 2) do DQL, store in obuf,
+-- block on socket write;
+-- 3) do DQL, store in obuf,
+-- block on socket write;
+-- 4) start DML, yield;
+-- 5) do DQL, store in obuf,
+-- block on socket write;
+-- 6) flush all DQL responses,
+-- DML still yields in both
+-- obufs;
+--
+-- 7) both DML finishes, and
+-- are dumped into both
+-- obuf[0] and obuf[1], but
+-- block on socket write
+-- again;
+--
+-- 8) Now TX thread owns obuf[1] and thinks, that obuf[0] is fully
+-- flushed, and on a next DQL it resets obuf[0], discarding all
+-- its data, including non-sent DML response.
+--
+--
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+---
+...
+s = box.schema.create_space('test')
+---
+...
+pk = s:create_index('pk')
+---
+...
+s:replace{1}
+---
+- [1]
+...
+c = net_box.connect(box.cfg.listen)
+---
+...
+errinj.set("ERRINJ_WAL_DELAY", true)
+---
+- ok
+...
+errinj.set("ERRINJ_IPROTO_FLUSH_DELAY", true)
+---
+- ok
+...
+long_dml1_f = fiber.create(function() c.space.test:replace{100} end)
+---
+...
+fiber.sleep(0.05)
+---
+...
+long_get1_f = fiber.create(function() c.space.test:get{1} end)
+---
+...
+fiber.sleep(0.05)
+---
+...
+long_get2_f = fiber.create(function() c.space.test:get{1} end)
+---
+...
+fiber.sleep(0.05)
+---
+...
+long_dml2_f = fiber.create(function() c.space.test:replace{200} end)
+---
+...
+fiber.sleep(0.05)
+---
+...
+long_get3_f = fiber.create(function() c.space.test:get{1} end)
+---
+...
+fiber.sleep(0.05)
+---
+...
+errinj.set("ERRINJ_IPROTO_FLUSH_DELAY", false)
+---
+- ok
+...
+while long_get1_f:status() ~= 'dead' or long_get2_f:status() ~= 'dead' or long_get3_f:status() ~= 'dead' do fiber.sleep(0.1) end
+---
+...
+errinj.set("ERRINJ_IPROTO_FLUSH_DELAY", true)
+---
+- ok
+...
+errinj.set("ERRINJ_WAL_DELAY", false)
+---
+- ok
+...
+fiber.sleep(0.05)
+---
+...
+long_get1_f = fiber.create(function() c.space.test:get{1} end)
+---
+...
+fiber.sleep(0.05)
+---
+...
+errinj.set("ERRINJ_IPROTO_FLUSH_DELAY", false)
+---
+- ok
+...
+fiber.sleep(0.05)
+---
+...
+long_dml1_f:status() == 'dead' and long_dml2_f:status() == 'dead'
+---
+- true
+...
+c:close()
+---
+...
+s:drop()
+---
+...
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+---
+...
diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
index 9bcf53847..666d1e790 100644
--- a/test/box/errinj.test.lua
+++ b/test/box/errinj.test.lua
@@ -348,3 +348,63 @@ box.error.injection.set('ERRINJ_BUILD_SECONDARY', sk.id)
sk:alter({parts = {2, 'number'}})
box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1)
s:drop()
+
+--
+-- Check that IProto commands get obuf right before sending it to
+-- IProto thread. The test was failed with discarding an response
+-- before its sending to a client. A plan is the following:
+--
+-- obuf[0] obuf[1]
+-- 1) start DML, yield;
+-- 2) do DQL, store in obuf,
+-- block on socket write;
+-- 3) do DQL, store in obuf,
+-- block on socket write;
+-- 4) start DML, yield;
+-- 5) do DQL, store in obuf,
+-- block on socket write;
+-- 6) flush all DQL responses,
+-- DML still yields in both
+-- obufs;
+--
+-- 7) both DML finishes, and
+-- are dumped into both
+-- obuf[0] and obuf[1], but
+-- block on socket write
+-- again;
+--
+-- 8) Now TX thread owns obuf[1] and thinks, that obuf[0] is fully
+-- flushed, and on a next DQL it resets obuf[0], discarding all
+-- its data, including non-sent DML response.
+--
+--
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+s = box.schema.create_space('test')
+pk = s:create_index('pk')
+s:replace{1}
+c = net_box.connect(box.cfg.listen)
+errinj.set("ERRINJ_WAL_DELAY", true)
+errinj.set("ERRINJ_IPROTO_FLUSH_DELAY", true)
+long_dml1_f = fiber.create(function() c.space.test:replace{100} end)
+fiber.sleep(0.05)
+long_get1_f = fiber.create(function() c.space.test:get{1} end)
+fiber.sleep(0.05)
+long_get2_f = fiber.create(function() c.space.test:get{1} end)
+fiber.sleep(0.05)
+long_dml2_f = fiber.create(function() c.space.test:replace{200} end)
+fiber.sleep(0.05)
+long_get3_f = fiber.create(function() c.space.test:get{1} end)
+fiber.sleep(0.05)
+errinj.set("ERRINJ_IPROTO_FLUSH_DELAY", false)
+while long_get1_f:status() ~= 'dead' or long_get2_f:status() ~= 'dead' or long_get3_f:status() ~= 'dead' do fiber.sleep(0.1) end
+errinj.set("ERRINJ_IPROTO_FLUSH_DELAY", true)
+errinj.set("ERRINJ_WAL_DELAY", false)
+fiber.sleep(0.05)
+long_get1_f = fiber.create(function() c.space.test:get{1} end)
+fiber.sleep(0.05)
+errinj.set("ERRINJ_IPROTO_FLUSH_DELAY", false)
+fiber.sleep(0.05)
+long_dml1_f:status() == 'dead' and long_dml2_f:status() == 'dead'
+c:close()
+s:drop()
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
--
2.14.3 (Apple Git-98)
More information about the Tarantool-patches
mailing list