From: Serge Petrenko <sergepetrenko@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, tarantool-patches@dev.tarantool.org, gorcunov@gmail.com Subject: Re: [Tarantool-patches] [PATCH 05/12] raft: stop using instance_id Date: Tue, 17 Nov 2020 11:59:03 +0300 [thread overview] Message-ID: <84b85640-b75a-c206-ca84-e4cae2bced64@tarantool.org> (raw) In-Reply-To: <b2b867ed8cdf9f2158d693c06babfc90e1d32f22.1605570907.git.v.shpilevoy@tarantool.org> 17.11.2020 03:02, Vladislav Shpilevoy пишет: > Raft is being moved to a separate library in src/lib. It means, > it can't depend on anything from box/. > > The patch makes raft stop using instance_id. > > Instead, it has a new option 'instance_id'. It is stored inside > struct raft as 'self', and should be configured using > raft_cfg_instance_id(). > > The configuration is done when bootstrap ends and the instance_id > is either recovered successfully, or the instance is anonymous. > > While working on this, I also considered introducing a new > function raft_boot() instead of raft_cfg_instance_id(). Which I > would also use to configure vclock later. Raft_boot() would be > meant to be called only one time with non-dynamic parameters > instance_id and vclock. > > But then I decided to keep adding new raft_cfg_*() functions. > Because: > > - It is more consistent with the existing options; > > - Does not require to think about too many different functions > like raft_create(), raft_boot(), raft_cfg_*() and in which order > to call them; > > Also I was thinking to introduce a single raft_cfg() like I did > in swim with swim_cfg(), to reduce number of raft_cfg_*() > functions, but decided it would be even worse with so many > options. > > Part of #5303 > --- LGTM. > src/box/box.cc | 10 +++++----- > src/box/raftlib.c | 32 ++++++++++++++++++++------------ > src/box/raftlib.h | 9 +++++++++ > 3 files changed, 34 insertions(+), 17 deletions(-) > > diff --git a/src/box/box.cc b/src/box/box.cc > index cc0d7b81d..8f5f3558e 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -2763,12 +2763,12 @@ box_cfg_xc(void) > * Fill in leader election parameters after bootstrap. Before it is not > * possible - there may be relevant data to recover from WAL and > * snapshot. Also until recovery is done, it is not possible to write > - * new records into WAL. It is also totally safe, because relaying is > - * not started until the box is configured. So it can't happen, that > - * this election-enabled node will try to relay to another > - * election-enabled node without election actually enabled leading to > - * disconnect. > + * new records into WAL. Another reason - before recovery is done, > + * instance_id is not known, so Raft simply can't work. > */ > + if (!replication_anon) > + raft_cfg_instance_id(box_raft(), instance_id); > + > if (box_set_election_timeout() != 0) > diag_raise(); > /* > diff --git a/src/box/raftlib.c b/src/box/raftlib.c > index 0657fa85a..ca1940ba6 100644 > --- a/src/box/raftlib.c > +++ b/src/box/raftlib.c > @@ -296,7 +296,7 @@ raft_process_msg(struct raft *raft, const struct raft_request *req, > say_info("RAFT: message %s from %u", raft_request_to_string(req), > source); > assert(source > 0); > - assert(source != instance_id); > + assert(source != raft->self); > if (req->term == 0 || req->state == 0) { > diag_set(ClientError, ER_PROTOCOL, "Raft term and state can't " > "be zero"); > @@ -337,7 +337,7 @@ raft_process_msg(struct raft *raft, const struct raft_request *req, > raft->leader); > break; > } > - if (req->vote == instance_id) { > + if (req->vote == raft->self) { > /* > * This is entirely valid. This instance could > * request a vote, then become a follower or > @@ -373,7 +373,7 @@ raft_process_msg(struct raft *raft, const struct raft_request *req, > break; > case RAFT_STATE_CANDIDATE: > /* Check if this is a vote for a competing candidate. */ > - if (req->vote != instance_id) { > + if (req->vote != raft->self) { > say_info("RAFT: vote request is skipped - " > "competing candidate"); > break; > @@ -382,7 +382,7 @@ raft_process_msg(struct raft *raft, const struct raft_request *req, > * Vote for self was requested earlier in this round, > * and now was answered by some other instance. > */ > - assert(raft->volatile_vote == instance_id); > + assert(raft->volatile_vote == raft->self); > bool was_set = bit_set(&raft->vote_mask, source); > raft->vote_count += !was_set; > if (raft->vote_count < raft->election_quorum) { > @@ -547,7 +547,7 @@ end_dump: > } else if (raft->leader != 0) { > /* There is a known leader. Wait until it is dead. */ > raft_sm_wait_leader_dead(raft); > - } else if (raft->vote == instance_id) { > + } else if (raft->vote == raft->self) { > /* Just wrote own vote. */ > if (raft->election_quorum == 1) > raft_sm_become_leader(raft); > @@ -561,7 +561,7 @@ end_dump: > raft_sm_wait_election_end(raft); > } else { > /* No leaders, no votes. */ > - raft_sm_schedule_new_vote(raft, instance_id); > + raft_sm_schedule_new_vote(raft, raft->self); > } > } else { > memset(&req, 0, sizeof(req)); > @@ -596,7 +596,7 @@ raft_worker_handle_broadcast(struct raft *raft) > req.vote = raft->vote; > req.state = raft->state; > if (req.state == RAFT_STATE_CANDIDATE) { > - assert(raft->vote == instance_id); > + assert(raft->vote == raft->self); > req.vclock = &replicaset.vclock; > } > replicaset_foreach(replica) > @@ -652,7 +652,7 @@ raft_sm_become_leader(struct raft *raft) > assert(raft->is_candidate); > assert(!raft->is_write_in_progress); > raft->state = RAFT_STATE_LEADER; > - raft->leader = instance_id; > + raft->leader = raft->self; > ev_timer_stop(loop(), &raft->timer); > /* Make read-write (if other subsystems allow that. */ > box_update_ro_summary(); > @@ -682,14 +682,14 @@ raft_sm_become_candidate(struct raft *raft) > say_info("RAFT: enter candidate state with 1 self vote"); > assert(raft->state == RAFT_STATE_FOLLOWER); > assert(raft->leader == 0); > - assert(raft->vote == instance_id); > + assert(raft->vote == raft->self); > assert(raft->is_candidate); > assert(!raft->is_write_in_progress); > assert(raft->election_quorum > 1); > raft->state = RAFT_STATE_CANDIDATE; > raft->vote_count = 1; > raft->vote_mask = 0; > - bit_set(&raft->vote_mask, instance_id); > + bit_set(&raft->vote_mask, raft->self); > raft_sm_wait_election_end(raft); > /* State is visible and it is changed - broadcast. */ > raft_schedule_broadcast(raft); > @@ -736,7 +736,7 @@ raft_sm_schedule_new_election(struct raft *raft) > assert(raft->is_candidate); > /* Everyone is a follower until its vote for self is persisted. */ > raft_sm_schedule_new_term(raft, raft->term + 1); > - raft_sm_schedule_new_vote(raft, instance_id); > + raft_sm_schedule_new_vote(raft, raft->self); > box_update_ro_summary(); > } > > @@ -783,7 +783,7 @@ raft_sm_wait_election_end(struct raft *raft) > assert(raft->is_candidate); > assert(raft->state == RAFT_STATE_FOLLOWER || > (raft->state == RAFT_STATE_CANDIDATE && > - raft->volatile_vote == instance_id)); > + raft->volatile_vote == raft->self)); > assert(raft->leader == 0); > double election_timeout = raft->election_timeout + > raft_new_random_election_shift(raft); > @@ -979,6 +979,14 @@ raft_cfg_death_timeout(struct raft *raft, double death_timeout) > } > } > > +void > +raft_cfg_instance_id(struct raft *raft, uint32_t instance_id) > +{ > + assert(raft->self == 0); > + assert(instance_id != 0); > + raft->self = instance_id; > +} > + > void > raft_new_term(struct raft *raft) > { > diff --git a/src/box/raftlib.h b/src/box/raftlib.h > index c9c13136e..f75ed2567 100644 > --- a/src/box/raftlib.h > +++ b/src/box/raftlib.h > @@ -95,6 +95,8 @@ const char * > raft_state_str(uint32_t state); > > struct raft { > + /** Instance ID of this node. */ > + uint32_t self; > /** Instance ID of leader of the current term. */ > uint32_t leader; > /** State of the instance. */ > @@ -241,6 +243,13 @@ raft_cfg_election_quorum(struct raft *raft, int election_quorum); > void > raft_cfg_death_timeout(struct raft *raft, double death_timeout); > > +/** > + * Configure ID of the given Raft instance. The ID can't be changed after it is > + * assigned first time. > + */ > +void > +raft_cfg_instance_id(struct raft *raft, uint32_t instance_id); > + > /** > * Bump the term. When it is persisted, the node checks if there is a leader, > * and if there is not, a new election is started. That said, this function can -- Serge Petrenko
next prev parent reply other threads:[~2020-11-17 8:59 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-17 0:02 [Tarantool-patches] [PATCH 00/12] Raft module, part 2 - relocation to src/lib/raft Vladislav Shpilevoy 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 01/12] raft: move sources to raftlib.h/.c Vladislav Shpilevoy 2020-11-17 8:14 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 10/12] raft: move box_update_ro_summary to update trigger Vladislav Shpilevoy 2020-11-17 12:42 ` Serge Petrenko 2020-11-17 15:17 ` Serge Petrenko 2020-11-18 23:21 ` Vladislav Shpilevoy 2020-11-19 10:08 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 11/12] raft: introduce RaftError Vladislav Shpilevoy 2020-11-17 15:13 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 12/12] raft: move algorithm code to src/lib/raft Vladislav Shpilevoy 2020-11-17 15:13 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 02/12] raft: move box_raft_* to src/box/raft.h and .c Vladislav Shpilevoy 2020-11-17 8:14 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 03/12] raft: stop using replication_disconnect_timeout() Vladislav Shpilevoy 2020-11-17 8:15 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 04/12] raft: stop using replication_synchro_quorum Vladislav Shpilevoy 2020-11-17 8:17 ` Serge Petrenko 2020-11-19 23:42 ` Vladislav Shpilevoy 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 05/12] raft: stop using instance_id Vladislav Shpilevoy 2020-11-17 8:59 ` Serge Petrenko [this message] 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 06/12] raft: make raft_request.vclock constant Vladislav Shpilevoy 2020-11-17 9:17 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 07/12] raft: stop using replicaset.vclock Vladislav Shpilevoy 2020-11-17 9:23 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 08/12] raft: introduce vtab for disk and network Vladislav Shpilevoy 2020-11-17 9:35 ` Serge Petrenko 2020-11-19 23:43 ` Vladislav Shpilevoy 2020-11-17 10:00 ` Serge Petrenko 2020-11-19 23:43 ` Vladislav Shpilevoy 2020-11-20 7:56 ` Serge Petrenko 2020-11-20 19:40 ` Vladislav Shpilevoy 2020-11-23 8:09 ` Serge Petrenko 2020-11-17 0:02 ` [Tarantool-patches] [PATCH 09/12] raft: introduce raft_msg, drop xrow dependency Vladislav Shpilevoy 2020-11-17 10:22 ` Serge Petrenko 2020-11-19 23:43 ` Vladislav Shpilevoy 2020-11-20 8:03 ` Serge Petrenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=84b85640-b75a-c206-ca84-e4cae2bced64@tarantool.org \ --to=sergepetrenko@tarantool.org \ --cc=gorcunov@gmail.com \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 05/12] raft: stop using instance_id' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox