Re: [Tarantool-patches] [PATCH] recovery: make it yield when positioning in a WAL

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

29.04.2021 23:03, Vladislav Shpilevoy пишет:
> Hi! Thanks for the patch!
>
> Technically looks good now, can be pushed.
>
> But in case you would find it interesting: now relay->row_count looks odd.
> Because its comment is not very correct, most of that is moved to the
> recovery layer. And because it is used only by initial join. And because we
> still have a single place where we use xstream/recovery, but the yields
> are not managed by them.
>
> Did you think about moving your code to xstream instead of recovery? Then
> it would be truly not related to recovery. It would have 2 methods:
>
> - write(struct xstream *, struct xrow_header *)
> - skip(struct xstream *, struct xrow_header *)
>
> Recovery calls skip() while establishes a position, for each skipped row.
> Relaying from memory never calls skip, but calls only write() like now.
>
> In order to yield you inherit xtream, add a member row_count, and both in
> skip() and write() increment it like you do. And then yield periodically.
>
> It would work for the initial join too.
>
> Or add row_count right to the xtream struct, and replace skip() with your
> function schedule_yield(). This way row_count would be visible to recovery
> and available for reporting in the logs like '... rows recovered'. And we
> wouldn't all too many virtual functions for the skipped rows.
>
> All that up to you.

Thanks for your input!

I took the approach with moving schedule_yield to struct xstream.
Please, check it out in v2.

-- 
Serge Petrenko