From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Cyrill Gorcunov <gorcunov@gmail.com> Cc: tml <tarantool-patches@dev.tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v6 0/5] qsync: evaluate replication_synchro_quorum dynamically Date: Wed, 23 Dec 2020 15:14:15 +0100 [thread overview] Message-ID: <f9c76a2a-6276-7ec8-c798-6598861a8ac4@tarantool.org> (raw) In-Reply-To: <20201222171352.GC8261@grain> Hi! Thanks for the fixes! > diff --git a/test/replication/gh-5446-qsync-eval-quorum.result b/test/replication/gh-5446-qsync-eval-quorum.result > new file mode 100644 > index 000000000..832c3f6e5 > --- /dev/null > +++ b/test/replication/gh-5446-qsync-eval-quorum.result > @@ -0,0 +1,298 @@ > +-- test-run result file version 2 > +test_run = require('test_run').new() > + | --- > + | ... > +engine = test_run:get_cfg('engine') > + | --- > + | ... > + > +box.schema.user.grant('guest', 'replication') > + | --- > + | ... > + > +-- Test syntax error > +box.cfg{replication_synchro_quorum = "aaa"} > + | --- > + | - error: 'Incorrect value for option ''replication_synchro_quorum'': [string "local > + | expr = [[aaa]]..."]:7: Expression should return a number' > + | ... > + > +-- Test out of bounds values > +box.cfg{replication_synchro_quorum = "N+1"} > + | --- > + | - error: 'Incorrect value for option ''replication_synchro_quorum'': the formula is > + | evaluated to the quorum 32 for replica number 31, which is out of range [1;31]' > + | ... > +box.cfg{replication_synchro_quorum = "N-1"} > + | --- > + | - error: 'Incorrect value for option ''replication_synchro_quorum'': the formula is > + | evaluated to the quorum 0 for replica number 1, which is out of range [1;31]' > + | ... > + > +-- 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. > + | --- > + | - error: 'Incorrect value for option ''replication_synchro_quorum'': the value must > + | be greater than zero and less than maximal number of replicas' > + | ... > +box.cfg{replication_synchro_quorum = 4294967297} > + | --- > + | - error: 'Incorrect value for option ''replication_synchro_quorum'': the value must > + | be greater than zero and less than maximal number of replicas' > + | ... > + > +-- Timeouts for replication > +function cfg_set_pass_tmo() box.cfg{replication_synchro_timeout = 1000} end > + | --- > + | ... > +function cfg_set_fail_tmo() box.cfg{replication_synchro_timeout = 0.5} end > + | --- > + | ... > + > +-- 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. Also the diff is below: ==================== diff --git a/test/replication/gh-5446-qsync-eval-quorum.result b/test/replication/gh-5446-qsync-eval-quorum.result index 832c3f6e5..2acd633b2 100644 --- a/test/replication/gh-5446-qsync-eval-quorum.result +++ b/test/replication/gh-5446-qsync-eval-quorum.result @@ -10,6 +10,13 @@ box.schema.user.grant('guest', 'replication') | --- | ... +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"} | --- @@ -30,10 +37,10 @@ box.cfg{replication_synchro_quorum = "N-1"} | ... -- Test big number value -box.cfg{replication_synchro_quorum = '4294967297'} +box.cfg{replication_synchro_quorum = '4294967296 + 1'} | --- - | - error: 'Incorrect value for option ''replication_synchro_quorum'': the value must - | be greater than zero and less than maximal number of replicas' + | - error: 'Incorrect value for option ''replication_synchro_quorum'': the formula is + | evaluated to the quorum 1 for replica number 1, which is out of range [1;31]' | ... box.cfg{replication_synchro_quorum = 4294967297} | --- @@ -58,13 +65,10 @@ cfg_set_pass_tmo() | ... -- Create a sync space we will operate on -_ = box.schema.space.create('sync', {is_sync = true, engine = engine}) +s = box.schema.space.create('sync', {is_sync = true, engine = engine}) | --- | ... -_ = box.space.sync:create_index('pk') - | --- - | ... -s = box.space.sync +_ = s:create_index('pk') | --- | ... @@ -293,6 +297,17 @@ test_run:cmd('delete server replica6') | - true | ... +s:drop() + | --- + | ... + box.schema.user.revoke('guest', 'replication') | --- | ... + +box.cfg{ \ + replication_synchro_quorum = old_synchro_quorum, \ + replication_synchro_timeout = old_synchro_timeout, \ +} + | --- + | ... diff --git a/test/replication/gh-5446-qsync-eval-quorum.test.lua b/test/replication/gh-5446-qsync-eval-quorum.test.lua index b905af3d9..e8c067246 100644 --- a/test/replication/gh-5446-qsync-eval-quorum.test.lua +++ b/test/replication/gh-5446-qsync-eval-quorum.test.lua @@ -3,6 +3,9 @@ engine = test_run:get_cfg('engine') box.schema.user.grant('guest', 'replication') +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 +14,7 @@ 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 = 4294967297} -- Timeouts for replication @@ -23,9 +26,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 +105,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, \ +}
next prev parent reply other threads:[~2020-12-23 14:14 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-22 11:14 Cyrill Gorcunov 2020-12-22 11:14 ` [Tarantool-patches] [PATCH v6 1/5] cfg: add cfg_isnumber helper Cyrill Gorcunov 2020-12-22 11:14 ` [Tarantool-patches] [PATCH v6 2/5] cfg: rework box_check_replication_synchro_quorum Cyrill Gorcunov 2020-12-22 11:14 ` [Tarantool-patches] [PATCH v6 3/5] cfg: support symbolic evaluation of replication_synchro_quorum Cyrill Gorcunov 2020-12-22 11:14 ` [Tarantool-patches] [PATCH v6 4/5] cfg: more precise check for replication_synchro_quorum value Cyrill Gorcunov 2020-12-22 11:14 ` [Tarantool-patches] [PATCH v6 5/5] test: add replication/gh-5446-qsync-eval-quorum.test.lua Cyrill Gorcunov 2020-12-22 13:49 ` [Tarantool-patches] [PATCH v6 0/5] qsync: evaluate replication_synchro_quorum dynamically Vladislav Shpilevoy 2020-12-22 13:53 ` Cyrill Gorcunov 2020-12-22 14:32 ` Cyrill Gorcunov 2020-12-22 17:13 ` Cyrill Gorcunov 2020-12-23 14:14 ` Vladislav Shpilevoy [this message] 2020-12-23 15:54 ` Cyrill Gorcunov 2020-12-23 17:52 ` Vladislav Shpilevoy 2020-12-24 13:55 ` Vladislav Shpilevoy
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=f9c76a2a-6276-7ec8-c798-6598861a8ac4@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=gorcunov@gmail.com \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v6 0/5] qsync: evaluate replication_synchro_quorum dynamically' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox