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 6E5BB6EC59; Thu, 4 Feb 2021 17:01:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6E5BB6EC59 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1612447274; bh=RCR/THySmWgjYdTeLNx/ulxhMsH8ILG+4idjz/5oTuw=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=VtDOrcCbioZ0Drlscp1OATQ8AfPr0zfzjq1lMvVeLU5keRDqLnCKLKOAyDlojFUPh cCXG8oBqlupEKIxrc53MELx4wa8n0pLnJoNaS7zHNwzyZBcdSlApxL12Ea7vXS2t1O yyblxlVDWUT0LGZhp+SzN24WMUvEhxHFWeF6Ix98= Received: from smtp31.i.mail.ru (smtp31.i.mail.ru [94.100.177.91]) (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 F157C6EC59 for ; Thu, 4 Feb 2021 17:01:12 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F157C6EC59 Received: by smtp31.i.mail.ru with esmtpa (envelope-from ) id 1l7fC0-0000S5-8Y; Thu, 04 Feb 2021 17:01:08 +0300 To: Cyrill Gorcunov , tml References: <20210204111912.496547-1-gorcunov@gmail.com> Message-ID: Date: Thu, 4 Feb 2021 17:01:07 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.16; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <20210204111912.496547-1-gorcunov@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD953AC099BC0052A9C4647521586BE7E6324520D2A088600D8182A05F538085040D11A5030389FD41EED57219D8B0072B50D60F4996C05BDBB93C065675D87DF27 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE787D390A33DAA1DB5EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375104F503C4FA42DF8638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FCA6C37AB319BF122F47D589FBC500221B389642BE0808A4E2389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0A29E2F051442AF778941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B601F8F2FECC0250C8CC7F00164DA146DA6F5DAA56C3B73B23C77107234E2CFBA567F23339F89546C55F5C1EE8F4F765FCFF4145385FF5C95075ECD9A6C639B01BBD4B6F7A4D31EC0BC0CAF46E325F83A522CA9DD8327EE4930A3850AC1BE2E7353218473BE5707D41C4224003CC836476C0CAF46E325F83A50BF2EBBBDD9D6B0F05F538519369F3743B503F486389A921A5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5528D9C0A6B2D9A881B0EF6CD9A0089CA5D7DC34D7512F43AD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D349379E7F8541B6C9A4441128CD33207DF8A406C625836768C584E28265771D9717746415CE6E7D9771D7E09C32AA3244C1F0A08F6651708680752C37A6A1E068F9CA7333006C390A0927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj9Cv0SAP7uQXAravbWTG7Tw== X-Mailru-Sender: 3B9A0136629DC9125D61937A2360A446E483089192C5CECAA5198C5F9AA3510ACB438233D945F16D424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v3] txn: convert flags to explicit bitfield 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: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Cc: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 04.02.2021 14:19, Cyrill Gorcunov пишет: > Instead of shifting flags inside txn_x_flag() helpers > lets define precompiled value. This allows us to > operate with several flags at onces and shrink code > a bit. > > Closes #5128 > > Signed-off-by: Cyrill Gorcunov > --- Hi! Thanks for the patch! Please, find one comment below. > issue https://github.com/tarantool/tarantool/issues/5128 > branch gorcunov/gh-5128-txn-flags-3 > > src/box/txn.c | 3 +-- > src/box/txn.h | 26 +++++++++++++------------- > src/box/txn_limbo.c | 9 +++------ > test/unit/snap_quorum_delay.cc | 3 +-- > 4 files changed, 18 insertions(+), 23 deletions(-) > > diff --git a/src/box/txn.c b/src/box/txn.c > index a5edbfc60..150280905 100644 > --- a/src/box/txn.c > +++ b/src/box/txn.c > @@ -642,8 +642,7 @@ txn_journal_entry_new(struct txn *txn) > */ > if (!txn_has_flag(txn, TXN_FORCE_ASYNC)) { > if (is_sync) { > - txn_set_flag(txn, TXN_WAIT_SYNC); > - txn_set_flag(txn, TXN_WAIT_ACK); > + txn_set_flag(txn, TXN_WAIT_SYNC | TXN_WAIT_ACK); > } else if (!txn_limbo_is_empty(&txn_limbo)) { > /* > * There some sync entries on the > diff --git a/src/box/txn.h b/src/box/txn.h > index fca9bc1d0..b42249b17 100644 > --- a/src/box/txn.h > +++ b/src/box/txn.h > @@ -56,25 +56,25 @@ struct Vdbe; > > enum txn_flag { > /** Transaction has been processed. */ > - TXN_IS_DONE, > + TXN_IS_DONE = 0x1, > /** > * Transaction has been aborted by fiber yield so > * should be rolled back at commit. > */ > - TXN_IS_ABORTED_BY_YIELD, > + TXN_IS_ABORTED_BY_YIELD = 0x2, > /** > * fiber_yield() is allowed inside the transaction. > * See txn_can_yield() for more details. > */ > - TXN_CAN_YIELD, > + TXN_CAN_YIELD = 0x4, > /** on_commit and/or on_rollback list is not empty. */ > - TXN_HAS_TRIGGERS, > + TXN_HAS_TRIGGERS = 0x8, > /** > * Synchronous transaction touched sync spaces, or an > * asynchronous transaction blocked by a sync one until it > * is confirmed. > */ > - TXN_WAIT_SYNC, > + TXN_WAIT_SYNC = 0x10, > /** > * Synchronous transaction 'waiting for ACKs' state before > * commit. In this state it waits until it is replicated > @@ -82,14 +82,14 @@ enum txn_flag { > * commit and returns success to a user. > * TXN_WAIT_SYNC is always set, if TXN_WAIT_ACK is set. > */ > - TXN_WAIT_ACK, > + TXN_WAIT_ACK = 0x20, Since setting `TXN_WAIT_ACK` always implies setting `TXN_WAIT_SYNC`, and when TXN_WAIT_ACK is cleared, TXN_WAIT_SYNC is also cleared, you may define TXN_WAIT_ACK = 0x30 for simplicity. Then setting TXN_WAIT_ACK will set TXN_WAIT_SYNC implicitly. And same for clearing TXN_WAIT_ACK. Other than that LGTM. > /** > * A transaction may be forced to be asynchronous, not > * wait for any ACKs, and not depend on prepending sync > * transactions. This happens in a few special cases. For > * example, when applier receives snapshot from master. > */ > - TXN_FORCE_ASYNC, > + TXN_FORCE_ASYNC = 0x40, > }; > > enum { > @@ -394,21 +394,21 @@ struct txn { > }; > > static inline bool > -txn_has_flag(struct txn *txn, enum txn_flag flag) > +txn_has_flag(struct txn *txn, unsigned int flag) > { > - return (txn->flags & (1 << flag)) != 0; > + return (txn->flags & flag) != 0; > } > > static inline void > -txn_set_flag(struct txn *txn, enum txn_flag flag) > +txn_set_flag(struct txn *txn, unsigned int flag) > { > - txn->flags |= 1 << flag; > + txn->flags |= flag; > } > > static inline void > -txn_clear_flag(struct txn *txn, enum txn_flag flag) > +txn_clear_flag(struct txn *txn, unsigned int flag) > { > - txn->flags &= ~(1 << flag); > + txn->flags &= ~flag; > } > > /* Pointer to the current transaction (if any) */ > diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c > index 9c4c3cdf1..1bf292670 100644 > --- a/src/box/txn_limbo.c > +++ b/src/box/txn_limbo.c > @@ -266,8 +266,7 @@ txn_limbo_wait_complete(struct txn_limbo *limbo, struct txn_limbo_entry *entry) > in_queue, tmp) { > e->txn->signature = TXN_SIGNATURE_QUORUM_TIMEOUT; > txn_limbo_abort(limbo, e); > - txn_clear_flag(e->txn, TXN_WAIT_SYNC); > - txn_clear_flag(e->txn, TXN_WAIT_ACK); > + txn_clear_flag(e->txn, TXN_WAIT_SYNC | TXN_WAIT_ACK); > txn_complete_fail(e->txn); > if (e == entry) > break; > @@ -399,8 +398,7 @@ txn_limbo_read_confirm(struct txn_limbo *limbo, int64_t lsn) > } > e->is_commit = true; > txn_limbo_remove(limbo, e); > - txn_clear_flag(e->txn, TXN_WAIT_SYNC); > - txn_clear_flag(e->txn, TXN_WAIT_ACK); > + txn_clear_flag(e->txn, TXN_WAIT_SYNC | TXN_WAIT_ACK); > /* > * If already written to WAL by now, finish tx processing. > * Otherwise just clear the sync flags. Tx procesing will finish > @@ -456,8 +454,7 @@ txn_limbo_read_rollback(struct txn_limbo *limbo, int64_t lsn) > return; > rlist_foreach_entry_safe_reverse(e, &limbo->queue, in_queue, tmp) { > txn_limbo_abort(limbo, e); > - txn_clear_flag(e->txn, TXN_WAIT_SYNC); > - txn_clear_flag(e->txn, TXN_WAIT_ACK); > + txn_clear_flag(e->txn, TXN_WAIT_SYNC | TXN_WAIT_ACK); > if (e->txn->signature >= 0) { > /* Rollback the transaction. */ > e->txn->signature = TXN_SIGNATURE_SYNC_ROLLBACK; > diff --git a/test/unit/snap_quorum_delay.cc b/test/unit/snap_quorum_delay.cc > index b9d4cc6c4..e618b1cc4 100644 > --- a/test/unit/snap_quorum_delay.cc > +++ b/test/unit/snap_quorum_delay.cc > @@ -98,8 +98,7 @@ txn_process_func(va_list ap) > struct txn *txn = txn_begin(); > txn->fiber = fiber(); > /* Simulate a sync transaction. */ > - txn_set_flag(txn, TXN_WAIT_SYNC); > - txn_set_flag(txn, TXN_WAIT_ACK); > + txn_set_flag(txn, TXN_WAIT_SYNC | TXN_WAIT_ACK); > /* > * The true way to push the transaction to limbo is to call > * txn_commit() for sync transaction. But, if txn_commit() -- Serge Petrenko