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 F27476EC40; Mon, 27 Sep 2021 09:31:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F27476EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1632724290; bh=mB3qsBykRvHV+HzGiQbkXoGQ3wCPeHwQzfzM3CAIDhg=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=yjp6+Swo2S1k/N3TC4yUMNeiIogYZ+Ifhv1nnLpq1/RZPKUuDB82JqhXtStJhCHny fKcTPq4VHYOzLgA8/Ql4LXAA8GKhqRtPOxb98E5goxW8PzgS61La10qsWVGGydj7tj r6UUBgSSJVPVDvLZFD81iaz0g2FpFl+nHX0r1b48= 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 2766B6EC40 for ; Mon, 27 Sep 2021 09:31:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2766B6EC40 Received: by smtp50.i.mail.ru with esmtpa (envelope-from ) id 1mUkAh-0004tf-7R; Mon, 27 Sep 2021 09:31:28 +0300 To: Yan Shtunder , shemeneval@gmail.com Cc: tarantool-patches@dev.tarantool.org References: <20210921163110.39841-1-ya.shtunder@gmail.com> Message-ID: Date: Mon, 27 Sep 2021 09:31:26 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20210921163110.39841-1-ya.shtunder@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: ru X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD96A58C36AA2E996498E412B95EB25F30472F9DA0B5FF28EB2182A05F538085040964C0D94500897A2BECEC03DE62701EA86DF462909492D4C5D0A7FB595B139F3 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7DC4D3EB6E237C32AEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063768D6DD405B71470F8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8EE81731ABE39A253E228A1B8D95E44A6117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD182CC0D3CB04F14752D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6B4612C9E9B341BF4089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5C56344DD24D3DFD5E3AF708FFB4E026BDF3101674B88421CD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75BFC02AB3DF06BA5A410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3458239C6F30CE7ACF811322A92A171A9D1BA6EA6E3306BC7D9AF99AC9F86862B672C99FAFDCBFAB201D7E09C32AA3244C8D95E0C9BC3E888CEBDC46C9C299FC50A95CA90A1D8AC565FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojnnOrOlyI/jPuopH/PHJbxA== X-Mailru-Sender: 3B9A0136629DC9125D61937A2360A446DB4F97D982AFF09D94CAD5F6E0D2FE1342AB7DCDE6D22970424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH] 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: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 21.09.2021 19:31, Yan Shtunder пишет: > 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 Hi! Thanks for the patch! > --- > Issue: https://github.com/tarantool/tarantool/issues/6123 > Patch: https://github.com/tarantool/tarantool/tree/yshtunder/gh-6123-truncate-is-local-transaction Please, rebase the branch on top of current master. > > src/box/alter.cc | 11 +++ > test/replication-luatest/gh_6123_test.lua | 61 +++++++++++++ > .../instance_files/master.lua | 22 +++++ > .../instance_files/replica.lua | 21 +++++ > test/replication-luatest/suite.ini | 3 + > ...-6123-truncate-is-local-transaction.result | 90 +++++++++++++++++++ > ...123-truncate-is-local-transaction.test.lua | 36 ++++++++ > 7 files changed, 244 insertions(+) > create mode 100644 test/replication-luatest/gh_6123_test.lua > create mode 100755 test/replication-luatest/instance_files/master.lua > create mode 100755 test/replication-luatest/instance_files/replica.lua > create mode 100644 test/replication-luatest/suite.ini > create mode 100644 test/replication/gh-6123-truncate-is-local-transaction.result > create mode 100644 test/replication/gh-6123-truncate-is-local-transaction.test.lua > > diff --git a/src/box/alter.cc b/src/box/alter.cc > index 89bb5946c..35bd603d0 100644 > --- a/src/box/alter.cc > +++ b/src/box/alter.cc > @@ -2901,6 +2901,17 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) > if (space_is_temporary(old_space) || > space_group_id(old_space) == GROUP_LOCAL) { > stmt->row->group_id = GROUP_LOCAL; > + /* > + * In order to append a dummy NOP statement > + * to preserve the tx replication boundaries, > + * it is necessary that the condition > + * txn->n_local_rows > 0 must be true. > + * Therefore it is necessary to increase the > + * value of n_local_rows, because the checking > + * on the flag of GROUP_LOCAL will occurs > + * before it is set. > + */ > + ++txn->n_local_rows; Let's shorten the comment to something like "The trigger is invoked after txn->n_local_rows is counted, so don't forget to update it here." > } > > try { > diff --git a/test/replication-luatest/gh_6123_test.lua b/test/replication-luatest/gh_6123_test.lua > new file mode 100644 > index 000000000..7341ccc95 > --- /dev/null > +++ b/test/replication-luatest/gh_6123_test.lua > @@ -0,0 +1,61 @@ > +local fio = require('fio') > +local log = require('log') > + > +local t = require('luatest') > +local g = t.group() > + > +local Server = t.Server > + > +g.before_all(function() > + g.master = Server:new({ > + alias = 'master', > + command = './test/replication-luatest/instance_files/master.lua', > + workdir = fio.tempdir(), > + env = {TARANTOOL_REPLICA = '13302'}, > + http_port = 8081, > + net_box_port = 13301, Can the command be shortened to `./master.lua`? Can you omit the http_port? > + }) > + > + g.replica = Server:new({ > + alias = 'replica', > + command = './test/replication-luatest/instance_files/replica.lua', > + workdir = fio.tempdir(), > + env = {TARANTOOL_MASTER = '13301'}, > + http_port = 8082, > + net_box_port = 13302, > + }) > + > + > + g.master:start() > + g.replica:start() > + > + t.helpers.retrying({}, function() g.master:connect_net_box() end) > + t.helpers.retrying({}, function() g.replica:connect_net_box() end) > + > + log.info('Everything is started') > +end) > + > +g.after_all(function() > + g.replica:stop() > + g.master:stop() > + fio.rmtree(g.master.workdir) > + fio.rmtree(g.replica.workdir) > +end) > + > +g.test_truncate_is_local_transaction = function() > + g.master:eval("s = box.schema.space.create('temp', {temporary = 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 > diff --git a/test/replication-luatest/instance_files/master.lua b/test/replication-luatest/instance_files/master.lua > new file mode 100755 > index 000000000..9063ad1db > --- /dev/null > +++ b/test/replication-luatest/instance_files/master.lua > @@ -0,0 +1,22 @@ > +#!/usr/bin/env tarantool > + > +local function instance_uri(instance) > + local port = os.getenv(instance) > + return 'localhost:' .. port > +end > + > +box.cfg({ > + --log_level = 7, > + work_dir = os.getenv('TARANTOOL_WORKDIR'), > + listen = os.getenv('TARANTOOL_LISTEN'), > + replication = { > + instance_uri('TARANTOOL_LISTEN'), > + instance_uri('TARANTOOL_REPLICA') > + }, > + memtx_memory = 107374182, > + replication_timeout = 0.1, > + read_only = false > +}) > + > +box.schema.user.grant('guest', 'read, write, execute, create', 'universe', nil, {if_not_exists=true}) > +require('log').warn("master is ready") > diff --git a/test/replication-luatest/instance_files/replica.lua b/test/replication-luatest/instance_files/replica.lua > new file mode 100755 > index 000000000..493ec3aab > --- /dev/null > +++ b/test/replication-luatest/instance_files/replica.lua > @@ -0,0 +1,21 @@ > +#!/usr/bin/env tarantool > + > +local function instance_uri(instance) > + local port = os.getenv(instance) > + return 'localhost:' .. port > +end > + > +box.cfg({ > + work_dir = os.getenv('TARANTOOL_WORKDIR'), > + listen = os.getenv('TARANTOOL_LISTEN'), > + replication = { > + instance_uri('TARANTOOL_MASTER'), > + instance_uri('TARANTOOL_LISTEN') > + }, > + memtx_memory = 107374182, > + replication_timeout = 0.1, > + replication_connect_timeout = 0.5, > + read_only = true > +}) > + > +require('log').warn("replica is ready") > diff --git a/test/replication-luatest/suite.ini b/test/replication-luatest/suite.ini > new file mode 100644 > index 000000000..ccd624099 > --- /dev/null > +++ b/test/replication-luatest/suite.ini > @@ -0,0 +1,3 @@ > +[default] > +core = luatest > +description = first try of using luatest > diff --git a/test/replication/gh-6123-truncate-is-local-transaction.result b/test/replication/gh-6123-truncate-is-local-transaction.result > new file mode 100644 > index 000000000..db57d37d8 > --- /dev/null > +++ b/test/replication/gh-6123-truncate-is-local-transaction.result You don't need the diff-based test anymore. Please, remove it. > @@ -0,0 +1,90 @@ > +-- test-run result file version 2 > +test_run = require('test_run').new() > + | --- > + | ... > + > + > +-- Step 1 > +box.schema.user.grant("guest", "replication") > + | --- > + | ... > +s = box.schema.space.create("temp", {temporary=true}) > + | --- > + | ... > +_ = s:create_index('pk') > + | --- > + | ... > +s:insert{1,2} > + | --- > + | - [1, 2] > + | ... > +s:insert{4} > + | --- > + | - [4] > + | ... > +s:select() > + | --- > + | - - [1, 2] > + | - [4] > + | ... > + > + > +-- Step 2 > +test_run:cmd('create server replica with rpl_master=default,\ > + script="replication/replica.lua"') > + | --- > + | - true > + | ... > +test_run:cmd('start server replica') > + | --- > + | - true > + | ... > + > + > +-- Step 3 > +box.begin() box.space._schema:replace{"smth"} s:truncate() box.commit() > + | --- > + | ... > +s:select() > + | --- > + | - [] > + | ... > +box.space._schema:select{'smth'} > + | --- > + | - - ['smth'] > + | ... > + > + > +-- Step 4 > +-- Checking that replica has received the last transaction, > +-- and that replication isn't broken. > +test_run:switch('replica') > + | --- > + | - true > + | ... > +box.space._schema:select{'smth'} > + | --- > + | - - ['smth'] > + | ... > +box.info.replication[1].upstream.status > + | --- > + | - follow > + | ... > + > + > +test_run:switch('default') > + | --- > + | - true > + | ... > +test_run:cmd('stop server replica') > + | --- > + | - true > + | ... > +test_run:cmd('cleanup server replica') > + | --- > + | - true > + | ... > +test_run:cmd('delete server replica') > + | --- > + | - true > + | ... > diff --git a/test/replication/gh-6123-truncate-is-local-transaction.test.lua b/test/replication/gh-6123-truncate-is-local-transaction.test.lua > new file mode 100644 > index 000000000..336c0c44d > --- /dev/null > +++ b/test/replication/gh-6123-truncate-is-local-transaction.test.lua > @@ -0,0 +1,36 @@ > +test_run = require('test_run').new() > + > + > +-- Step 1 > +box.schema.user.grant("guest", "replication") > +s = box.schema.space.create("temp", {temporary=true}) > +_ = s:create_index('pk') > +s:insert{1,2} > +s:insert{4} > +s:select() > + > + > +-- Step 2 > +test_run:cmd('create server replica with rpl_master=default,\ > + script="replication/replica.lua"') > +test_run:cmd('start server replica') > + > + > +-- Step 3 > +box.begin() box.space._schema:replace{"smth"} s:truncate() box.commit() > +s:select() > +box.space._schema:select{'smth'} > + > + > +-- Step 4 > +-- Checking that replica has received the last transaction, > +-- and that replication isn't broken. > +test_run:switch('replica') > +box.space._schema:select{'smth'} > +box.info.replication[1].upstream.status > + > + > +test_run:switch('default') > +test_run:cmd('stop server replica') > +test_run:cmd('cleanup server replica') > +test_run:cmd('delete server replica') > -- > 2.25.1 > -- Serge Petrenko