From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 4774D6F3E5; Thu, 11 Nov 2021 19:17:19 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4774D6F3E5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1636647439; bh=JY1FBeV9aD2tZ8hVDCg0CWLy+Q7KuOSmMBVf5J2hth8=; h=In-Reply-To:Date:Cc:References:To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=tWGozTN3jPXXLtEOZP+veCDarS6uSq4pt9J7zgCfyILKGOxLnOnlXa2qUHnzeYe1R Lo/HL+corT2J4p51x7j1Gpw1UOVQioai4KPCio6cM1CkEW6JJNtXNld1zoO+x6e+nl eXvv9uFCeabl9gEyejW7LPrg+b6fARVjHtmW+COI= Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (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 6ECFD6F3E5 for ; Thu, 11 Nov 2021 19:17:17 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6ECFD6F3E5 Received: by smtp50.i.mail.ru with esmtpa (envelope-from ) id 1mlClI-0002Yz-ML; Thu, 11 Nov 2021 19:17:17 +0300 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.120.0.1.13\)) In-Reply-To: <6a035fcc-a416-c3b3-5cd6-55c617e1c6fd@tarantool.org> Date: Thu, 11 Nov 2021 19:17:15 +0300 Cc: =?utf-8?B?0K/QvSDQqNGC0YPQvdC00LXRgA==?= , tml Content-Transfer-Encoding: quoted-printable Message-Id: <9BBD98B5-E8BC-4552-9826-D5F4A2C16C99@tarantool.org> References: <20211025104449.26693-1-ya.shtunder@gmail.com> <986cb3d1-64a9-87ec-8734-afe69c81c245@tarantool.org> <6a035fcc-a416-c3b3-5cd6-55c617e1c6fd@tarantool.org> To: Serge Petrenko X-Mailer: Apple Mail (2.3654.120.0.1.13) X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9731B3922EC063979787B19701143D42A39EF849E3128968B00894C459B0CD1B9DB54F1519CEDF66AF373840F13CA9AD6B0F620D9D1E8D896B4427F3AD7048003 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE758E3775C3D7FDEE4EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637D8FE4B420C2EEC248638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8C2B59EC7A0E7D6DA108BF70A172FB5EA117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC2EE5AD8F952D28FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735201E561CDFBCA1751F618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B642416645EBD45DC2089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A54EB94092C04195B777D153D6AC67A4E2AD149B3BF853AF62D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75438CC92D4039F4E2410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D346B71C4B0698719D7EBE1D23DCBB282DB4B8A2BF514F1892B9A6B19FC1AE74215E202E7EAF0E4F0C01D7E09C32AA3244CC3D1362FA7A97E811AA75375E089831F24AF4FAF06DA24FDFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj4t8MBgWr8bKKB+12P8A2+w== X-Mailru-Sender: 3B9A0136629DC912F4AABCEFC589C81E4912B9FB7DFE1A5A5B8D9073B304AB0FDDC877F1C187AAA2AD07DD1419AC565FA614486B47F28B67C5E079CCF3B0523AED31B7EB2E253A9E112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v3] replication: the truncate method called from within a transaction X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: sergos via Tarantool-patches Reply-To: sergos Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thanks for the patch! LGTM. Sergos > On 3 Nov 2021, at 17:37, Serge Petrenko via Tarantool-patches = wrote: >=20 >=20 >=20 > 03.11.2021 13:03, =D0=AF=D0=BD =D0=A8=D1=82=D1=83=D0=BD=D0=B4=D0=B5=D1=80= =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> Thanks for the comments! >> I corrected your remarks. >=20 > Thanks for the changes! > LGTM now. >=20 >>=20 >> Diff: >>=20 >> diff --git = a/test/replication-luatest/gh_6123_truncate_temp_space_test.lua = b/test/replication-luatest/gh_6123_truncate_temp_space_test.lua >> new file mode 100644 >> index 000000000..7940e722f >> --- /dev/null >> +++ b/test/replication-luatest/gh_6123_truncate_temp_space_test.lua >> @@ -0,0 +1,62 @@ >> +local t =3D require('luatest') >> +local cluster =3D require('test.luatest_helpers.cluster') >> +local helpers =3D require('test.luatest_helpers') >> + >> +local g =3D t.group('gh_6123', {{engine =3D 'memtx'}, {engine =3D = 'vinyl'}}) >> + >> +g.before_each(function(cg) >> + local engine =3D cg.params.engine >> + >> + cg.cluster =3D cluster:new({}) >> + >> + local box_cfg =3D { >> + replication =3D { >> + helpers.instance_uri('master') >> + }, >> + replication_timeout =3D 1, >> + read_only =3D false >> + } >> + >> + cg.master =3D cg.cluster:build_server({alias =3D 'master', = engine =3D engine, box_cfg =3D box_cfg}) >> + >> + local box_cfg =3D { >> + replication =3D { >> + helpers.instance_uri('master'), >> + helpers.instance_uri('replica') >> + }, >> + replication_timeout =3D 1, >> + replication_connect_timeout =3D 4, >> + read_only =3D true >> + } >> + >> + cg.replica =3D cg.cluster:build_server({alias =3D 'replica', = engine =3D engine, box_cfg =3D box_cfg}) >> + >> + cg.cluster:add_server(cg.master) >> + cg.cluster:add_server(cg.replica) >> + cg.cluster:start() >> +end) >> + >> + >> +g.after_each(function(cg) >> + cg.cluster.servers =3D nil >> + cg.cluster:drop() >> +end) >> + >> + >> +g.test_truncate_is_local_transaction =3D function(cg) >> + cg.master:eval("s =3D box.schema.space.create('temp', {temporary = =3D true})") >> + cg.master:eval("s:create_index('pk')") >> + >> + cg.master:eval("s:insert{1, 2}") >> + cg.master:eval("s:insert{4}") >> + t.assert_equals(cg.master:eval("return s:select()"), {{1, 2}, = {4}}) >> + >> + cg.master:eval("box.begin() box.space._schema:replace{'smth'} = s:truncate() box.commit()") >> + t.assert_equals(cg.master:eval("return s:select()"), {}) >> + t.assert_equals(cg.master:eval("return = box.space._schema:select{'smth'}"), {{'smth'}}) >> + >> + -- Checking that replica has received the last transaction, >> + -- and that replication isn't broken. >> + t.assert_equals(cg.replica:eval("return = box.space._schema:select{'smth'}"), {{'smth'}}) >> + t.assert_equals(cg.replica:eval("return = box.info.replication[1].upstream.status"), 'follow') >> +end >>=20 >> -- >> Yan Shtunder >>=20 >> =D0=BF=D1=82, 29 =D0=BE=D0=BA=D1=82. 2021 =D0=B3. =D0=B2 12:20, Serge = Petrenko : >>=20 >>=20 >>=20 >> 28.10.2021 18:48, =D0=AF=D0=BD =D0=A8=D1=82=D1=83=D0=BD=D0=B4=D0=B5=D1= =80 =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> > Hi! Thank you for the review! >> > I have fixed the errors >> > >> > Please, find a better test name. >> > Like "gh_6123_truncate_temp_space_test.lua" >> > >> > >> > gh_6123_test.lua -> gh_6123_truncate_temp_space_test.lua >> > >> > -- >> > Yan Shtunder >> > >> > >>=20 >> Thanks for the fixes! >>=20 >> One comment left. I'm pasting part of the patch here to review. >>=20 >> In test: >>=20 >> +g.after_each(function(cg) >> + cg.cluster.servers =3D nil >> + cg.cluster:drop() >> + fio.rmtree(cg.master.workdir) >> + fio.rmtree(cg.replica.workdir) >> +end) >> + >> + >> +g.before_test('test_truncate_is_local_transaction', function(cg) >> + cg.cluster:add_server(cg.master) >> + cg.cluster:add_server(cg.replica) >> + cg.cluster:start() >> +end) >> + >> +g.after_test('test_truncate_is_local_transaction', function(cg) >> + cg.cluster:drop({cg.master, cg.replica}) >> +end) >> + >>=20 >> You have a single test: test_truncate_is_local_transaction. >>=20 >> So g.after_each and g.after_test both have the same meaning in >> your case. >>=20 >> I propose to dorp g.after_test, since g.after_each supersedes it. >>=20 >> Also, you don't need to call fio.rmtree in g.after_each. >> luatest does this for you, when needed. >> > >> > =D0=BF=D0=BD, 25 =D0=BE=D0=BA=D1=82. 2021 =D0=B3. =D0=B2 16:34, = Serge Petrenko >> : >> > >> > >> > >> > 25.10.2021 13:44, Yan Shtunder via Tarantool-patches = =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> > > The truncate method could be called from within a = transaction. >> > The flag >> > > of GROUP_LOCAL was set in truncate method after statement = row >> > had been >> > > being checked on the GROUP_LOCAL. Accordingly, after a = local >> > transaction >> > > NOP row was not appended. >> > > >> > > Closes #6123 >> > >> > Thanks for the patch! >> > >> > Please, find 2 comments below. >> > > --- >> > > Issue: https://github.com/tarantool/tarantool/issues/6123 >> > > Patch: >> > >> = https://github.com/tarantool/tarantool/tree/yshtunder/gh-6123-truncate-is-= local-transaction >> > > >> > > src/box/alter.cc | 5 ++ >> > > test/replication-luatest/gh_6123_test.lua | 62 >> > +++++++++++++++++++++++ >> > > 2 files changed, 67 insertions(+) >> > > create mode 100644 = test/replication-luatest/gh_6123_test.lua >> > > >> > > diff --git a/src/box/alter.cc b/src/box/alter.cc >> > > index 31ac82fc5..52b9ea4d2 100644 >> > > --- a/src/box/alter.cc >> > > +++ b/src/box/alter.cc >> > > @@ -2914,6 +2914,11 @@ on_replace_dd_truncate(struct = trigger * >> > /* trigger */, void *event) >> > > if (space_is_temporary(old_space) || >> > > space_group_id(old_space) =3D=3D GROUP_LOCAL) { >> > > stmt->row->group_id =3D GROUP_LOCAL; >> > > + /* >> > > + * The trigger is invoked after >> txn->n_local_rows >> > > + * is counted, so don't forget to update it >> here. >> > > + */ >> > > + ++txn->n_local_rows; >> > > } >> > > >> > > try { >> > >> > The patch doesn't build on my machine. For the same reason >> as the >> > other >> > patch. >> > I suppose you'll fix that with the other patch. >> > >> > > diff --git a/test/replication-luatest/gh_6123_test.lua >> > b/test/replication-luatest/gh_6123_test.lua >> > > new file mode 100644 >> > > index 000000000..d0ffa0c8b >> > > --- /dev/null >> > > +++ b/test/replication-luatest/gh_6123_test.lua >> > >> > Please, find a better test name. >> > Like "gh_6123_truncate_temp_space_test.lua" >> > > @@ -0,0 +1,62 @@ >> > > +local fio =3D require('fio') >> > > +local log =3D require('log') >> > > +local t =3D require('luatest') >> > > +local cluster =3D require('test.luatest_helpers.cluster') >> > > +local helpers =3D require('test.luatest_helpers.helpers') >> > > + >> > > +local g =3D t.group('gh-6123') >> > > + >> > > +g.before_test('test_truncate_is_local_transaction', >> function() >> > > + g.cluster =3D cluster:new({}) >> > > + >> > > + local box_cfg =3D { >> > > + replication =3D { >> > > + helpers.instance_uri('master') >> > > + }, >> > > + replication_timeout =3D 0.1, >> > > + read_only =3D false >> > > + } >> > > + >> > > + g.master =3D g.cluster:build_server({alias =3D = 'master'}, >> > engine, box_cfg) >> > > + >> > > + local box_cfg =3D { >> > > + replication =3D { >> > > + helpers.instance_uri('master'), >> > > + helpers.instance_uri('replica') >> > > + }, >> > > + replication_timeout =3D 0.1, >> > > + replication_connect_timeout =3D 0.5, >> > > + read_only =3D true >> > > + } >> > > + >> > > + g.replica =3D g.cluster:build_server({alias =3D = 'replica'}, >> > engine, box_cfg) >> > > + >> > > + g.cluster:join_server(g.master) >> > > + g.cluster:join_server(g.replica) >> > > + g.cluster:start() >> > > + log.info ('Everything >> is started') >> > > +end) >> > > + >> > > +g.after_test('test_truncate_is_local_transaction', = function() >> > > + g.cluster:stop() >> > > + fio.rmtree(g.master.workdir) >> > > + fio.rmtree(g.replica.workdir) >> > > +end) >> > > + >> > > +g.test_truncate_is_local_transaction =3D function() >> > > + g.master:eval("s =3D box.schema.space.create('temp', >> > {temporary =3D true})") >> > > + g.master:eval("s:create_index('pk')") >> > > + >> > > + g.master:eval("s:insert{1, 2}") >> > > + g.master:eval("s:insert{4}") >> > > + t.assert_equals(g.master:eval("return s:select()"), = {{1, >> > 2}, {4}}) >> > > + >> > > + g.master:eval("box.begin() >> > box.space._schema:replace{'smth'} s:truncate() = box.commit()") >> > > + t.assert_equals(g.master:eval("return s:select()"), = {}) >> > > + t.assert_equals(g.master:eval("return >> > box.space._schema:select{'smth'}"), {{'smth'}}) >> > > + >> > > + -- Checking that replica has received the last >> transaction, >> > > + -- and that replication isn't broken. >> > > + t.assert_equals(g.replica:eval("return >> > box.space._schema:select{'smth'}"), {{'smth'}}) >> > > + t.assert_equals(g.replica:eval("return >> > box.info.replication[1].upstream.status"), 'follow') >> > > +end >> > > -- >> > > 2.25.1 >> > > >> > >> > -- >> > Serge Petrenko >> > >>=20 >> -- Serge Petrenko >>=20 >=20 > --=20 > Serge Petrenko >=20