From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (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 8459F469719 for ; Tue, 17 Nov 2020 13:22:12 +0300 (MSK) References: From: Serge Petrenko Message-ID: Date: Tue, 17 Nov 2020 13:22:11 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH 09/12] raft: introduce raft_msg, drop xrow dependency List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org, gorcunov@gmail.com 17.11.2020 03:02, Vladislav Shpilevoy пишет: > Raft used to depend on xrow, because it used raft_request as a > communication and persistence unit. Xrow is a part of src/box > library set, so it blocked Raft extraction into src/lib/raft. > > This patch makes Raft not depend on xrow. For that Raft introduces > a new communication and persistence unit - struct raft_msg. > Interestingly, throughout its source code Raft already uses term > 'message' to describe requests, so this patch also restores the > consistency. This is because raft_request name was used to be > consistent with other *_request structs in xrow.h. Now Raft does > not depend on this, and can use its own name. > > Struct raft_msg repeats raft_request literally, but it actually > makes sense. Because when Raft is extracted to a new library, it > may start evolving independently. Its raft_msg may be populated > with new members, or their behaviour may change depending on how > the algorithm will evolve. > > But inside box it will be possible to tweak and extend raft_msg > whenever it is necessary, via struct raft_request, and without > changing the basic library. > > For instance, in future we may want to make nodes forward the > messages to each other during voting to speed the process up, and > for that we may want to add an explicit 'source' field to > raft_request, while it won't be necessary on the level of > raft_msg. > > There is a new compatibility layer in src/box/raft.h which hides > raft_msg details from other box code, and does the msg <-> request > conversions. > > Part of #5303 > --- > src/box/applier.cc | 2 +- > src/box/box.cc | 4 +-- > src/box/memtx_engine.c | 4 +-- > src/box/raft.c | 70 ++++++++++++++++++++++++++++++++++++++---- > src/box/raft.h | 24 +++++++++++++++ > src/box/raftlib.c | 24 ++++++--------- > src/box/raftlib.h | 38 ++++++++++++++++++----- > src/box/xrow.h | 4 +++ > 8 files changed, 137 insertions(+), 33 deletions(-) > > > diff --git a/src/box/raft.c b/src/box/raft.c > index 845525660..f3652bbcb 100644 > --- a/src/box/raft.c > +++ b/src/box/raft.c > @@ -49,6 +49,28 @@ struct raft box_raft_global = { > */ > static struct trigger box_raft_on_update; > > +static void > +box_raft_msg_to_request(const struct raft_msg *msg, struct raft_request *req) > +{ > + *req = (struct raft_request) { > + .term = msg->term, > + .vote = msg->vote, > + .state = msg->state, > + .vclock = msg->vclock, > + }; > +} > + > +static void > +box_raft_request_to_msg(const struct raft_request *req, struct raft_msg *msg) > +{ > + *msg = (struct raft_msg) { > + .term = req->term, > + .vote = req->vote, > + .state = req->state, > + .vclock = req->vclock, > + }; > +} > + > static int > box_raft_on_update_f(struct trigger *trigger, void *event) > { Have you considered making `struct raft_msg` a member of `struct raft_request`? This way you'll avoid copying. Yes, xrow will start depending on raftlib, but is this too bad? Never mind, it'll only work on raft_request -> raft_msg transition, but not vice versa. So, just an idea. > diff --git a/src/box/raft.h b/src/box/raft.h > index 09297273f..4dffce380 100644 > --- a/src/box/raft.h > +++ b/src/box/raft.h > @@ -35,6 +35,8 @@ > extern "C" { > #endif > > +struct raft_request; > + > /** Raft state of this instance. */ > static inline struct raft * > box_raft(void) > @@ -56,6 +58,28 @@ box_raft(void) > void > box_raft_reconsider_election_quorum(void); > > +/** > + * Recovery a single Raft request. Raft state machine is not turned on yet, this > + * works only during instance recovery from the journal. > + */ Typo: Recovery -> Recover > +void > +box_raft_recover(const struct raft_request *req); > + > +/** Save complete Raft state into a request to be persisted on disk locally. */ > +void > +box_raft_checkpoint_local(struct raft_request *req); > + > +/** > + * Save complete Raft state into a request to be sent to other instances of the > + * cluster. > + */ > +void > +box_raft_checkpoint_remote(struct raft_request *req); > + > +/** Handle a single Raft request from a node with instance id @a source. */ > +int > +box_raft_process(struct raft_request *req, uint32_t source); > + > void > box_raft_init(void); > -- Serge Petrenko