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 AC575469710
 for <tarantool-patches@dev.tarantool.org>;
 Sat, 21 Nov 2020 17:41:14 +0300 (MSK)
References: <cover.1605629206.git.sergeyb@tarantool.org>
 <5ab780fcf5f8c94a6516206a622ed05f4d659894.1605629206.git.sergeyb@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <211ff428-4511-6b0e-17e9-7783d2fb2eb2@tarantool.org>
Date: Sat, 21 Nov 2020 15:41:13 +0100
MIME-Version: 1.0
In-Reply-To: <5ab780fcf5f8c94a6516206a622ed05f4d659894.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 2/4] replication: test
 clear_synchro_queue() function
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!

See 2 comments below.

On 17.11.2020 17:13, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Part of #5055
> Part of #4849
> ---
>  test/replication/qsync_basic.result   | 129 ++++++++++++++++++++++++++
>  test/replication/qsync_basic.test.lua |  45 +++++++++
>  2 files changed, 174 insertions(+)
> 
> diff --git a/test/replication/qsync_basic.result b/test/replication/qsync_basic.result
> index bd3c3cce1..a06f65ce1 100644
> --- a/test/replication/qsync_basic.result
> +++ b/test/replication/qsync_basic.result
> @@ -637,6 +637,135 @@ box.space.sync:count()
>   | - 0
>   | ...
>  
> +--
> +-- gh-4849: clear synchro queue on a master
> +--
> +test_run:switch('default')
> + | ---
> + | - true
> + | ...
> +box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout = 1000}
> + | ---
> + | ...
> +ok, err = nil
> + | ---
> + | ...
> +f = fiber.create(function()							\
> +    ok, err = pcall(box.space.sync.insert, box.space.sync, {10})		\

1. In Lua files we use spaces and 4 symbols indentation step. I
know it would be easier if it would be like in C, but at this point
it is too late to change without a reason.

Here you apparently used tabs for '\' symbols.

> +end)
> + | ---
> + | ...
> +f:status()
> + | ---
> + | - suspended
> + | ...
> +test_run:switch('replica')
> + | ---
> + | - true
> + | ...
> +test_run:wait_cond(function() return box.space.sync:get{10} ~= nil end)
> + | ---
> + | - true
> + | ...
> +test_run:switch('default')
> + | ---
> + | - true
> + | ...
> +box.cfg{replication_synchro_timeout = 0.1}
> + | ---
> + | ...
> +box.ctl.clear_synchro_queue()

2. Current behaviour of the queue clearance is incorrect,
because it should never rollback local rows, see
https://github.com/tarantool/tarantool/issues/5435. It is
fine to have this test, but better leave a comment, that the
test must stop working eventually, and it will be ok to
delete or rework it.