Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Serge Petrenko <sergepetrenko@tarantool.org>, gorcunov@gmail.com
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] recovery: make it yield when positioning in a WAL
Date: Thu, 29 Apr 2021 22:03:54 +0200
Message-ID: <70224f08-737b-db3c-b809-1472ed603568@tarantool.org> (raw)
In-Reply-To: <3d0997f3-066f-5b2c-3ae3-22b90583e9bf@tarantool.org>

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.

  reply	other threads:[~2021-04-29 20:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-26 16:59 Serge Petrenko via Tarantool-patches
2021-04-26 21:20 ` Vladislav Shpilevoy via Tarantool-patches
2021-04-28 15:34   ` Serge Petrenko via Tarantool-patches
2021-04-28 20:50     ` Vladislav Shpilevoy via Tarantool-patches
2021-04-29  8:55       ` Serge Petrenko via Tarantool-patches
2021-04-29 20:03         ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-05-12 11:30           ` Serge Petrenko via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=70224f08-737b-db3c-b809-1472ed603568@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergepetrenko@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git