From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@freelists.org Subject: Re: [PATCH 1/1] iproto: get iproto obuf only right before usage Date: Tue, 20 Mar 2018 16:07:35 +0300 [thread overview] Message-ID: <20180320130735.xujvp3pc52d5djsj@esperanza> (raw) In-Reply-To: <ce4fd164e4053830cba98052ff92ec1c28c2b35b.1521211045.git.v.shpilevoy@tarantool.org> On Fri, Mar 16, 2018 at 05:41:42PM +0300, Vladislav Shpilevoy wrote: > --- 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. Please open a Github ticket. Just describe the problem there. A test case is not necessary. This will be useful for release changelog. (FYI I asked both Kostja and Kirill - they both agree). > > --- Changes: do not rely on buffers number and rotation. Just > --- send multiple fast read/write requests with a small delay. > --- With no patch the test hangs or crashes since some responses > --- are corrupted or discarded. When I talked about extra info you don't want to be included into the commit message, I meant the '---' generated by git-format-patch, as everything following '---' is ignored by git-am. Also, please don't forget to add the version to the subject line (--subject-prefix='PATCH v2'). > > 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@tarantool.org> We don't sign patches, not yet. Please fix your git config. > @@ -1175,8 +1175,15 @@ tx_process1(struct cmsg *m) > > struct tuple *tuple; > struct obuf_svp svp; > - if (box_process1(&msg->dml, &tuple) || > - iproto_prepare_select(out, &svp)) > + ERROR_INJECT(ERRINJ_IPROTO_TX_DELAY, { > + if (rand() % 100 < 10) > + fiber_sleep(0.001); > + }); > + 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 +1199,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; > @@ -1204,11 +1211,18 @@ tx_process_select(struct cmsg *m) > if (tx_check_schema(msg->header.schema_version)) > goto error; > > + ERROR_INJECT(ERRINJ_IPROTO_TX_DELAY, { > + /* Select can yield, for example, on vinyl. */ > + if (rand() % 100 < 10) > + fiber_sleep(0.001); > + }); There are quite a few constants in this error injection so copying-and-pasting doesn't look good. I'd wrap it into an inline function (tx_inject_delay or something, think of the name pls). > diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua > index 9bcf53847..a24b357e6 100644 > --- a/test/box/errinj.test.lua > +++ b/test/box/errinj.test.lua > @@ -348,3 +348,19 @@ box.error.injection.set('ERRINJ_BUILD_SECONDARY', sk.id) > sk:alter({parts = {2, 'number'}}) > box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1) > s:drop() Please add a brief comment explaining what and why we are testing here. > + > +box.schema.user.grant('guest', 'read,write,execute', 'universe') > +s = box.schema.space.create('test') > +_ = s:create_index('pk') > + > +c = net_box.connect(box.cfg.listen) > + > +ch = fiber.channel(200) > +errinj.set("ERRINJ_IPROTO_TX_DELAY", true) > +for i = 1, 100 do fiber.create(function() for j = 1, 10 do c.space.test:replace{1} end ch:put(true) end) end > +for i = 1, 100 do fiber.create(function() for j = 1, 10 do c.space.test:select() end ch:put(true) end) end > +for i = 1, 200 do ch:get() end > +errinj.set("ERRINJ_IPROTO_TX_DELAY", false) > + > +s:drop() > +box.schema.user.revoke('guest', 'read,write,execute','universe')
next prev parent reply other threads:[~2018-03-20 13:07 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <20180316135103.usjz6veqe5mpste6@esperanza> 2018-03-16 14:41 ` Vladislav Shpilevoy 2018-03-20 13:07 ` Vladimir Davydov [this message] 2018-03-20 13:43 ` [PATCH v3 " Vladislav Shpilevoy 2018-03-20 16:49 ` Vladimir Davydov
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=20180320130735.xujvp3pc52d5djsj@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [PATCH 1/1] iproto: get iproto obuf only right before usage' \ /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