Re: [tarantool-patches] Re: [PATCH 02/10] vinyl: add a separate thread for vylog

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Sat, May 18, 2019 at 09:39:58PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [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.