Re: [Tarantool-patches] [PATCH v9 0/1] applier: filter incoming synchro packets via transaction initiator

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the patch!

See 4 comments below.

> diff --git a/test/replication/gh-6035-applier-filter.result b/test/replication/gh-6035-applier-filter.result
> new file mode 100644
> index 000000000..7345a19f7
> --- /dev/null
> +++ b/test/replication/gh-6035-applier-filter.result

1. Maybe better rename it to gh-6035-election-filter. Because otherwise
this test is not included when I run `python test-run.py election`. And I
do that quite often when make a patch which affects election only.

> @@ -0,0 +1,140 @@
> +-- test-run result file version 2
> +--
> +-- gh-6035: verify synchronous rows filtration in applier,
> +-- we need to be sure that filtering synchronous rows is
> +-- done via transaction initiator not sender (iow via
> +-- xrow->replica_id).
> +--
> +test_run = require('test_run').new()
> + | ---
> + | ...
> +
> +--
> +-- Prepare a scheme with transitional node
> +--
> +--  master <=> replica1 => replica2
> +--
> +-- such as transaction initiated on the master node would
> +-- be replicated to the replica2 via interim replica1 node.
> +--
> +
> +test_run:cmd('create server master with script="replication/gh-6035-master.lua"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('create server replica1 with script="replication/gh-6035-replica1.lua"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('create server replica2 with script="replication/gh-6035-replica2.lua"')
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('start server master')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica1')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica2')
> + | ---
> + | - true
> + | ...
> +
> +test_run:switch('replica2')
> + | ---
> + | - true
> + | ...
> +box.cfg{replication = {require('fio').cwd() .. "/replica1.sock"}}

2. You don't need require('fio').cwd(). "unix/:./replica1.sock" works
just fine. The same for the other socket paths.

> + | ---
> + | ...
> +
> +--
> +-- Make the master to be RAFT leader, this drops connection
> +-- to the replica2.

3. There was no connection to replica2 from master.

> +test_run:switch('master')
> + | ---
> + | - true
> + | ...
> +box.cfg({                                               \
> +    replication = {                                     \
> +            require('fio').cwd() .. "/master.sock",     \
> +            require('fio').cwd() .. "/replica1.sock",   \
> +        },                                              \
> +    replication_synchro_quorum = 2,                     \
> +    election_mode = 'manual',                           \
> +})
> diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
> index 3a0a8649f..3b5cee75b 100644
> --- a/test/replication/suite.cfg
> +++ b/test/replication/suite.cfg
> @@ -50,6 +50,9 @@
>      "gh-6057-qsync-confirm-async-no-wal.test.lua": {},
>      "gh-6094-rs-uuid-mismatch.test.lua": {},
>      "gh-6127-election-join-new.test.lua": {},
> +    "gh-6035-applier-filter.test.lua": {
> +        "memtx": {"engine": "memtx"}
> +    },

4. You don't need to specify the engine. You don't use the
engine variable in the test. Just leave it empty {}.