From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 31 Jul 2018 23:34:10 +0300 From: Konstantin Osipov Subject: Re: [RFC PATCH 08/23] vinyl: check key uniqueness before modifying tx write set Message-ID: <20180731203410.GD15235@chai> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <34aef16e5a3dfab28c9d7e9edbd7a66d91279388.1531065648.git.vdavydov.dev@gmail.com> References: <34aef16e5a3dfab28c9d7e9edbd7a66d91279388.1531065648.git.vdavydov.dev@gmail.com> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * 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. 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. > > 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. > > Needed for #2129 > --- > src/box/vinyl.c | 128 +++++++++++++++++++++++++++----------------------------- > 1 file changed, 62 insertions(+), 66 deletions(-) > -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov