Re: [tarantool-patches] [PATCH v3 07/14] wal: remove fiber from a journal_entry structure

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

On Sun, Jun 09, 2019 at 11:44:36PM +0300, Georgy Kirichenko wrote:
> Use a fiber_cond to signal a condition and wake up a waiting fiber.
> This relaxes friction between fiber and transaction life cycles.

I don't see how it relaxes anything ;-)

Please be more specific when writing comments.

> 
> Prerequisites: #1254
> ---
>  src/box/box.cc    |  4 +++-
>  src/box/journal.c |  7 ++++---
>  src/box/journal.h |  9 +++++++--
>  src/box/wal.c     | 29 +++++++++--------------------
>  4 files changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/src/box/wal.c b/src/box/wal.c
> index 0ea15a432..5951817d0 100644
> --- a/src/box/wal.c
> +++ b/src/box/wal.c
> @@ -1172,15 +1166,10 @@ wal_write(struct journal *journal, struct journal_entry *entry)
>  	batch->approx_len += entry->approx_len;
>  	writer->wal_pipe.n_input += entry->n_rows * XROW_IOVMAX;
>  	cpipe_flush_input(&writer->wal_pipe);
> -	/**
> -	 * It's not safe to spuriously wakeup this fiber
> -	 * since in that case it will ignore a possible
> -	 * error from WAL writer and not roll back the
> -	 * transaction.
> -	 */
> -	bool cancellable = fiber_set_cancellable(false);
> -	fiber_yield(); /* Request was inserted. */
> -	fiber_set_cancellable(cancellable);
> +
> +	while (!entry->done)
> +		fiber_cond_wait(&entry->done_cond);
> +

Using a fiber_cond to wake up a single fiber is an overkill. You could
as well do

	while (!entry->done)
		fiber_yield_timeout(TIMEOUT_INFINITY);

Anyway, I have my reservations re how you handle WAL writer wakeups.
Please see my comments to the final patch.