From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 553B84765E0 for ; Wed, 23 Dec 2020 18:54:15 +0300 (MSK) Received: by mail-lf1-f49.google.com with SMTP id h205so41130185lfd.5 for ; Wed, 23 Dec 2020 07:54:15 -0800 (PST) Date: Wed, 23 Dec 2020 18:54:12 +0300 From: Cyrill Gorcunov Message-ID: <20201223155412.GE8261@grain> References: <20201222111408.48368-1-gorcunov@gmail.com> <8b570db3-858d-4475-1f9f-7e5c84321129@tarantool.org> <20201222171352.GC8261@grain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH v6 0/5] qsync: evaluate replication_synchro_quorum dynamically List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tml 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, \ +}