From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (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 CCDE0469719 for ; Thu, 22 Oct 2020 11:53:05 +0300 (MSK) Date: Thu, 22 Oct 2020 11:53:03 +0300 From: "Alexander V. Tikhonov" Message-ID: <20201022085303.GA630047@hpalx> References: <15c343a9d2140a6c7ba31cb8761b2bca0ca6e09a.1602954690.git.v.shpilevoy@tarantool.org> <47356d83-4464-070b-0df8-c7fcbbdb1e02@tarantool.org> <1905f9a9-cddb-c016-0444-b665b5cbc948@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <1905f9a9-cddb-c016-0444-b665b5cbc948@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 1/3] raft: send state to new subscribers if Raft worked List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Hi Vlad, thanks that you dropped the patch and created the new issue. Without the patch testing of the test fixed. Check your branch rebased in my branch: https://gitlab.com/tarantool/tarantool/-/pipelines/206144990 On Wed, Oct 21, 2020 at 11:41:52PM +0200, Vladislav Shpilevoy wrote: > Hi! Thanks for the review! >=20 > >> diff --git a/src/box/box.cc b/src/box/box.cc > >> index 18568df3b..86daaf626 100644 > >> --- a/src/box/box.cc > >> +++ b/src/box/box.cc > >> =A0 @@ -2154,13 +2155,14 @@ box_process_subscribe(struct ev_io *io, st= ruct xrow_header *header) > >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 tt_uuid_str(&replica_uuid), sio_socketn= ame(io->fd)); > >> =A0=A0=A0=A0=A0 say_info("remote vclock %s local vclock %s", > >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 vclock_to_string(&replica_clock), vcloc= k_to_string(&vclock)); > >> -=A0=A0=A0 if (raft_is_enabled()) { > >> +=A0=A0=A0 if (replica_version_id >=3D version_id(2, 6, 0)) { > >=20 > > Why don't you use 'raft_was_used()` here together with version check? >=20 > It stops making sense. Its purpose was not to send anything while there c= an be > old versions. Assuming that in this case Raft was never enabled. After we > start checking subscriber's version, we can send Raft always when it is > supported. >=20 > > I've just understood we should also check for relay version in relay_pu= sh_raft(), shouldn't we? >=20 > Perhaps, yeah. >=20 > The patch is dropped from the patchset, and has forked into a new issue: > https://github.com/tarantool/tarantool/issues/5438 >=20 > Main reason is that the patch breaks the tests. The problem is that when > Raft is enabled and delivered to a subscriber, it bumps its local LSN. But > after that the subscriber may get XlogGapError from master, if some of > master logs were deleted. And the follower won't be able to auto-rejoin, = because > local LSN is > 0 already - Raft wrote something to WAL. >=20 > That breaks replication/replica_rejoin.test.lua. Not sure what to do with > that yet. Auto-rejoin in a cluster with election is not so vital I think,= so > we can work on it in the next release. And for now not to send anything > when Raft is off. It is bearable.