From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (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 68175469710 for ; Mon, 23 Nov 2020 18:13:34 +0300 (MSK) References: <5ab780fcf5f8c94a6516206a622ed05f4d659894.1605629206.git.sergeyb@tarantool.org> <211ff428-4511-6b0e-17e9-7783d2fb2eb2@tarantool.org> From: Sergey Bronnikov Message-ID: <3f7eaafc-1f9f-4fe8-ab8b-a305ec3e83b1@tarantool.org> Date: Mon, 23 Nov 2020 18:13:33 +0300 MIME-Version: 1.0 In-Reply-To: <211ff428-4511-6b0e-17e9-7783d2fb2eb2@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH 2/4] replication: test clear_synchro_queue() function List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org Hi, thanks for review! On 21.11.2020 17:41, Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 2 comments below. > > On 17.11.2020 17:13, sergeyb@tarantool.org wrote: >> From: Sergey Bronnikov >> >> 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. Updated in a branch. I haven't put relevant patch here because whitespaces are broken after copy-paste. > >> +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. Updated comment: @@ -251,11 +251,16 @@ box.space.sync:count()  --  -- gh-4849: clear synchro queue on a master  -- +-- NOTE: current behavior of the queue clearance is incorrect, +-- because it should never rollback local rows, see +-- https://github.com/tarantool/tarantool/issues/5435. Test may +-- stop working eventually. +--  test_run:switch('default')  box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout = 1000}  ok, err = nil