From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 20 May 2019 11:17:34 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH 02/10] vinyl: add a separate thread for vylog Message-ID: <20190520081734.h2ku6kw3kgbx35yx@esperanza> References: <16044855a7f1cb73e13baaa6ccd20dfdc0c9e48f.1558103547.git.vdavydov.dev@gmail.com> <20190518183958.GC9448@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190518183958.GC9448@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Sat, May 18, 2019 at 09:39:58PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [19/05/17 17:54]: > > We need an infrastructure to run an opaque piece of code in any > OS thread, not create a new thread any time we have an itch for > refactoring. > > If you wish to run vylog in an own thread, fine, WAL thread has > plenty of capacity to host 10 vylog threads, please use it. It doesn't have capacity to load vylog and I wrote that in the comment below. We have threads for vinyl writers and readers and that's okay, but one more thread for vylog thread is suddenly a no-go. Is there a *technical* reason why you're reluctant to add a new thread. > > > 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. > > > > - 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. > > > > - 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. > > > > That being said, it's time to move vy_log writer back to where it > > belongs - vy_log.c, separate thread. This patch does just that. It > > doesn't patch the implementation to fix the flaws enumerated above - > > this will be done by following patches.