From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id C56E7741FD; Fri, 6 Aug 2021 11:51:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C56E7741FD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1628239873; bh=1jHYXdUG15+0vio8MMlLfSK6iJI6sWe0C+JOzwPZS2I=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=i27VXBcNy3fJO+cG8yD6fOCpooUVUkHIBPCLoguaZBPk/FcrduFMROU7l+SnHE95U qYs3bOj3Nw2Q/YK2LioerbQSf5PjYIvBcWsP0sO9/3Xal9/eEyXaZKS9ImGi2A7vIn 8jgtZzNVS4UxfMD5YRlaa33Ij3gqp+BBeVfRjEZo= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 60828741FA for ; Fri, 6 Aug 2021 11:51:12 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 60828741FA Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1mBvZP-0000Mv-Eg; Fri, 06 Aug 2021 11:51:11 +0300 Date: Fri, 6 Aug 2021 11:51:07 +0300 To: mechanik20051988 Cc: v.shpilevoy@tarantool.org, tarantool-patches@dev.tarantool.org, mechanik20051988 Message-ID: <20210806085107.e3z6s6she6ikgmha@esperanza> References: <970c0f5f41acb002a2f95f0774aba7667b91036c.1628184138.git.mechanik20.05.1988@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <970c0f5f41acb002a2f95f0774aba7667b91036c.1628184138.git.mechanik20.05.1988@gmail.com> X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9ECFD080E047A606F56291F8739A291D6182A05F538085040296956D11748AF24417978A0C6CCC46981E883A493AD4F24047D024C9446EEDD X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE71D44F6E7EB16B5A3EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006376F978168E59B07A5EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BBCA57AF85F7723F2F41EC5DECD36423D0E4FE25D1393A43ECC7F00164DA146DAFE8445B8C89999728AA50765F7900637F3E38EE449E3E2AE389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8C2B5EEE3591E0D35F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8B08F9A42B2210255C75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A596B79982866B4719004C94278AED1F1B9F1652EAA295EA05D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7501A9DF589746230F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3472E5ECC12A9739C1A33B36FD504214DD45C5AD32B566B29F87D4EB36F773E9E3ADCBC6408B5CED161D7E09C32AA3244CA51D9333E4BD8E6F5C472216A02E6BD67C0C08F7987826B983B48618A63566E0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojFhlvmGwdUwQqHfVPRTOe+w== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D538ADCBBF1DFA4D848B1DF31B70F823B274CEFED1673C562683ABF942079399BFB559BB5D741EB966A65DFF43FF7BE03240331F90058701C67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 3/7] txn: detach transaction from fiber. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladimir Davydov via Tarantool-patches Reply-To: Vladimir Davydov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Thu, Aug 05, 2021 at 09:17:41PM +0300, mechanik20051988 wrote: > From: mechanik20051988 > > Detach transaction from fiber: now it is possible to start > transaction in fiber, save the pointer to it in some place > independent from the fiber and then restore it other fiber. > For this purpose `fiber_on_stop` and `fiber_on_yield` triggers > was changed: previously they received pointer to transaction > from the fiber storage, now they receive it as their argument, > because fiber storage can be already empty when they called. This is not true anymore, because you clear both triggers on detach. Please revert this part. > Also implement function `txn_detach`, which clear `fiber_on_stop` > and `fiber_on_yeild` triggers. Detached transaction does not > rollback in case when fiber stopped, but can be aborted in case > it does not support yeild. > > Path of #5860 > --- > src/box/txn.c | 29 ++++++++++++++++++++++------- > src/box/txn.h | 11 +++++++++++ > 2 files changed, 33 insertions(+), 7 deletions(-) > > diff --git a/src/box/txn.c b/src/box/txn.c > index b80e722a4..de6a7f37d 100644 > --- a/src/box/txn.c > +++ b/src/box/txn.c > @@ -302,8 +302,9 @@ txn_begin(void) > rlist_create(&txn->savepoints); > txn->fiber = NULL; > fiber_set_txn(fiber(), txn); > - /* fiber_on_yield is initialized by engine on demand */ > - trigger_create(&txn->fiber_on_stop, txn_on_stop, NULL, NULL); > + trigger_create(&txn->fiber_on_yield, txn_on_yield, txn, NULL); > + trigger_create(&txn->fiber_on_stop, txn_on_stop, txn, NULL); > + /* fiber_on_yield is added by engine on demand */ > trigger_add(&fiber()->on_stop, &txn->fiber_on_stop); > /* > * By default all transactions may yield. > @@ -1016,7 +1017,6 @@ txn_can_yield(struct txn *txn, bool set) > trigger_clear(&txn->fiber_on_yield); > } else if (!set && could) { > txn_clear_flags(txn, TXN_CAN_YIELD); > - trigger_create(&txn->fiber_on_yield, txn_on_yield, NULL, NULL); > trigger_add(&fiber()->on_yield, &txn->fiber_on_yield); > } > return could; > @@ -1216,9 +1216,8 @@ txn_savepoint_release(struct txn_savepoint *svp) > static int > txn_on_stop(struct trigger *trigger, void *event) > { > - (void) trigger; > (void) event; > - struct txn *txn = in_txn(); > + struct txn *txn = trigger->data; > assert(txn->signature == TXN_SIGNATURE_UNKNOWN); > txn->signature = TXN_SIGNATURE_ROLLBACK; > txn_rollback(txn); > @@ -1246,12 +1245,28 @@ txn_on_stop(struct trigger *trigger, void *event) > static int > txn_on_yield(struct trigger *trigger, void *event) > { > - (void) trigger; > (void) event; > - struct txn *txn = in_txn(); > + struct txn *txn = trigger->data; > assert(txn != NULL); > assert(!txn_has_flag(txn, TXN_CAN_YIELD)); > txn_rollback_to_svp(txn, NULL); > txn_set_flags(txn, TXN_IS_ABORTED_BY_YIELD); > return 0; > } > + > +struct txn * > +txn_detach(void) > +{ > + struct txn *txn = in_txn(); > + if (txn == NULL) > + return NULL; > + if (!txn_has_flag(txn, TXN_CAN_YIELD)) { > + struct trigger trigger; > + trigger.data = txn; > + txn_on_yield(&trigger, NULL); > + } > + trigger_clear(&txn->fiber_on_yield); This trigger needs to be cleared only if TXN_CAN_YIELD is unset. Then you don't need to modify txn_begin and txn_can_yield. > + trigger_clear(&txn->fiber_on_stop); > + fiber_set_txn(fiber(), NULL); > + return txn; > +} > \ No newline at end of file Please fix. > diff --git a/src/box/txn.h b/src/box/txn.h > index 8741dc6a1..b8bba67b8 100644 > --- a/src/box/txn.h > +++ b/src/box/txn.h > @@ -457,6 +457,17 @@ fiber_set_txn(struct fiber *fiber, struct txn *txn) > fiber->storage.txn = txn; > } > > +/** > + * Detach transaction from fiber. > + * By default if the fiber is stopped the transaction > + * started in this fiber is rollback. This function > + * detaches transaction from fiber, leaving it alive > + * after fiber is stopped. > + * @pre txn == in_txn() > + */ > +struct txn * > +txn_detach(void); > + I think we should introduce txn_attach, for symmetry and so as not access fiber storage directly from the iproto code, but to be sure I need to see the patch that makes use of this function, which is the next to the last one in the series... This means that there's no point in this patch alone - it doesn't ease the review process, because to figure out how the function it introduces is going to be used, the reviewer needs to look at a following patch. Besides, the code it introduces is never used nor tested until the rest of the series is applied so it can't be committed right now. That being said, please fold this patch into the commit introducing transactions in the protocol.