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 --- Issue: https://github.com/tarantool/tarantool/issues/6123 Patch: https://github.com/tarantool/tarantool/tree/yshtunder/gh-6123-truncate-is-local-transaction src/box/txn.c | 16 +++-- ...-6123-truncate-is-local-transaction.result | 69 +++++++++++++++++++ ...123-truncate-is-local-transaction.test.lua | 27 ++++++++ 3 files changed, 107 insertions(+), 5 deletions(-) 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/txn.c b/src/box/txn.c index b80e722a4..d37bec7ae 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -397,6 +397,7 @@ txn_is_distributed(struct txn *txn) int txn_commit_stmt(struct txn *txn, struct request *request) { + int rc = 0; assert(txn->in_sub_stmt > 0); /* * Run on_replace triggers. For now, disallow mutation @@ -415,9 +416,6 @@ txn_commit_stmt(struct txn *txn, struct request *request) assert(stmt->row != NULL); if (stmt->row->replica_id == 0) { ++txn->n_new_rows; - if (stmt->row->group_id == GROUP_LOCAL) - ++txn->n_local_rows; - } else { ++txn->n_applier_rows; } @@ -447,10 +445,18 @@ txn_commit_stmt(struct txn *txn, struct request *request) */ stmt->does_require_old_tuple = true; - if(trigger_run(&stmt->space->on_replace, txn) != 0) - goto fail; + rc = trigger_run(&stmt->space->on_replace, txn); } } + + if (stmt->row != NULL && stmt->row->replica_id == 0 && + stmt->row->group_id == GROUP_LOCAL) { + ++txn->n_local_rows; + } + + if (rc != 0) + goto fail; + --txn->in_sub_stmt; return 0; fail: 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..0a4b990c1 --- /dev/null +++ b/test/replication/gh-6123-truncate-is-local-transaction.result @@ -0,0 +1,69 @@ +-- 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'] + | ... + + +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..b68514d0b --- /dev/null +++ b/test/replication/gh-6123-truncate-is-local-transaction.test.lua @@ -0,0 +1,27 @@ +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'} + + +test_run:cmd('stop server replica') +test_run:cmd('cleanup server replica') +test_run:cmd('delete server replica') -- 2.25.1
25.08.2021 08:54, 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! Please, find a couple of 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/txn.c | 16 +++-- > ...-6123-truncate-is-local-transaction.result | 69 +++++++++++++++++++ > ...123-truncate-is-local-transaction.test.lua | 27 ++++++++ > 3 files changed, 107 insertions(+), 5 deletions(-) > 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/txn.c b/src/box/txn.c > index b80e722a4..d37bec7ae 100644 > --- a/src/box/txn.c > +++ b/src/box/txn.c > @@ -397,6 +397,7 @@ txn_is_distributed(struct txn *txn) > int > txn_commit_stmt(struct txn *txn, struct request *request) > { > + int rc = 0; > assert(txn->in_sub_stmt > 0); > /* > * Run on_replace triggers. For now, disallow mutation > @@ -415,9 +416,6 @@ txn_commit_stmt(struct txn *txn, struct request *request) > assert(stmt->row != NULL); > if (stmt->row->replica_id == 0) { > ++txn->n_new_rows; > - if (stmt->row->group_id == GROUP_LOCAL) > - ++txn->n_local_rows; > - > } else { > ++txn->n_applier_rows; > } Why can't you move this whole block lower, not just the if() body? I mean both if() {} and else {} > @@ -447,10 +445,18 @@ txn_commit_stmt(struct txn *txn, struct request *request) > */ > stmt->does_require_old_tuple = true; > > - if(trigger_run(&stmt->space->on_replace, txn) != 0) > - goto fail; > + rc = trigger_run(&stmt->space->on_replace, txn); > } > } > + > + if (stmt->row != NULL && stmt->row->replica_id == 0 && > + stmt->row->group_id == GROUP_LOCAL) { > + ++txn->n_local_rows; > + } > + > + if (rc != 0) > + goto fail; > + > --txn->in_sub_stmt; > return 0; This could be written simpler: if (rc != 0) { // the code below the "fail" label goes here } else { --txn->in_sub_stmt; } return rc; > fail: > 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..0a4b990c1 > --- /dev/null > +++ b/test/replication/gh-6123-truncate-is-local-transaction.result > @@ -0,0 +1,69 @@ > +-- 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'] > + | ... > + > + > +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..b68514d0b > --- /dev/null > +++ b/test/replication/gh-6123-truncate-is-local-transaction.test.lua > @@ -0,0 +1,27 @@ > +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'} > + > + > +test_run:cmd('stop server replica') > +test_run:cmd('cleanup server replica') > +test_run:cmd('delete server replica') > -- > 2.25.1 > The test looks good with one comment: Before stopping the replica, you should check that replica has received the last transaction, and that replication isn't broken. -- Serge Petrenko
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 --- 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 | 11 +++ ...-6123-truncate-is-local-transaction.result | 90 +++++++++++++++++++ ...123-truncate-is-local-transaction.test.lua | 36 ++++++++ 3 files changed, 137 insertions(+) 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; } try { 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 @@ -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
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 --- 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 | 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; } 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, + }) + + 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 @@ -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
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