From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (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 65D50469710 for ; Tue, 17 Nov 2020 22:51:54 +0300 (MSK) Date: Tue, 17 Nov 2020 22:51:51 +0300 From: "Alexander V. Tikhonov" Message-ID: <20201117195151.GA80591@hpalx> References: <2161b3318413560c0145f011a62c395b5e4fffad.1604425009.git.korablev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2161b3318413560c0145f011a62c395b5e4fffad.1604425009.git.korablev@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] vinyl: update gh-4957-too-many-upserts test List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org Hi Nikita, thanks for the patch, as I see no new degradations found in gitlab-ci testing commit criteria pipeline [1], patch LGTM. [1] - https://gitlab.com/tarantool/tarantool/-/pipelines/217447464 On Tue, Nov 03, 2020 at 08:37:33PM +0300, Nikita Pettik wrote: > After upsert rework in 5a61c4717 (#5107 issue) update operations are > applied consistently corresponding to upserts they belong to: if update > operation from single upsert fails - all update operations from that > upsert are skipped. But the rest of update operations related to other > upserts (after squashing two upserts) are applied. So let's update #4957 > test case: now upsert operation can be processed only if it contains > more than BOX_UPDATE_OP_CNT_MAX (4000) operations before (before > squashing with any other upsert). > > Follow-up #4957 > Follow-up #5107 > --- > Branch: https://github.com/tarantool/tarantool/tree/np/gh-4957-test-follow-up > > test/vinyl/gh-4957-too-many-upserts.result | 49 +++++++++++++++----- > test/vinyl/gh-4957-too-many-upserts.test.lua | 29 ++++++++---- > 2 files changed, 58 insertions(+), 20 deletions(-) > > diff --git a/test/vinyl/gh-4957-too-many-upserts.result b/test/vinyl/gh-4957-too-many-upserts.result > index c942e62c8..d70a2e668 100644 > --- a/test/vinyl/gh-4957-too-many-upserts.result > +++ b/test/vinyl/gh-4957-too-many-upserts.result > @@ -32,26 +32,24 @@ ups_cnt = 5000 > box.begin() > | --- > | ... > -for i = 1, ups_cnt do s:upsert({1}, {{'&', 2, 1}}) end > +for i = 1, ups_cnt do s:upsert({1}, {{'|', 2, i}}) end > | --- > | ... > box.commit() > | --- > | ... > --- Upserts are not able to squash, so scheduler will get stuck. > --- So let's not waste much time here, just check that no crash > --- takes place. > +-- Since 2.6 update operations of single upsert are applied consistently. > +-- So despite upserts' update operations can be squashed, they are anyway > +-- applied as they come in corresponding upserts. The same concerns the > +-- second test. > -- > box.snapshot() > | --- > | - ok > | ... > - > -fiber = require('fiber') > - | --- > - | ... > -fiber.sleep(0.01) > +s:select() > | --- > + | - - [1, 8191] > | ... > > s:drop() > @@ -82,7 +80,7 @@ box.snapshot() > box.begin() > | --- > | ... > -for k = 1, ups_cnt do s:upsert({1}, {{'+', k, 1}}) end > +for k = 2, ups_cnt do s:upsert({1}, {{'+', k, 1}}) end > | --- > | ... > box.commit() > @@ -92,10 +90,39 @@ box.snapshot() > | --- > | - ok > | ... > -fiber.sleep(0.01) > +s:select()[1][ups_cnt] > + | --- > + | - 5001 > + | ... > + > +s:drop() > | --- > | ... > > +-- Test that single upsert with too many (> 4000) operations doesn't > +-- pass check so it is rejected. > +-- > +s = box.schema.create_space('test', {engine = 'vinyl'}) > + | --- > + | ... > +pk = s:create_index('pk') > + | --- > + | ... > + > +ups_ops = {} > + | --- > + | ... > +for k = 2, ups_cnt do ups_ops[k] = {'+', k, 1} end > + | --- > + | ... > +s:upsert({1}, ups_ops) > + | --- > + | - error: Illegal parameters, too many operations for update > + | ... > +s:select() > + | --- > + | - [] > + | ... > s:drop() > | --- > | ... > diff --git a/test/vinyl/gh-4957-too-many-upserts.test.lua b/test/vinyl/gh-4957-too-many-upserts.test.lua > index 572540a4c..48e923bf1 100644 > --- a/test/vinyl/gh-4957-too-many-upserts.test.lua > +++ b/test/vinyl/gh-4957-too-many-upserts.test.lua > @@ -12,16 +12,15 @@ box.snapshot() > -- > ups_cnt = 5000 > box.begin() > -for i = 1, ups_cnt do s:upsert({1}, {{'&', 2, 1}}) end > +for i = 1, ups_cnt do s:upsert({1}, {{'|', 2, i}}) end > box.commit() > --- Upserts are not able to squash, so scheduler will get stuck. > --- So let's not waste much time here, just check that no crash > --- takes place. > +-- Since 2.6 update operations of single upsert are applied consistently. > +-- So despite upserts' update operations can be squashed, they are anyway > +-- applied as they come in corresponding upserts. The same concerns the > +-- second test. > -- > box.snapshot() > - > -fiber = require('fiber') > -fiber.sleep(0.01) > +s:select() > > s:drop() > > @@ -34,11 +33,23 @@ _ = s:insert(tuple) > box.snapshot() > > box.begin() > -for k = 1, ups_cnt do s:upsert({1}, {{'+', k, 1}}) end > +for k = 2, ups_cnt do s:upsert({1}, {{'+', k, 1}}) end > box.commit() > box.snapshot() > -fiber.sleep(0.01) > +s:select()[1][ups_cnt] > + > +s:drop() > + > +-- Test that single upsert with too many (> 4000) operations doesn't > +-- pass check so it is rejected. > +-- > +s = box.schema.create_space('test', {engine = 'vinyl'}) > +pk = s:create_index('pk') > > +ups_ops = {} > +for k = 2, ups_cnt do ups_ops[k] = {'+', k, 1} end > +s:upsert({1}, ups_ops) > +s:select() > s:drop() > > -- restart the current server to resolve the issue #5141 > -- > 2.17.1 >