From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2CC46469710 for ; Thu, 21 May 2020 05:51:55 +0300 (MSK) Date: Thu, 21 May 2020 02:51:54 +0000 From: Nikita Pettik Message-ID: <20200521025153.GC19031@tarantool.org> References: <670c3876e58a7cfa14d45db1dc074a10dd034759.1586808463.git.korablev@tarantool.org> <20200413221229.GA3462@atlas> <20200514021155.GB18509@tarantool.org> <20200514065608.GA12581@atlas> <20200519191006.GB13813@tarantool.org> <20200519193948.GA12955@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200519193948.GA12955@atlas> Subject: Re: [Tarantool-patches] [PATCH 2/2] vinyl: skip invalid upserts during squash List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov , tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org On 19 May 22:39, Konstantin Osipov wrote: > * Nikita Pettik [20/05/19 22:10]: > > On 14 May 09:56, Konstantin Osipov wrote: > > > * Nikita Pettik [20/05/14 09:50]: > > > > On 14 Apr 01:12, Konstantin Osipov wrote: > > > > > * Nikita Pettik [20/04/14 00:57]: > > > > > > > > Finally, let's double check there are no issues with the used > > > > > format - can it become obsolete by the time it's used, e.g. if > > > > > there is an online/non-blocking schema change that happened in tx > > > > > thread (compaction is running in the write thread)? > > > > > > > > Upserts are supported only by primary index. Meanwhile vinyl does > > > > not support altering primary index of non-empty space. Am I missing > > > > something? > > > > > > I mean the space format object itself. Squashing is happening in a > > > different thread, the > > > > > > > > Seems like you've cut sentence. Could you please suggest exact > > test scenario? We already have similar test in vinyl/errinj.test.lua: > > 1. Memtable dump is started. > 2. Space is altered. > 3. The format object is gone. > 4. Memtable dump uses the old format object for upsert > squashing. Look, the format is used for upsert squashing is stored in write iterator: in vy_read_view_merge() it is passed to vy_apply_upsert() from stream->format. When write iterator is created, format which is obtained (i.e. lsm->disk_format) is refed. So, as you can see, it can't be destroyed before write iterator is finished. Finally, as I already mentioned, lsm used for squashing is PK, and PK in turn can't be altered (lsm->disk_format will be destroyed only on pk:drop()). > Similar with compaction: > 1. Compaction starts and uses the existing format objects > 2. It uses the format object for upsert squashing > 3. Space is altered and format is changed > 4. We report a squash failure/suppress failure. > > > This is both related to memory liveness issues (e.g. we may use a > freed format object, which isn't the case because we refcount them > at start of dump/compaction), but also correctness issues: we > accept data which is no longer acceptable according to the new > format, or report/skip data which shouldn't be skipped. Data will always will be acceptable according to the format because PK format can't change. > -- > Konstantin Osipov, Moscow, Russia