Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Nikita Pettik <korablev@tarantool.org>,
	tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2] vinyl: restart read iterator in case of rolled back WAL
Date: Mon, 1 Jun 2020 19:40:06 +0200	[thread overview]
Message-ID: <4e9d5184-1b3a-d4de-e040-a3edb6ca38cd@tarantool.org> (raw)
In-Reply-To: <50a25fbae907f1b0d5406fb2e78d40d3e42a8a8d.1591029888.git.korablev@tarantool.org>

Hi! Thanks for the patch!

On 01/06/2020 18:46, Nikita Pettik wrote:
> Data read in vinyl is known to yield in case of disc access. So it opens
> a window for modifications of in-memory level. Imagine following scenario:
> right before data selection tuple is inserted into space. It passes first
> stage of commit procedure, i.e. it is prepared to be committed but still
> is not yet reached WAL.  Meanwhile iterator is starting to read the same key.
> At this moment prepared statement is already inserted to in-memory tree
> ergo visible to read iterator. So, read iterator fetches this statement
> and proceeds to disk scan.  In turn, disk scan yields and in this moment
> WAL fails to write statement on disk. Next, two cases are possible:
> 1. WAL thread has enough time to complete rollback procedure.
> 2. WAL thread fails to finish rollback in this time gap.
Seems like the second test case is not really different from no rollback
at all. Because basically the WAL thread execution is still in the same
function, wal_write_to_disk(). Both ERRINJ_WAL_DELAY and ERRINJ_RELAY_FASTER_THAN_TX
give the same effect, if the transaction is rolled back. If it is committed,
they give also the same effect from TX<->WAL interaction point of view. The
difference only appears in TX<->WAL<->Relay threads.

So probably you could drop this case from the patch. It would simplify
things. But up to you and Alexander L.

Talking of the tests, I tried running multiple instances of the new
test:

    python test-run.py gh-3395-read-prepared-un gh-3395-read-prepared-un gh-3395-read-prepared-un gh-3395-read-prepared-un gh-3395-read-prepared-un gh-3395-read-prepared-un

And it fails sometimes. So it is flaky. Better fix it before push.

> diff --git a/src/box/vy_read_iterator.c b/src/box/vy_read_iterator.c
> index dcc9a0873..62a8722d9 100644
> --- a/src/box/vy_read_iterator.c
> +++ b/src/box/vy_read_iterator.c
> @@ -382,6 +382,9 @@ vy_read_iterator_scan_disk(struct vy_read_iterator *itr, uint32_t disk_src,
>   * Restore the position of the active in-memory tree iterator
>   * after a yield caused by a disk read and update 'next'
>   * if necessary.
> + * @retval -1 In case of error (e.g. OOM);
> + * @retval 0 Successful execution;
> + * @retval 1 Restart of advance_iterator is required.

What is advance_iterator?

>   */
>  static NODISCARD int
>  vy_read_iterator_restore_mem(struct vy_read_iterator *itr,

  reply	other threads:[~2020-06-01 17:40 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-01 16:46 Nikita Pettik
2020-06-01 17:40 ` Vladislav Shpilevoy [this message]
2020-06-19 12:34   ` Nikita Pettik
2020-06-20 16:33     ` Vladislav Shpilevoy
2020-06-16 12:10 ` Aleksandr Lyapunov
2020-06-19 12:24   ` Nikita Pettik
2020-06-19 12:42     ` Konstantin Osipov
2020-06-23  5:15       ` Aleksandr Lyapunov
2020-06-23 11:08         ` Konstantin Osipov
2020-06-19 13:01     ` Aleksandr Lyapunov
2020-06-24 13:41       ` Nikita Pettik
2020-06-20 16:33     ` Vladislav Shpilevoy

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=4e9d5184-1b3a-d4de-e040-a3edb6ca38cd@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2] vinyl: restart read iterator in case of rolled back WAL' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox