Re: [tarantool-patches] [PATCH v3 09/14] wal: a dedicated wal scheduling fiber

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Sun, Jun 09, 2019 at 11:44:38PM +0300, Georgy Kirichenko wrote:
> In order to implement asynchronous transaction we need to run a
> transaction completion handler but tx_prio is not able to yield.
> 
> Prerequisites: #1254
> ---
>  src/box/wal.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/src/box/wal.c b/src/box/wal.c
> index 86d021896..e868a8e71 100644
> --- a/src/box/wal.c
> +++ b/src/box/wal.c
> @@ -100,6 +100,12 @@ struct wal_writer
>  	struct cpipe wal_pipe;
>  	/** A memory pool for messages. */
>  	struct mempool msg_pool;
> +	/** A queue to schedule journal entry completions. */
> +	struct stailq schedule_queue;
> +	/** True if writer is in rollback state. */
> +	bool is_in_rollback;
> +	/** A condition to signal about new schedule queue entries. */
> +	struct fiber_cond schedule_cond;
>  	/* ----------------- wal ------------------- */
>  	/** A setting from instance configuration - rows_per_wal */
>  	int64_t wal_max_rows;
> @@ -254,17 +260,36 @@ xlog_write_entry(struct xlog *l, struct journal_entry *entry)
>  	return xlog_tx_commit(l);
>  }
>  
> +/*
> + * Tx schedule fiber function.
> + */
> +static int
> +tx_schedule_f(va_list ap)
> +{
> +	struct wal_writer *writer = va_arg(ap, struct wal_writer *);
> +	while (!fiber_is_cancelled()) {
> +		while (!stailq_empty(&writer->schedule_queue)) {
> +			struct journal_entry *req =
> +				stailq_shift_entry(&writer->schedule_queue,
> +						   struct journal_entry, fifo);
> +			req->done = true;
> +			fiber_cond_broadcast(&req->done_cond);
> +		}
> +		writer->is_in_rollback = false;
> +		fiber_cond_wait(&writer->schedule_cond);
> +	}
> +	return 0;
> +}
> +
>  /**
> - * Signal done condition.
> + * Attach requests to a scheduling queue.
>   */
>  static void
>  tx_schedule_queue(struct stailq *queue)
>  {
> -	struct journal_entry *req;
> -	stailq_foreach_entry(req, queue, fifo) {
> -		req->done = true;
> -		fiber_cond_broadcast(&req->done_cond);
> -	}
> +	struct wal_writer *writer = &wal_writer_singleton;
> +	stailq_concat(&writer->schedule_queue, queue);
> +	fiber_cond_signal(&writer->schedule_cond);

This adds an extra ctxsw to the relatively hot WAL writer wakeup path so
we can't commit it. I assume it's a temporary hack you added so that you
can submit this patches for review.

I understand why you're doing this, but the right way would be removing
yields from on_commit triggers. We need to make vy_log_write yield-free.