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 CD15E6EC40; Mon, 9 Aug 2021 19:24:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CD15E6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1628526268; bh=+1HUb3xs/i2i26l5aaZViRqTFp6+9/3IGvzXxqeXcTc=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ahMMI/q5l544GPf/gEP/m2MKOovEhxZO3AqXt+dOyNg1Aqp5TdZaYRmpUDlLs5eym KFx7WLNBd5iIj1d8tn0fMVf2+D/ULV0UbfVgzThY9fK6gzL1c3C1LUy6IIMIGp/ty8 muqZP2pkx8jshIbiiu1lsy8xfEByAtwtHNfEz0us= Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 708046EC40 for ; Mon, 9 Aug 2021 19:24:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 708046EC40 Received: by mail-lj1-f182.google.com with SMTP id l4so9795067ljq.4 for ; Mon, 09 Aug 2021 09:24:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=cnY0//gyt6151LmV1mvi+arvJB63jfJDGq638B8F1zE=; b=FtyBrmIlPjUB3AAVOF8SmNR7oFawqLJjptn8PufPrPD+wl0t3MQDXhPsTSy62GJUBF EXqfCQPF0JvtbUnwOs9N/3eBN9M2NU8Hb1vRRU4dzEeAp23aXjp681aDgFnSjlaXRCU9 CU1i6BVBJkbhIgtyXbLw0g85VT7VhrTUemg46Y6Pva7Dg0wb1AKpGlAmbU4JfXEbhnb9 /3LkpyIWxxCym1IEH1yz2OuJhaRfn4FYwVTdEwB60hpFpWzbF1Q3M050+Ef+sXP3xE2L eSswke+Nl7Abc1AOwMqLgnmaAnXoymi/1j28sZa2+qEx6RnqZarxDvEdtr44JMKUWOaM TaWg== X-Gm-Message-State: AOAM533g92ctZ9pDPD0D3F5RvxUjL9RlLlBad5kPyneRGd53VV5vXhU/ RFnZL5uzUer8UTajpL+Tu0Lc6AWegJQA6w== X-Google-Smtp-Source: ABdhPJxQs+zxtTH4t4GeStaVV8BkqytxL3hVzxQmTeV7KyG/mpj4Jn//46ZF/hgj5lX00nLUCWBToA== X-Received: by 2002:a05:651c:2129:: with SMTP id a41mr16232869ljq.8.1628526266499; Mon, 09 Aug 2021 09:24:26 -0700 (PDT) Received: from grain.localdomain ([5.18.253.97]) by smtp.gmail.com with ESMTPSA id x14sm1938839lfg.220.2021.08.09.09.24.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Aug 2021 09:24:25 -0700 (PDT) Received: by grain.localdomain (Postfix, from userid 1000) id C63E05A001E; Mon, 9 Aug 2021 19:24:24 +0300 (MSK) Date: Mon, 9 Aug 2021 19:24:24 +0300 To: Vladislav Shpilevoy Message-ID: References: <20210804190752.488147-1-gorcunov@gmail.com> <20210804190752.488147-3-gorcunov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.0.7 (2021-05-04) 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: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Cc: tml Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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? Cyrill