Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja@tarantool.org>
To: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH 02/10] vinyl: add a separate thread for vylog
Date: Sat, 1 Jun 2019 11:26:08 +0300	[thread overview]
Message-ID: <20190601082608.GC29429@atlas> (raw)
In-Reply-To: <16044855a7f1cb73e13baaa6ccd20dfdc0c9e48f.1558103547.git.vdavydov.dev@gmail.com>

* Vladimir Davydov <vdavydov.dev@gmail.com> [19/05/17 17:54]:
> Historically, we use the WAL thread for writing vylog files, because,
> I guess, we didn't want to introduce a separate thread. However, that
> design decision turned out to be quite dubious:

>  - vy_log (vy_log.c) calls vy_log_writer (wal.c) while vy_log_writer
>    calls back to vy_log. That is we have a piece of logic split crudely
>    between two files, which makes the code difficult to follow and just
>    looks ugly.

This is because we can not ship arbitrary logic into a thread
without the thread becoming aware of it. This can be easily fixed
with run_in_cord() function, which would take a cord pointer and a 
fiber function pointer with context, and run the fiber in a given
cord. A mature implementation would take more than just a function
pointer and a context, but some sort of runnable object, which
responds to cancel, exit, suspend and resume. This would be very
useful in a bunch of other places, not just for vy_log.

>  - We can't make vy_log part of vy_env because of this relationship.
>    In fact, vy_log is the last singleton in the whole vy implementation:
>    everything else is wrapped neatly (well, not quite, but still) in
>    vy_env struct.

See above. The wal thread doesn't have to know anything about
struct vy_log or struct wal, for that matter, it's just a
container of runnable objects. We should also be able to move any
runnable object along with its context from a thread to thread
by suspending it first and then resuming at another thread, as I
described.

> 
>  - We can't kill the infamous vy_log.latch, which is a prerequisite for
>    transactional DDL. The latch is needed to sync between vy_log readers
>    and writers. You see, currently we can't read vy_log in the same
>    thread where we write it, which would eliminate the need for any kind
>    of synchronization, because vy_log read is quite a heavy operation -
>    it may stall WAL writes and thus badly affect latency. So we have to
>    do it from a coio thread.

This would be the main reason for the change, however, I would
rather ensure vy_log readers and writers use entirely different
objects as contexts. We already have vy_recovery as vy log reader
context, what prevents you from populating it using an own
xlog object?

As discussed verbally, there is a way to shift entire vy_log
contents to the existing WAL log which would make our future life
much easier. 

Please feel free to accept the above comments and complete this
patch or switch to an entirely new approach (which may well
require the same housekeeping changes I am pushing for in this
review).

-- 
Konstantin Osipov, Moscow, Russia

  parent reply	other threads:[~2019-06-01  9:16 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-17 14:52 [PATCH 00/10] vinyl: don't yield in DDL on_commit triggers Vladimir Davydov
2019-05-17 14:52 ` [PATCH 01/10] box: zap atfork callback Vladimir Davydov
2019-05-18 18:37   ` [tarantool-patches] " Konstantin Osipov
2019-05-20  8:13     ` Vladimir Davydov
2019-06-01  8:16     ` Konstantin Osipov
2019-06-06 10:04       ` Vladimir Davydov
2019-05-17 14:52 ` [PATCH 02/10] vinyl: add a separate thread for vylog Vladimir Davydov
2019-05-18 18:39   ` [tarantool-patches] " Konstantin Osipov
2019-05-20  8:17     ` Vladimir Davydov
2019-06-01  8:26   ` Konstantin Osipov [this message]
2019-06-06 10:20     ` Vladimir Davydov
2019-05-17 14:52 ` [PATCH 03/10] vinyl: move vylog recovery to vylog thread Vladimir Davydov
2019-06-01  8:36   ` [tarantool-patches] " Konstantin Osipov
2019-06-06 10:23     ` Vladimir Davydov
2019-06-07 13:39       ` Konstantin Osipov
2019-06-10 15:24         ` Vladimir Davydov
2019-06-07 13:40       ` Konstantin Osipov
2019-05-17 14:52 ` [PATCH 04/10] vinyl: rework vylog transaction backlog implementation Vladimir Davydov
2019-06-01  8:38   ` [tarantool-patches] " Konstantin Osipov
2019-06-06 11:58     ` Vladimir Davydov
2019-05-17 14:52 ` [PATCH 05/10] vinyl: don't purge deleted runs from vylog on compaction Vladimir Davydov
2019-05-18 18:47   ` [tarantool-patches] " Konstantin Osipov
2019-05-20  8:27     ` Vladimir Davydov
2019-06-01  8:39   ` Konstantin Osipov
2019-06-06 12:40     ` Vladimir Davydov
2019-05-17 14:52 ` [PATCH 06/10] vinyl: lock out compaction while checkpointing is in progress Vladimir Davydov
2019-05-17 14:52 ` [PATCH 07/10] vinyl: don't access last vylog signature outside vylog implementation Vladimir Davydov
2019-05-17 14:52 ` [PATCH 08/10] vinyl: zap ERRINJ_VY_LOG_FLUSH_DELAY Vladimir Davydov
2019-05-17 14:52 ` [PATCH 09/10] key_def: pass alloc callback to key_def_dump_parts Vladimir Davydov
2019-05-18 18:52   ` [tarantool-patches] " Konstantin Osipov
2019-05-20  8:34     ` Vladimir Davydov
2019-06-01  8:41     ` Konstantin Osipov
2019-06-10 15:28       ` Vladimir Davydov
2019-06-16 14:57         ` Konstantin Osipov
2019-05-17 14:52 ` [PATCH 10/10] vinyl: get rid of the latch protecting vylog buffer Vladimir Davydov
2019-06-01  8:44   ` [tarantool-patches] " Konstantin Osipov
2019-06-06 13:15     ` Vladimir Davydov
2019-05-18 18:35 ` [tarantool-patches] Re: [PATCH 00/10] vinyl: don't yield in DDL on_commit triggers Konstantin Osipov
2019-05-20  8:09   ` Vladimir Davydov
2019-06-01  8:09 ` Konstantin Osipov

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=20190601082608.GC29429@atlas \
    --to=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH 02/10] vinyl: add a separate thread for vylog' \
    /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