[tarantool-patches] Re: [PATCH v4 8/9] applier: apply transaction in parallel

[Konstantin Osipov]

* Georgy Kirichenko <georgy at tarantool.org> [19/06/20 09:54]:
>  applier_apply_tx(struct stailq *rows)
>  {
> +	struct xrow_header *first_row =
> +		&stailq_first_entry(rows, struct applier_tx_row,
> +				    next)->row;
> +	struct replica *replica = replica_by_id(first_row->replica_id);
> +	struct latch *latch = (replica ? &replica->order_latch :
> +			       &replicaset.applier.order_latch);
> +	latch_lock(latch);

Now that we have a sequencer shouldn't it handle the job of this
latch as well?

> +	if (vclock_get(&replicaset.applier.net_vclock, first_row->replica_id) >=
> +	    first_row->lsn) {
> +		/* Check there is a heathbeat message and wake a writers up. */
> +		if (first_row->lsn == 0)
> +			fiber_cond_broadcast(&replicaset.applier.commit_cond);
> +		latch_unlock(latch);
> +		return 0;
> +	}
> +
> +		/*
> +		 * A vclock of the last transaction wich was read
> +		 * from an applier connection.
> +		 */
> +		struct vclock net_vclock;
> +		/* Signaled on replicated transaction commit. */

I don't understand this comment, what is a replicated transaction
commit? Could you please rephrase? Why do  you need this cond?
Please add more comments explaining how and why.
> +		struct fiber_cond commit_cond;
> +		/*
> +		 * Trigger to fire when replication stops in case
> +		 * of an error.
> +		 */
> +		struct rlist on_replication_fail;

I would simply call it on_error on_failure. It's already clear it's related to the 
applier work. Please explain how and why
this trigger is used in the comment. 
> +		/* Diag to populate an error acros all appliers. */
> +		struct diag diag;
>  	} applier;
>  	/** Map of all known replica_id's to correspponding replica's. */
>  	struct replica **replica_by_id;

-- 
Konstantin Osipov, Moscow, Russia