Re: [Tarantool-patches] [PATCH v2 2/7] wal: bind asynchronous write completion to an entry

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the patch!

See 5 comments below.

On 23.07.2020 14:29, Cyrill Gorcunov wrote:
> In commit 77ba0e3504464131fe81c672d508d0275be2173a we've redesigned
> wal journal operations such that asynchronous write completion
> is a signle instance per journal.

1. signle -> single

> It turned out that such simplification is too tight and doesn't
> allow us to pass entries into the journal with custom completions.
> 
> Thus lets allow back such ability. We will need it to be able
> to write "confirm" records into wal directly without touching
> trasactions code at all.

2. trasactions -> transactions

3. Please, add 'Part of #5129' to the commits related to the issue.

> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 83eef5d98..7d61f2ed2 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -2628,7 +2630,7 @@ box_cfg_xc(void)
>  	}
>  
>  	struct journal bootstrap_journal;
> -	journal_create(&bootstrap_journal, NULL, txn_complete_async,
> +	journal_create(&bootstrap_journal, bootstrap_journal_write,

4. Please, leave the sync write() NULL. I did it intentionally so as
it would early fail if bootstrap will call blocking write anywhere.

>  		       bootstrap_journal_write);
>  	journal_set(&bootstrap_journal);
>  	auto bootstrap_journal_guard = make_scoped_guard([] {
> diff --git a/src/box/journal.c b/src/box/journal.c
> index f1e89aaa2..fb81acb39 100644
> --- a/src/box/journal.c
> +++ b/src/box/journal.c
> @@ -36,6 +36,7 @@ struct journal *current_journal = NULL;
>  
>  struct journal_entry *
>  journal_entry_new(size_t n_rows, struct region *region,
> +		  void (*write_async_cb)(struct journal_entry *entry),

5.I think it would be better to move the callback definition to
a typedef. Otherwise it looks too bulky and too many places to change,
if will need to change the signature.

>  		  void *complete_data)
>  {
>  	struct journal_entry *entry;