Re: [Tarantool-patches] [PATCH v2 14/19] applier: remove writer_cond

[Serge Petrenko <sergepetrenko@tarantool.org>]

30.06.2020 02:15, Vladislav Shpilevoy пишет:
> Writer condition variable was used by the writer fiber to be woken
> up when it is time to send a heartbeat or an ACK.
>
> However it is not really needed, because writer fiber pointer is
> always available in the same structure as writer_cond, and can be
> used to call fiber_wakeup() directly.
>
> Note, fiber_cond_signal() is basically the same fiber_wakeup().
>
> The patch is not just refactoring for nothing. It is a
> prerequisite for #5100. In this issue it will be needed to wakeup
> the applier's writer fiber directly on each WAL write from txn.c
> module. So the writer_cond won't be available. The only usable
> thing will be txn->fiber, which will be set to applier's writer.
>
> Part of #5100
> ---
>   src/box/applier.cc | 11 ++++-------
>   src/box/applier.h  |  2 --
>   2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index fbb452dc0..635a9849c 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -155,11 +155,9 @@ applier_writer_f(va_list ap)
>   		 * replication_timeout seconds any more.
>   		 */
>   		if (applier->version_id >= version_id(1, 7, 7))
> -			fiber_cond_wait_timeout(&applier->writer_cond,
> -						TIMEOUT_INFINITY);
> +			fiber_yield_timeout(TIMEOUT_INFINITY);
>   		else
> -			fiber_cond_wait_timeout(&applier->writer_cond,
> -						replication_timeout);
> +			fiber_yield_timeout(replication_timeout);
>   		/*
>   		 * A writer fiber is going to be awaken after a commit or
>   		 * a heartbeat message. So this is an appropriate place to
> @@ -928,7 +926,7 @@ applier_on_commit(struct trigger *trigger, void *event)
>   {
>   	(void) event;
>   	struct applier *applier = (struct applier *)trigger->data;
> -	fiber_cond_signal(&applier->writer_cond);
> +	fiber_wakeup(applier->writer);
>   	return 0;
>   }
>   
> @@ -1093,7 +1091,7 @@ applier_subscribe(struct applier *applier)
>   		 */
>   		if (stailq_first_entry(&rows, struct applier_tx_row,
>   				       next)->row.lsn == 0)
> -			fiber_cond_signal(&applier->writer_cond);
> +			fiber_wakeup(applier->writer);
>   		else if (applier_apply_tx(&rows) != 0)
>   			diag_raise();
>   
> @@ -1289,7 +1287,6 @@ applier_new(const char *uri)
>   	applier->last_row_time = ev_monotonic_now(loop());
>   	rlist_create(&applier->on_state);
>   	fiber_cond_create(&applier->resume_cond);
> -	fiber_cond_create(&applier->writer_cond);
>   	diag_create(&applier->diag);
>   
>   	return applier;
> diff --git a/src/box/applier.h b/src/box/applier.h
> index c9fdc2955..4f8bee84e 100644
> --- a/src/box/applier.h
> +++ b/src/box/applier.h
> @@ -78,8 +78,6 @@ struct applier {
>   	struct fiber *reader;
>   	/** Background fiber to reply with vclock */
>   	struct fiber *writer;
> -	/** Writer cond. */
> -	struct fiber_cond writer_cond;
>   	/** Finite-state machine */
>   	enum applier_state state;
>   	/** Local time of this replica when the last row has been received */
Thanks for the patch! LGTM.

-- 
Serge Petrenko