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

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

15.06.2021 20:02, Cyrill Gorcunov пишет:
> On Tue, Jun 15, 2021 at 05:26:06PM +0300, Serge Petrenko wrote:
>>> +box.space.sync:select{}
>>> + | ---
>>> + | - - [1]
>>> + | ...
>> You need to wait for the space creation, just like
>> you do below. Otherwise the test'll be flaky.
>> Also, please see a comment regarding wait_lsn vs wait_cond
>> below.
> As being discussed due to quorum=2 and sync space we don't need
> to wait.

Yes, indeed. I missed that.

>>> +test_run:wait_cond(function() return                    \
>>> +                   box.space.sync ~= nil and            \
>>> +                   box.space.sync:get{1} ~= nil and     \
>>> +                   box.space.sync:get{1}[1] == 1 end, 100)
>>> + | ---
>>> + | - true
>>> + | ...
>> I suggest you use wait_lsn('replica2', 'master') here
>> instead of this bulky wait_cond.
>> First of all, it takes a single line, instead of 4 lines.
>>
>> Secondly, you forgot to test `box.space.sync.index.pk ~= nil`, meaning
>> the test will still fail occasionally, when index creation doesn't replicate
>> in time.
> You mean something like below?
> ---
> diff --git a/test/replication/gh-6035-applier-filter.result b/test/replication/gh-6035-applier-filter.result
> index 2620e7b6f..7345a19f7 100644
> --- a/test/replication/gh-6035-applier-filter.result
> +++ b/test/replication/gh-6035-applier-filter.result
> @@ -102,12 +102,8 @@ test_run:switch('replica2')
>    | ---
>    | - true
>    | ...
> -test_run:wait_cond(function() return                    \
> -                   box.space.sync ~= nil and            \
> -                   box.space.sync:get{1} ~= nil and     \
> -                   box.space.sync:get{1}[1] == 1 end, 100)
> +test_run:wait_lsn('replica2', 'master')
>    | ---
> - | - true
>    | ...
>   box.space.sync:select{}
>    | ---
> diff --git a/test/replication/gh-6035-applier-filter.test.lua b/test/replication/gh-6035-applier-filter.test.lua
> index 9bfd91288..beca5258e 100644
> --- a/test/replication/gh-6035-applier-filter.test.lua
> +++ b/test/replication/gh-6035-applier-filter.test.lua
> @@ -53,10 +53,7 @@ box.space.sync:select{}
>   -- And the second hop is replica2 where
>   -- replica1 replicated the data to us.
>   test_run:switch('replica2')
> -test_run:wait_cond(function() return                    \
> -                   box.space.sync ~= nil and            \
> -                   box.space.sync:get{1} ~= nil and     \
> -                   box.space.sync:get{1}[1] == 1 end, 100)
> +test_run:wait_lsn('replica2', 'master')
>   box.space.sync:select{}
>   
>   test_run:switch('default')

Exactly.

-- 
Serge Petrenko