Re: [Tarantool-patches] [PATCH v14 4/6] qsync: order access to the limbo terms

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the patch!

See 3 comments below.

> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index b981bd436..845a7d015 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -915,8 +916,10 @@ apply_synchro_row(uint32_t replica_id, struct xrow_header *row)
>  		diag_set_journal_res(entry.base.res);
>  		goto err;
>  	}
> +	txn_limbo_term_unlock(&txn_limbo);
>  	return 0;
>  err:
> +	txn_limbo_term_unlock(&txn_limbo);
>  	diag_log();

1. This function can go to 'err' before the lock is taken.


2. As for your complaint about the begin/commit/rollback API
being not working because you can't unlock from a non-owner
fiber - well, it works in your patch somehow, doesn't it?

Why do you in your patch unlock here, but in the newly proposed
API you only tried to unlock in the trigger?

You could call commit/rollback from this function, like you
do with unlock now.

>  	return -1;
>  }
> diff --git a/src/box/txn_limbo.h b/src/box/txn_limbo.h
> index e0d17de4b..1ee815d1c 100644
> --- a/src/box/txn_limbo.h
> +++ b/src/box/txn_limbo.h
> @@ -217,14 +222,39 @@ txn_limbo_last_entry(struct txn_limbo *limbo)
>  				in_queue);
>  }
>  
> +/** Lock promote data. */
> +static inline void
> +txn_limbo_term_lock(struct txn_limbo *limbo)
> +{
> +	latch_lock(&limbo->promote_latch);
> +}
> +
> +/** Unlock promote data. */
> +static inline void
> +txn_limbo_term_unlock(struct txn_limbo *limbo)
> +{
> +	latch_unlock(&limbo->promote_latch);
> +}
> +
> +/** Fetch replica's term with lock taken. */
> +static inline uint64_t
> +txn_limbo_replica_term_locked(struct txn_limbo *limbo, uint32_t replica_id)

3. Limbo can be made const here.

> +{
> +	assert(latch_is_locked(&limbo->promote_latch));
> +	return vclock_get(&limbo->promote_term_map, replica_id);
> +}