From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Thu, 13 Jun 2019 17:21:11 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] [PATCH v3 08/14] wal: enable asyncronous wal
 writes
Message-ID: <20190613142111.iith557g6i6mz5ba@esperanza>
References: <cover.1560112747.git.georgy@tarantool.org>
 <6eb76383a0e8cb04b52e7c4dd0374e18cf536e23.1560112747.git.georgy@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6eb76383a0e8cb04b52e7c4dd0374e18cf536e23.1560112747.git.georgy@tarantool.org>
To: Georgy Kirichenko <georgy@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Sun, Jun 09, 2019 at 11:44:37PM +0300, Georgy Kirichenko wrote:
> Allow to send a journal entry to wal without to wait until the writing
> was finished. Two methods were introduced:
>  * async_write method emits an entry to be written, returns 0 if the
>  entry was successfully scheduled;
>  * async_wait method waits until writing was finished and returns a
>  result of journal write.
> 
> Prerequisites: #1254
> ---
>  src/box/box.cc    | 30 ++++++++++++++++++++---
>  src/box/journal.c | 21 +++++++++++++++-
>  src/box/journal.h | 30 +++++++++++++++++++++++
>  src/box/wal.c     | 62 +++++++++++++++++++++++++++++++++++++++++------
>  4 files changed, 130 insertions(+), 13 deletions(-)
> 
> diff --git a/src/box/box.cc b/src/box/box.cc
> index a88e762c0..d0616095b 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -308,19 +308,41 @@ struct recovery_journal {
>   * min/max LSN of created LSM levels.
>   */
>  static int64_t
> -recovery_journal_write(struct journal *base,
> -		       struct journal_entry *entry)
> +recovery_journal_async_write(struct journal *base,
> +			     struct journal_entry *entry)

And in one of the following patches you rename it back to
recovery_journal_write...

This series needs to be resplit.

>  {
>  	struct recovery_journal *journal = (struct recovery_journal *) base;
> +	entry->res = vclock_sum(journal->vclock);
>  	entry->done = true;
>  	fiber_cond_broadcast(&entry->done_cond);
> -	return vclock_sum(journal->vclock);
> +	return 0;
> +}
> +
> +static int64_t
> +recovery_journal_async_wait(struct journal *base,
> +			    struct journal_entry *entry)
> +{
> +	(void) base;
> +	assert(entry->done);
> +	return entry->res;
> +}
> +
> +static int64_t
> +recovery_journal_write(struct journal *base,
> +		       struct journal_entry *entry)
> +{
> +	if (recovery_journal_async_write(base, entry) == 0)
> +		return recovery_journal_async_wait(base, entry);
> +	return -1;

The 'journal_write' callback looks exactly the same for all
implementations hence we could move this code up to the generic
level.

Anyway, as I said I have concerns re the wakeup design - see my comments
to the final patch.