Tarantool development patches archive
 help / color / mirror / Atom feed
From: Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Cyrill Gorcunov <gorcunov@gmail.com>,
	tml <tarantool-patches@dev.tarantool.org>
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v8 1/2] applier: filter incoming synchro packets via transaction initiator
Date: Tue, 15 Jun 2021 14:55:04 +0300
Message-ID: <9a0f867f-f890-7f2a-ab39-05ffbd68b286@tarantool.org> (raw)
In-Reply-To: <7f270051-995e-f94f-2bda-f2fd065bf59e@tarantool.org>



15.06.2021 14:35, Serge Petrenko via Tarantool-patches пишет:
>
>
> 11.06.2021 18:22, Cyrill Gorcunov пишет:
>> Currently we use synchro packets filtration based on their contents,
>> in particular by their xrow->replica_id value. Still there was a
>> question if we can optimize this moment and rather filter out all
>> packets coming from non-leader replica.
>>
>> Raft specification requires that only data from a current leader
>> should be applied to local WAL but doesn't put a concrete claim on
>> the data transport, ie how exactly rows are reaching replicas. This
>> implies that data propagation may reach replicas indirectly via transit
>> hops. Thus we drop applier->instance_id filtering and rely on
>> xrow->replica_id matching instead.
>>
>> In the test (inspired by Serge Petrenko's test) we recreate the 
>> situation
>> where replica3 obtains master's node data (which is a raft leader)
>> indirectly via replica2 node.
>>
>> Closes #6035
>>
>> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
>> ---
>>
>
> I've found a way to make the test work as discussed: replication 1 <-> 
> 2 -> 3 from
> the beginning. Without replication reconfiguration. Take a look at the 
> diff below.
>
> Although the diff doesn't simplify  anything, and it makes the test 
> run longer: 2.5 s vs 0.8 s
> on my machine. So it's up to you whether you want to apply it.
>
> I'm starting to like your test version more, to be honest.
>
>
> Here's the diff:

Pushed the diff on top of your branch as a separate commit.
> =================================================
>
> diff --git a/test/replication/gh-6035-applier-filter.result 
> b/test/replication/gh-6035-applier-filter.result
> index 077cd46e5..4ddebcc6b 100644
> --- a/test/replication/gh-6035-applier-filter.result
> +++ b/test/replication/gh-6035-applier-filter.result
> @@ -31,7 +31,7 @@ test_run:cmd('create server replica2 with 
> script="replication/gh-6035-replica2.l
>   | - true
>   | ...
>
> -test_run:cmd('start server master')
> +test_run:cmd('start server master with wait=False')
>   | ---
>   | - true
>   | ...
> @@ -44,25 +44,24 @@ test_run:cmd('start server replica2')
>   | - true
>   | ...
>
> -test_run:switch('replica2')
> +--
> +-- Make the master to be RAFT leader.
> +test_run:switch('replica1')
>   | ---
>   | - true
>   | ...
> -box.cfg{replication = {require('fio').cwd() .. "/replica1.sock"}}
> +box.cfg{election_mode = 'voter'}
>   | ---
>   | ...
>
> ---
> --- Make the master to be RAFT leader.
>  test_run:switch('master')
>   | ---
>   | - true
>   | ...
> +test_run:wait_lsn('master', 'replica1')
> + | ---
> + | ...
>  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/gh-6035-applier-filter.test.lua 
> b/test/replication/gh-6035-applier-filter.test.lua
> index 4e72abe5f..4ec7a6e6a 100644
> --- a/test/replication/gh-6035-applier-filter.test.lua
> +++ b/test/replication/gh-6035-applier-filter.test.lua
> @@ -19,21 +19,18 @@ test_run:cmd('create server master with 
> script="replication/gh-6035-master.lua"'
>  test_run:cmd('create server replica1 with 
> script="replication/gh-6035-replica1.lua"')
>  test_run:cmd('create server replica2 with 
> script="replication/gh-6035-replica2.lua"')
>
> -test_run:cmd('start server master')
> +test_run:cmd('start server master with wait=False')
>  test_run:cmd('start server replica1')
>  test_run:cmd('start server replica2')
>
> -test_run:switch('replica2')
> -box.cfg{replication = {require('fio').cwd() .. "/replica1.sock"}}
> -
>  --
>  -- Make the master to be RAFT leader.
> +test_run:switch('replica1')
> +box.cfg{election_mode = 'voter'}
> +
>  test_run:switch('master')
> +test_run:wait_lsn('master', 'replica1')
>  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/gh-6035-node.lua 
> b/test/replication/gh-6035-node.lua
> index e3819471a..4ed41b3cf 100644
> --- a/test/replication/gh-6035-node.lua
> +++ b/test/replication/gh-6035-node.lua
> @@ -10,6 +10,10 @@ require('console').listen(os.getenv('ADMIN'))
>  if INSTANCE_ID == "master" then
>      box.cfg({
>          listen = unix_socket("master"),
> +        replication = {
> +            unix_socket("master"),
> +            unix_socket("replica1")
> +        },
>      })
>  elseif INSTANCE_ID == "replica1" then
>      box.cfg({
> @@ -18,13 +22,12 @@ elseif INSTANCE_ID == "replica1" then
>              unix_socket("master"),
>              unix_socket("replica1")
>          },
> -        election_mode = 'voter'
>      })
>  else
>      assert(INSTANCE_ID == "replica2")
>      box.cfg({
>          replication = {
> -            unix_socket("master"),
> +            unix_socket("replica1"),
>          },
>          election_mode = 'voter'
>      })
>
>
> ====================================================
>

-- 
Serge Petrenko


  reply	other threads:[~2021-06-15 11:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11 15:22 [Tarantool-patches] [PATCH v8 0/2] filter incoming packets Cyrill Gorcunov via Tarantool-patches
2021-06-11 15:22 ` [Tarantool-patches] [PATCH v8 1/2] applier: filter incoming synchro packets via transaction initiator Cyrill Gorcunov via Tarantool-patches
2021-06-15 10:36   ` Serge Petrenko via Tarantool-patches
2021-06-15 11:35   ` Serge Petrenko via Tarantool-patches
2021-06-15 11:55     ` Serge Petrenko via Tarantool-patches [this message]
2021-06-11 15:22 ` [Tarantool-patches] [PATCH v8 2/2] Vlad: applier filtration Cyrill Gorcunov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9a0f867f-f890-7f2a-ab39-05ffbd68b286@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergepetrenko@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git