Re: [Tarantool-patches] [PATCH v4 14/12] txn: make NOPs fully asynchronous

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

21.04.2021 01:31, Vladislav Shpilevoy пишет:
> Thanks for the patch!
>
> I added a test + an explanation in the comments in my commit. See
> below and on the branch (sp/gh-5445-election-fixes-review):

Looks good, squashed as well.
>
> ====================
>      [tosquash] Add reason and test for nop limbo bypass
>
> diff --git a/src/box/txn.c b/src/box/txn.c
> index 8be102666..03b39e0de 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -603,7 +603,10 @@ txn_journal_entry_new(struct txn *txn)
>   	bool is_sync = false;
>   	/*
>   	 * A transaction which consists of NOPs solely should pass through the
> -	 * limbo without waiting. Even when the limbo is not empty.
> +	 * limbo without waiting. Even when the limbo is not empty. This is
> +	 * because otherwise they might fail with the limbo being not owned by
> +	 * the NOPs owner. But it does not matter, because they just need to
> +	 * bump vclock. There is nothing to confirm or rollback in them.
>   	 */
>   	bool is_fully_nop = true;
>   
> diff --git a/test/replication/qsync_basic.result b/test/replication/qsync_basic.result
> index 3457d2cc9..7e711ba13 100644
> --- a/test/replication/qsync_basic.result
> +++ b/test/replication/qsync_basic.result
> @@ -637,6 +637,67 @@ box.space.sync:count()
>    | - 0
>    | ...
>   
> +--
> +-- gh-5445: NOPs bypass the limbo for the sake of vclock bumps from foreign
> +-- instances, but also works for local rows.
> +--
> +test_run:switch('default')
> + | ---
> + | - true
> + | ...
> +box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout = 1000}
> + | ---
> + | ...
> +f = fiber.create(function() box.space.sync:replace{1} end)
> + | ---
> + | ...
> +test_run:wait_lsn('replica', 'default')
> + | ---
> + | ...
> +
> +test_run:switch('replica')
> + | ---
> + | - true
> + | ...
> +function skip_row() return nil end
> + | ---
> + | ...
> +old_lsn = box.info.lsn
> + | ---
> + | ...
> +_ = box.space.sync:before_replace(skip_row)
> + | ---
> + | ...
> +box.space.sync:replace{2}
> + | ---
> + | ...
> +box.space.sync:before_replace(nil, skip_row)
> + | ---
> + | ...
> +assert(box.space.sync:get{2} == nil)
> + | ---
> + | - true
> + | ...
> +assert(box.space.sync:get{1} ~= nil)
> + | ---
> + | - true
> + | ...
> +
> +test_run:switch('default')
> + | ---
> + | - true
> + | ...
> +box.cfg{replication_synchro_quorum = 2}
> + | ---
> + | ...
> +test_run:wait_cond(function() return f:status() == 'dead' end)
> + | ---
> + | - true
> + | ...
> +box.space.sync:truncate()
> + | ---
> + | ...
> +
>   --
>   -- gh-5191: test box.info.synchro interface. For
>   -- this sake we stop the replica and initiate data
> diff --git a/test/replication/qsync_basic.test.lua b/test/replication/qsync_basic.test.lua
> index a604d80ee..75c9b222b 100644
> --- a/test/replication/qsync_basic.test.lua
> +++ b/test/replication/qsync_basic.test.lua
> @@ -248,6 +248,29 @@ for i = 1, 100 do box.space.sync:delete{i} end
>   test_run:cmd('switch replica')
>   box.space.sync:count()
>   
> +--
> +-- gh-5445: NOPs bypass the limbo for the sake of vclock bumps from foreign
> +-- instances, but also works for local rows.
> +--
> +test_run:switch('default')
> +box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout = 1000}
> +f = fiber.create(function() box.space.sync:replace{1} end)
> +test_run:wait_lsn('replica', 'default')
> +
> +test_run:switch('replica')
> +function skip_row() return nil end
> +old_lsn = box.info.lsn
> +_ = box.space.sync:before_replace(skip_row)
> +box.space.sync:replace{2}
> +box.space.sync:before_replace(nil, skip_row)
> +assert(box.space.sync:get{2} == nil)
> +assert(box.space.sync:get{1} ~= nil)
> +
> +test_run:switch('default')
> +box.cfg{replication_synchro_quorum = 2}
> +test_run:wait_cond(function() return f:status() == 'dead' end)
> +box.space.sync:truncate()
> +
>   --
>   -- gh-5191: test box.info.synchro interface. For
>   -- this sake we stop the replica and initiate data

-- 
Serge Petrenko