Re: [Tarantool-patches] [PATCH 06/11] qsync: txn_limbo_append -- use owner_id in argument name

[Serge Petrenko <sergepetrenko@tarantool.org>]

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 <gorcunov@gmail.com>
> ---

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