From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
 bits)) (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 62DF14696C1
 for <tarantool-patches@dev.tarantool.org>;
 Thu, 26 Nov 2020 01:02:30 +0300 (MSK)
References: <cover.1605629206.git.sergeyb@tarantool.org>
 <ada63b17702e8be6f223788368309767228b8102.1605629206.git.sergeyb@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <ea46f682-cb92-c759-129a-f41bb0ee202d@tarantool.org>
Date: Wed, 25 Nov 2020 23:02:28 +0100
MIME-Version: 1.0
In-Reply-To: <ada63b17702e8be6f223788368309767228b8102.1605629206.git.sergeyb@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH 4/4] replication: add test with
 change space sync mode in a loop
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: sergeyb@tarantool.org, tarantool-patches@dev.tarantool.org

Thanks for the patch!

> diff --git a/test/replication/qsync_sync_mode.result b/test/replication/qsync_sync_mode.result
> new file mode 100644
> index 000000000..66f6a70b2
> --- /dev/null
> +++ b/test/replication/qsync_sync_mode.result
> @@ -0,0 +1,114 @@
> +test_run:cmd('start server replica with wait=True, wait_load=True')
> + | ---
> + | - true
> + | ...
> +
> +-- Write data to a leader and enable/disable sync mode in a loop.
> +-- Testcase setup.
> +_ = box.schema.space.create('sync', {is_sync=true, engine=engine})
> + | ---
> + | ...
> +_ = box.space.sync:create_index('pk')
> + | ---
> + | ...
> +box.cfg{replication_synchro_quorum=2, replication_synchro_timeout=1000}
> + | ---
> + | ...
> +
> +-- Testcase body.
> +for i = 1,100 do                                                               \
> +    box.space.sync:alter{is_sync=random_boolean()}                             \

I thought about it more, and I realized that it does not make
much sense to test alter of kind true -> true, and false -> false.
Maybe better simply change it to negative on each iteration.

Also if the sync mode is true, wait_cond is supposed to be not
needed. That is the purpose of the sync. Currently your test will
pass even if I will simply remove is_sync option at all.

> +    box.space.sync:insert{i}                                                   \
> +    test_run:switch('replica')                                                 \
> +    test_run:wait_cond(function() return box.space.sync:get{i} ~= nil end)     \
> +    test_run:switch('default')                                                 \
> +    test_run:wait_cond(function() return box.space.sync:get{i} ~= nil end)     \
> +end
> + | ---
> + | ...
> +box.space.sync:count() -- 100
> + | ---
> + | - 100
> + | ...