From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 2F68130665 for ; Sat, 1 Jun 2019 05:16:31 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7EgCcf2tGk-m for ; Sat, 1 Jun 2019 05:16:30 -0400 (EDT) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 56A1830655 for ; Sat, 1 Jun 2019 05:16:30 -0400 (EDT) Received: by smtp59.i.mail.ru with esmtpa (envelope-from ) id 1hX07o-0006gh-FP for tarantool-patches@freelists.org; Sat, 01 Jun 2019 12:16:28 +0300 Date: Sat, 1 Jun 2019 11:26:08 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH 02/10] vinyl: add a separate thread for vylog Message-ID: <20190601082608.GC29429@atlas> References: <16044855a7f1cb73e13baaa6ccd20dfdc0c9e48f.1558103547.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <16044855a7f1cb73e13baaa6ccd20dfdc0c9e48f.1558103547.git.vdavydov.dev@gmail.com> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org * Vladimir Davydov [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