From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 9FADE469719 for ; Thu, 22 Oct 2020 00:41:54 +0300 (MSK) References: <15c343a9d2140a6c7ba31cb8761b2bca0ca6e09a.1602954690.git.v.shpilevoy@tarantool.org> <47356d83-4464-070b-0df8-c7fcbbdb1e02@tarantool.org> From: Vladislav Shpilevoy Message-ID: <1905f9a9-cddb-c016-0444-b665b5cbc948@tarantool.org> Date: Wed, 21 Oct 2020 23:41:52 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit 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: Serge Petrenko , tarantool-patches@dev.tarantool.org Hi! Thanks for the review! >> 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 >>   @@ -2154,13 +2155,14 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header) >>            tt_uuid_str(&replica_uuid), sio_socketname(io->fd)); >>       say_info("remote vclock %s local vclock %s", >>            vclock_to_string(&replica_clock), vclock_to_string(&vclock)); >> -    if (raft_is_enabled()) { >> +    if (replica_version_id >= version_id(2, 6, 0)) { > > Why don't you use 'raft_was_used()` here together with version check? It stops making sense. Its purpose was not to send anything while there can 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. > I've just understood we should also check for relay version in relay_push_raft(), shouldn't we? Perhaps, yeah. The patch is dropped from the patchset, and has forked into a new issue: https://github.com/tarantool/tarantool/issues/5438 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. 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.