From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp18.mail.ru (smtp18.mail.ru [94.100.176.155]) (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 CEBDD469710 for ; Thu, 21 May 2020 01:00:08 +0300 (MSK) References: <48001497bf915ff9358b5d50a713a7c458255bae.1589892568.git.korablev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <84ad10a8-77b6-48a8-b6e5-ddcc55e172b6@tarantool.org> Date: Thu, 21 May 2020 00:00:06 +0200 MIME-Version: 1.0 In-Reply-To: <48001497bf915ff9358b5d50a713a7c458255bae.1589892568.git.korablev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] vinyl: restart read iterator in case of roll backed WAL List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik , tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! I would suggest adding Alexander L. to the review. He participated in implementation of iterator restoration a lot. See 2 comments below. 1. I see the test fails on the branch in one of the jobs: https://travis-ci.org/github/tarantool/tarantool/jobs/688791208 I also was able to reproduce it locally. The test hangs, or prints mismatching results like this: [001] vinyl/gh-3395-read-prepared-uncommitted.test.lua [ fail ] [001] [001] Test failed! Result content mismatch: [001] --- vinyl/gh-3395-read-prepared-uncommitted.result Wed May 20 23:42:38 2020 [001] +++ vinyl/gh-3395-read-prepared-uncommitted.reject Wed May 20 23:51:18 2020 [001] @@ -82,7 +82,8 @@ [001] [001] c:get() [001] | --- [001] - | - - [2, 2] [001] + | - - [1, 2] [001] + | - [2, 2] [001] | - [3, 2] [001] | ... [001] [001] The mismatch happens almost on every run. > diff --git a/test/vinyl/gh-3395-read-prepared-uncommitted.result b/test/vinyl/gh-3395-read-prepared-uncommitted.result > new file mode 100644 > index 000000000..a4ec023fd > --- /dev/null > +++ b/test/vinyl/gh-3395-read-prepared-uncommitted.result > @@ -0,0 +1,95 @@ > +-- test-run result file version 2 > +-- Disk scan in vinyl is known to yield during read. So it opens > +-- a window for modifications of in-memory level. Let's test > +-- following case: right before data selection tuple is inserted > +-- into space. It passes first stage of commit procedure, i.e. it > +-- is prepared to be committed but still not yet reached WAL. > +-- Meanwhile we are starting read of the same key. At this moment > +-- prepared statement is already inserted to in-memory tree. So, > +-- read iterator fetches this statement and proceeds to disk scan. > +-- In turn, disk scan yields and in this moment WAL fails to write > +-- statement on disk, so it is roll-backed. Read iterator must > +-- recognize this situation and must not include roll-backed tuple > +-- into result set. > +-- > +fiber = require('fiber') > + | --- > + | ... > + > +errinj = box.error.injection > + | --- > + | ... > +s = box.schema.create_space('test', {engine = 'vinyl'}) > + | --- > + | ... > +pk = s:create_index('pk') > + | --- > + | ... > +sk = s:create_index('sk', {parts = {{2, 'unsigned'}}, unique = false}) > + | --- > + | ... > +s:replace{3, 2} > + | --- > + | - [3, 2] > + | ... > +s:replace{2, 2} > + | --- > + | - [2, 2] > + | ... > +box.snapshot() > + | --- > + | - ok > + | ... > +-- Tuple {1, 2} is not going to be committed, so it should not > +-- appear in the result set. > +-- > +c = fiber.channel(1) > + | --- > + | ... > +function do_write() s:replace{1, 2} end > + | --- > + | ... > +errinj.set("ERRINJ_WAL_DELAY", true) > + | --- > + | - ok > + | ... > +writer = fiber.create(do_write) > + | --- > + | ... > + > +function do_read() local ret = sk:select{2} c:put(ret) end 2. What if it will read not the first tuple? What if it will read a second or a third? I didn't check, just want to ensure, that the iterator restart in this case won't literally restart the iteration, and won't return the same tuples twice.