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
next prev 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