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

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

On Sat, Jun 01, 2019 at 11:26:08AM +0300, Konstantin Osipov wrote:
> * 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?

In order to make a snapshot of a vylog (when we rotate it), we need to
lock out all writers. Otherwise we might get an inconsistent view.
If vylog was a part of xlog, we would use memtx read-view, but since it
isn't, we use a latch. For vylog it's okay as writers tolerate stalls by
design. Using the same thread would make the synchronization much
easier.

> 
> 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. 

I'm looking into this. The main problem is backward compatibility as
we'll have to support both xlog and vylog. Also, it looks like we'll
need to scan xlog twice on recovery - first to restore vinyl index
structure, then to apply rows missing in the memory level (as we don't
want to replay rows that have already been dumped to disk).