Re: [Tarantool-patches] [PATCH v6 0/5] qsync: evaluate replication_synchro_quorum dynamically

[Cyrill Gorcunov <gorcunov@gmail.com>]

On Wed, Dec 23, 2020 at 03:14:15PM +0100, Vladislav Shpilevoy wrote:
> > +-- Test big number value
> > +box.cfg{replication_synchro_quorum = '4294967297'}
> 
> 1. This test is exactly the same as the next line, because
> both return true from cfg_isnumber(). So you didn't test overflow
> in the formula result here.

True :( You've fixed it.

> > +-- Use canonical majority formula
> > +box.cfg{replication_synchro_quorum = "N/2+1"}
> 
> 2. I asked it in the previous email and in the chat, but
> you didn't pay attention, did you? The option is never restored
> to the initial value in the end of the test. Timeout too. This
> could break the next tests running in the same test-run worker.
> 
> Also you didn't drop the space in the end.
> 
> Since we don't have much time, I fixed these issues, and pushed
> on top of the branch in a separate commit. Please, review and
> squash if you agree.

Thanks a huge, Vlad! I happen to miss that I need to do it.
Here is a diff over previous patches (test file only
to not spam alot). I pushed everything as

	gorcunov/gh-5446-eval-quorum-8
--
diff --git a/test/replication/gh-5446-qsync-eval-quorum.test.lua b/test/replication/gh-5446-qsync-eval-quorum.test.lua
index b905af3d9..62d87ddcb 100644
--- a/test/replication/gh-5446-qsync-eval-quorum.test.lua
+++ b/test/replication/gh-5446-qsync-eval-quorum.test.lua
@@ -3,6 +3,10 @@ engine = test_run:get_cfg('engine')
 
 box.schema.user.grant('guest', 'replication')
 
+-- Save old settings for cleanup sake
+old_synchro_quorum = box.cfg.replication_synchro_quorum
+old_synchro_timeout = box.cfg.replication_synchro_timeout
+
 -- Test syntax error
 box.cfg{replication_synchro_quorum = "aaa"}
 
@@ -11,7 +15,8 @@ box.cfg{replication_synchro_quorum = "N+1"}
 box.cfg{replication_synchro_quorum = "N-1"}
 
 -- Test big number value
-box.cfg{replication_synchro_quorum = '4294967297'}
+box.cfg{replication_synchro_quorum = '4294967296 + 1'}
+box.cfg{replication_synchro_quorum = 'N + 4294967296'}
 box.cfg{replication_synchro_quorum = 4294967297}
 
 -- Timeouts for replication
@@ -23,9 +28,8 @@ box.cfg{replication_synchro_quorum = "N/2+1"}
 cfg_set_pass_tmo()
 
 -- Create a sync space we will operate on
-_ = box.schema.space.create('sync', {is_sync = true, engine = engine})
-_ = box.space.sync:create_index('pk')
-s = box.space.sync
+s = box.schema.space.create('sync', {is_sync = true, engine = engine})
+_ = s:create_index('pk')
 
 -- Only one master node -> 1/2 + 1 = 1
 s:insert{1} -- should pass
@@ -103,4 +107,11 @@ test_run:cmd('delete server replica5')
 test_run:cmd('stop server replica6')
 test_run:cmd('delete server replica6')
 
+s:drop()
+
 box.schema.user.revoke('guest', 'replication')
+
+box.cfg{                                                                        \
+    replication_synchro_quorum = old_synchro_quorum,                            \
+    replication_synchro_timeout = old_synchro_timeout,                          \
+}