Re: [Tarantool-patches] [PATCH v2 14/16] raft: move RO summary update to box-Raft

[Serge Petrenko <sergepetrenko@tarantool.org>]

20.11.2020 02:46, Vladislav Shpilevoy пишет:
> RO summary update is supposed to make the instance read-only, when
> it becomes a follower, and read-write when becomes a leader.
>
> But it makes Raft depend on box, and prevents Raft move to a
> separate library.
>
> The patch moves the RO update to box-Raft.
>
> This became possible after some preparatory work was done to make
> Raft update triggers non-yielding, and invoked right after state
> change (without a yield between the change and the triggers).
>
> Part of #5303
LGTM
> ---
>   src/box/raft.c    | 8 ++++++++
>   src/box/raftlib.c | 8 --------
>   2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/src/box/raft.c b/src/box/raft.c
> index 5ccfb3449..c5a27cdec 100644
> --- a/src/box/raft.c
> +++ b/src/box/raft.c
> @@ -153,6 +153,14 @@ box_raft_on_update_f(struct trigger *trigger, void *event)
>   	(void)trigger;
>   	struct raft *raft = (struct raft *)event;
>   	assert(raft == box_raft());
> +	/*
> +	 * XXX: in case the instance became a leader, RO must be updated only
> +	 * after clearing the synchro queue.
> +	 *
> +	 * When the instance became a follower, then on the contrary - make it
> +	 * read-only ASAP, this is good.
> +	 */
> +	box_update_ro_summary();
>   	if (raft->state != RAFT_STATE_LEADER)
>   		return 0;
>   	/*
> diff --git a/src/box/raftlib.c b/src/box/raftlib.c
> index f64a66942..6e6bc658f 100644
> --- a/src/box/raftlib.c
> +++ b/src/box/raftlib.c
> @@ -33,7 +33,6 @@
>   #include "error.h"
>   #include "fiber.h"
>   #include "small/region.h"
> -#include "box.h"
>   #include "tt_static.h"
>   
>   /**
> @@ -618,8 +617,6 @@ raft_sm_become_leader(struct raft *raft)
>   	raft->state = RAFT_STATE_LEADER;
>   	raft->leader = raft->self;
>   	ev_timer_stop(loop(), &raft->timer);
> -	/* Make read-write (if other subsystems allow that. */
> -	box_update_ro_summary();
>   	/* State is visible and it is changed - broadcast. */
>   	raft_schedule_broadcast(raft);
>   }
> @@ -670,7 +667,6 @@ raft_sm_schedule_new_term(struct raft *raft, uint64_t new_term)
>   	raft->volatile_vote = 0;
>   	raft->leader = 0;
>   	raft->state = RAFT_STATE_FOLLOWER;
> -	box_update_ro_summary();
>   	raft_sm_pause_and_dump(raft);
>   	/*
>   	 * State is visible and it is changed - broadcast. Term is also visible,
> @@ -701,7 +697,6 @@ raft_sm_schedule_new_election(struct raft *raft)
>   	/* 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, raft->self);
> -	box_update_ro_summary();
>   }
>   
>   static void
> @@ -786,7 +781,6 @@ raft_sm_start(struct raft *raft)
>   		 */
>   		raft_sm_wait_leader_found(raft);
>   	}
> -	box_update_ro_summary();
>   	/*
>   	 * Nothing changed. But when raft was stopped, its state wasn't sent to
>   	 * replicas. At least this was happening at the moment of this being
> @@ -808,7 +802,6 @@ raft_sm_stop(struct raft *raft)
>   		raft->leader = 0;
>   	raft->state = RAFT_STATE_FOLLOWER;
>   	ev_timer_stop(loop(), &raft->timer);
> -	box_update_ro_summary();
>   	/* State is visible and changed - broadcast. */
>   	raft_schedule_broadcast(raft);
>   }
> @@ -894,7 +887,6 @@ raft_cfg_is_candidate(struct raft *raft, bool is_candidate)
>   			raft_schedule_broadcast(raft);
>   		}
>   	}
> -	box_update_ro_summary();
>   }
>   
>   void

-- 
Serge Petrenko