From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (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 94A50469719 for ; Fri, 13 Nov 2020 12:43:03 +0300 (MSK) References: <20201112195121.191366-1-gorcunov@gmail.com> <20201112195121.191366-7-gorcunov@gmail.com> From: Serge Petrenko Message-ID: Date: Fri, 13 Nov 2020 12:43:02 +0300 MIME-Version: 1.0 In-Reply-To: <20201112195121.191366-7-gorcunov@gmail.com> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH 06/11] qsync: txn_limbo_append -- use owner_id in argument name List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov , tml Cc: Vladislav Shpilevoy 12.11.2020 22:51, Cyrill Gorcunov пишет: > And change the callers. This id is needed to test for > foreign keys and to set limbo owner on demand. > > Signed-off-by: Cyrill Gorcunov > --- Hi! Thanks for the patch! Don't do that, please. Now it looks like you're getting a limbo by its owner id and insert data there. This may be the case someday, but now we have a single limbo, and we may try to insert data with any instance id there (and fail, if the instance id doesn't match limbo owner id). > src/box/txn.c | 8 ++++---- > src/box/txn_limbo.c | 15 ++++++++++----- > src/box/txn_limbo.h | 2 +- > 3 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/src/box/txn.c b/src/box/txn.c > index f8726026b..d608f6989 100644 > --- a/src/box/txn.c > +++ b/src/box/txn.c > @@ -832,8 +832,8 @@ txn_commit_async(struct txn *txn) > } > > /* See txn_commit(). */ > - uint32_t origin_id = req->rows[0]->replica_id; > - limbo_entry = txn_limbo_append(&txn_limbo, origin_id, txn); > + uint32_t owner_id = req->rows[0]->replica_id; > + limbo_entry = txn_limbo_append(&txn_limbo, owner_id, txn); > if (limbo_entry == NULL) > goto rollback; > > @@ -900,14 +900,14 @@ txn_commit(struct txn *txn) > * Remote rows, if any, come before local rows, so > * check for originating instance id here. > */ > - uint32_t origin_id = req->rows[0]->replica_id; > + uint32_t owner_id = req->rows[0]->replica_id; > > /* > * Append now. Before even WAL write is done. > * After WAL write nothing should fail, even OOM > * wouldn't be acceptable. > */ > - limbo_entry = txn_limbo_append(&txn_limbo, origin_id, txn); > + limbo_entry = txn_limbo_append(&txn_limbo, owner_id, txn); > if (limbo_entry == NULL) > goto rollback; > } > diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c > index 2c35cd785..674dcad28 100644 > --- a/src/box/txn_limbo.c > +++ b/src/box/txn_limbo.c > @@ -49,7 +49,7 @@ txn_limbo_create(struct txn_limbo *limbo) > } > > struct txn_limbo_entry * > -txn_limbo_append(struct txn_limbo *limbo, uint32_t id, struct txn *txn) > +txn_limbo_append(struct txn_limbo *limbo, uint32_t owner_id, struct txn *txn) > { If  you dislike plain "id", call this parameter replica_id instead. > assert(txn_has_flag(txn, TXN_WAIT_SYNC)); > /* > @@ -70,12 +70,17 @@ txn_limbo_append(struct txn_limbo *limbo, uint32_t id, struct txn *txn) > diag_set(ClientError, ER_SYNC_ROLLBACK); > return NULL; > } > - if (id == 0) > - id = instance_id; > - if (limbo->owner_id != id) { > + if (owner_id == REPLICA_ID_NIL) { > + /* > + * Transactionis initiated on the local node, > + * thus we're the owner of the transaction. > + */ > + owner_id = instance_id; > + } > + if (limbo->owner_id != owner_id) { > if (limbo->owner_id == REPLICA_ID_NIL || > rlist_empty(&limbo->queue)) { > - limbo->owner_id = id; > + limbo->owner_id = owner_id; > limbo->confirmed_lsn = 0; > } else { > diag_set(ClientError, ER_UNCOMMITTED_FOREIGN_SYNC_TXNS, > diff --git a/src/box/txn_limbo.h b/src/box/txn_limbo.h > index c7e70ba64..1b56903b7 100644 > --- a/src/box/txn_limbo.h > +++ b/src/box/txn_limbo.h > @@ -191,7 +191,7 @@ txn_limbo_last_entry(struct txn_limbo *limbo) > * The limbo entry is allocated on the transaction's region. > */ > struct txn_limbo_entry * > -txn_limbo_append(struct txn_limbo *limbo, uint32_t id, struct txn *txn); > +txn_limbo_append(struct txn_limbo *limbo, uint32_t owner_id, struct txn *txn); > > /** Remove the entry from the limbo, mark as rolled back. */ > void -- Serge Petrenko