From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 1 Aug 2018 13:42:28 +0300 From: Vladimir Davydov Subject: Re: [RFC PATCH 08/23] vinyl: check key uniqueness before modifying tx write set Message-ID: <20180801104228.fu46dlks57w2hfcu@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180731203410.GD15235@chai> References: <34aef16e5a3dfab28c9d7e9edbd7a66d91279388.1531065648.git.vdavydov.dev@gmail.com> <20180731203410.GD15235@chai> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Tue, Jul 31, 2018 at 11:34:10PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [18/07/08 22:52]: > > > Currently, we handle INSERT/REPLACE/UPDATE requests by iterating over > > all space indexes starting from the primary and inserting the > > corresponding statements to tx write set, checking key uniqueness if > > necessary. This means that by the time we write a REPLACE to the write > > set of a secondary index, it has already been written to the primary > > index write set. This is OK, and vy_tx_prepare() relies on that to > > implement the common memory level. However, this also means that when we > > check uniqueness of a secondary index, the new REPLACE can be found via > > the primary index. This is OK now, because all indexes are fully > > independent, but it isn't going to fly after #2129 is implemented. The > > problem is in order to check if a tuple is present in a secondary index, > > we will have to look up the corresponding full tuple in the primary > > index. To illustrate the problem, consider the following situation: > > I don't understand how this patch works. You need to put the key > into transaction write set first, and yield second. If there is a > change which happens during yield made on behalf of the unique > check, it must be able to see the keys your transaction is reading > and abort it. It will. We abort transactions on conflicts in the read set. Write set has nothing to do with transaction conflict resolution. > > Besides, I don't understand how the order of checks is making any > difference in your example - until a transaction is committed it > is not present in the common memory level anyway. It isn't, but it will see its own write set, which is inconsistent between the primary and secondary indexes. > > > > > Primary index covers field 1. > > Secondary index covers field 2. > > > > Committed statements: > > > > REPLACE{1, 10, lsn=1} - present in both indexes > > DELETE{1, lsn=2} - present only in the primary index > > > > Transaction: > > > > REPLACE{1, 10} > > > > When we check uniqueness of the secondary index, we find committed > > statement REPLACE{1, 10, lsn=1}, then look up the corresponding full > > tuple in the primary index and find REPLACE{1, 10}. Since the two tuples > > match, we mistakenly assume that there's a conflict. > > > > To avoid a situation like that, let's check uniqueness before modifying > > the write set of any index.