From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 20 May 2019 11:27:56 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH 05/10] vinyl: don't purge deleted runs from vylog on compaction Message-ID: <20190520082756.penqyibruq4w73hu@esperanza> References: <67424f76249971cf6c4fc5a361f294a0b3c88283.1558103547.git.vdavydov.dev@gmail.com> <20190518184715.GD9448@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190518184715.GD9448@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Sat, May 18, 2019 at 09:47:15PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [19/05/17 17:54]: > > After compacting runs, we first mark them as dropped (VY_LOG_DROP_RUN), > > then try to delete their files unless they are needed for recovery from > > the checkpoint, and finally mark them as not needed in the vylog > > (VY_LOG_FORGET_RUN). There's a potential race sitting here: the problem > > is the garbage collector might kick in after files are dropped, but > > before they are marked as not needed. If this happens, there will be > > runs that have two VY_LOG_FORGET_RUN records, which will break recovery: > > > > Run XX is forgotten, but not registered > > > > The following patches make the race more likely to happen so let's > > eliminate it by making the garbage collector the only one who can mark > > runs as not needed (i.e. write VY_LOG_FORGET_RUN record). There will > > be no warnings, because the garbage collector silently ignores ENOENT > > errors, see vy_gc(). > > > > Another good thing about this patch is that now we never yield inside > > a vylog transaction, which makes it easier to remove the vylog latch > > blocking implementation of transactional DDL. > > Should be a separate commit with a test? By removing VY_LOG_FORGET_RUN from compaction, we get this for free. I don't see how this could be factored out into a separate patch.