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 58F0E6EC40; Tue, 10 Aug 2021 15:27:26 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 58F0E6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1628598446; bh=lIZYLe7cUgAMwnlzvtsMqPRgoWZj2UkRA0nSRozhc6c=; 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=DcYqz8xstlMqZ65QFoX5Z1jOyQeMDSAktvFZzVFOsBgzGk+0jJWPprBIdRKTXWbO2 fCqVD8SL0zcuMHBDbEqUzLhsC4SJE1FGKN9bGgvTsGegKiaY3G1g0dcIZ311FQhk88 8FxzhgvBHEm0AubdP/h9NGQuw1l9sPPURfQMIuvQ= Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [94.100.177.102]) (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 D18D66EC40 for ; Tue, 10 Aug 2021 15:27:24 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D18D66EC40 Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1mDQqq-0007T4-3g; Tue, 10 Aug 2021 15:27:24 +0300 To: Cyrill Gorcunov References: <20210804190752.488147-1-gorcunov@gmail.com> <20210804190752.488147-3-gorcunov@gmail.com> Message-ID: <1a58f1dc-bd03-7b45-a99f-389b9b37f325@tarantool.org> Date: Tue, 10 Aug 2021 15:27:23 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD910164DC12A5633065676A9727AC27C74182A05F538085040CA24AA098B18064B7D66CAA56E902D16A6A51E225C0A9EAFE9544BC53203A238 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A41A3668A00E2636EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006377F497DA9C8A5F4D48638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8C72A9B8C1EA87F36ADE3C613CED9CCE3117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCEC1C9C6CFAD2A0F5A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352026055571C92BF10FE5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE437C869540D2AB0F03CEA74F0D118906D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE3FE4D9CDE3FF759CFC0837EA9F3D19764C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637BBEA499411984DA1EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B50541C2D0C0994EB233EA2D8AC9CF219F2F X-C1DE0DAB: 0D63561A33F958A5CD048E25A7DB5660EFA04FBCA6C06BECB895FF9F9687EAFFD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA753177526CD55AFC11410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D2AC226BFD455724381F0D8F3DDE4C7502E03646D70CC5AD8AF1CBA1920C472E54F72E00C38BE08E1D7E09C32AA3244C45DF2D15385FEA94E33990E085D9E8CE7C0C08F7987826B9FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojGhQhWEp1aB/NBuSoM9fNDQ== X-Mailru-Sender: 6C3E74F07C41AE94D32402E5012278FA28959B1F440D44C08D35260F5C00F6B5C94C6FF03C0388C207784C02288277CA03E0582D3806FB6A5317862B1921BA260ED6CFD6382C13A6112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v10 2/4] limbo: order access to the limbo terms 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: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Cc: tml Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On 09.08.2021 19:24, Cyrill Gorcunov wrote: > On Sun, Aug 08, 2021 at 05:34:54PM +0300, Vladislav Shpilevoy wrote: >>>> In the next patch you would make txn_limbo_process_begin() >>>> also take the request to validate it. Then the 'filtering' >>>> would become internal to the limbo. >> >> I didn't propose to drop the locking. I said it could be hidden >> inside of the limbo's API. In the only example above you show: >> >>> txn_limbo_term_lock >>> txn_limbo_replica_term_locked >>> txn_limbo_term_unlock >> >> Here the lock is done inside of the limbo's API too. It is >> not exposed on the limbo's API level. So the questions is the >> same - can it be hidden inside of the API? Are there any usages >> of the lock done explicitly out of the limo? > > Actually, everything start looking a way more unattractive I think. > Lets gather the current API from the patchset. > > applier_synchro_filter_tx > txn_limbo_is_replica_outdated > txn_limbo_term_lock > txn_limbo_replica_term_locked > txn_limbo_term_unlock > > box_demote | box_promote_qsync | box_promote > txn_limbo_replica_term > txn_limbo_term_lock > txn_limbo_replica_term_locked > txn_limbo_term_unlock > > > wal_stream_apply_synchro_row | box_issue_promote | box_issue_demote | memtx_engine_recover_synchro > txn_limbo_process > txn_limbo_term_lock > txn_limbo_filter_locked > txn_limbo_process_locked > txn_limbo_term_unlock > > apply_synchro_row > txn_limbo_term_lock > txn_limbo_filter_locked > ** in-callback apply_synchro_row_cb -> txn_limbo_process_locked > txn_limbo_term_unlock > > Thus we have: > > - big txn_limbo_process function which operates with locked promote term > - txn_limbo_replica_term inliner, which relies on txn_limbo_term_lock/unlock > being present in header file > - txn_limbo_is_replica_outdated inliner, which relies on lock/unlock being > exported as well > > and apply_synchro_row as a special one which uses txn_limbo_process_locked > internally when commit happens. Note that all the functions above use explicit > lock/unlock inside single function, and even apply_synchro_row calls lock at > start and unlock at exit. > > Now if I gonna hide locking completely from usage ouside of limbo code I > have to: > > 1) Move txn_limbo_term_lock/txn_limbo_term_unlock into .c file, in result > txn_limbo_is_replica_outdated and txn_limbo_replica_term won't be > inliner anymore. Which is not critical I think but better to point out. > 2) We inroduce txn_txn_limbo_process_begin/complete/rollback which are basically > the wrappers arount txn_limbo_process_locked (because txn_limbo_process > will remain as is). Thus we will have > > txn_txn_limbo_process_begin() > txn_limbo_term_lock() > txn_limbo_filter_locked(); > > txn_txn_limbo_process_complete() > txn_limbo_process_locked() > txn_limbo_term_unlock > > txn_txn_limbo_process_rollback > txn_limbo_term_unlock > > And these three helpers looks very ugly. First of all they hide locking > unlocking between functions, since there is no explicit lock/unlock > in apply_synchro_row anymore. Do you really prefer this kind of > design, or I miss something obvious? They look consistent with txn_begin/commit/rollback. They hide the locking, exactly. This is what I wanted to achieve, because I don't like that the applier interferes into the limbo so hard. Yes, I would prefer this API. Lets wait for Sergey's opinion too.