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

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

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):

====================
    [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