Re: [Tarantool-patches] [PATCH 02/12] raft: move box_raft_* to src/box/raft.h and .c

[Serge Petrenko <sergepetrenko@tarantool.org>]

LGTM

17.11.2020 03:02, Vladislav Shpilevoy пишет:
> The commit moves Raft functions and objects specific for box to
> src/box/raft from src/box/box and src/box/raftlib.
>
> The goal is to gradually eliminate all box dependencies from
> src/box/raftlib and move it to src/lib/raft.
>
> Part of #5303
> ---
>   src/box/box.cc    | 25 --------------
>   src/box/raft.c    | 86 +++++++++++++++++++++++++++++++++++++++++++++++
>   src/box/raft.h    | 59 ++++++++++++++++++++++++++++++++
>   src/box/raftlib.c | 29 ----------------
>   src/box/raftlib.h | 19 -----------
>   5 files changed, 145 insertions(+), 73 deletions(-)
>   create mode 100644 src/box/raft.c
>   create mode 100644 src/box/raft.h
>
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 1f7dec362..8dd92a5f5 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -152,11 +152,6 @@ static struct fiber_pool tx_fiber_pool;
>    * are too many messages in flight (gh-1892).
>    */
>   static struct cbus_endpoint tx_prio_endpoint;
> -/**
> - * A trigger executed each time the Raft state machine updates any
> - * of its visible attributes.
> - */
> -static struct trigger box_raft_on_update;
>   
>   void
>   box_update_ro_summary(void)
> @@ -1060,23 +1055,6 @@ box_clear_synchro_queue(bool try_wait)
>   	}
>   }
>   
> -static int
> -box_raft_on_update_f(struct trigger *trigger, void *event)
> -{
> -	(void)trigger;
> -	struct raft *raft = (struct raft *)event;
> -	assert(raft == box_raft());
> -	if (raft->state != RAFT_STATE_LEADER)
> -		return 0;
> -	/*
> -	 * When the node became a leader, it means it will ignore all records
> -	 * from all the other nodes, and won't get late CONFIRM messages anyway.
> -	 * Can clear the queue without waiting for confirmations.
> -	 */
> -	box_clear_synchro_queue(false);
> -	return 0;
> -}
> -
>   void
>   box_listen(void)
>   {
> @@ -2658,9 +2636,6 @@ box_init(void)
>   	txn_limbo_init();
>   	sequence_init();
>   	box_raft_init();
> -
> -	trigger_create(&box_raft_on_update, box_raft_on_update_f, NULL, NULL);
> -	raft_on_update(box_raft(), &box_raft_on_update);
>   }
>   
>   bool
> diff --git a/src/box/raft.c b/src/box/raft.c
> new file mode 100644
> index 000000000..f289a6993
> --- /dev/null
> +++ b/src/box/raft.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright 2010-2020, Tarantool AUTHORS, please see AUTHORS file.
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above
> + *    copyright notice, this list of conditions and the
> + *    following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above
> + *    copyright notice, this list of conditions and the following
> + *    disclaimer in the documentation and/or other materials
> + *    provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +#include "box.h"
> +#include "raft.h"
> +
> +struct raft box_raft_global = {
> +	/*
> +	 * Set an invalid state to validate in runtime the global raft node is
> +	 * not used before initialization.
> +	 */
> +	.state = 0,
> +};
> +
> +/**
> + * A trigger executed each time the Raft state machine updates any
> + * of its visible attributes.
> + */
> +static struct trigger box_raft_on_update;
> +
> +static int
> +box_raft_on_update_f(struct trigger *trigger, void *event)
> +{
> +	(void)trigger;
> +	struct raft *raft = (struct raft *)event;
> +	assert(raft == box_raft());
> +	if (raft->state != RAFT_STATE_LEADER)
> +		return 0;
> +	/*
> +	 * When the node became a leader, it means it will ignore all records
> +	 * from all the other nodes, and won't get late CONFIRM messages anyway.
> +	 * Can clear the queue without waiting for confirmations.
> +	 */
> +	box_clear_synchro_queue(false);
> +	return 0;
> +}
> +
> +void
> +box_raft_init(void)
> +{
> +	raft_create(&box_raft_global);
> +	trigger_create(&box_raft_on_update, box_raft_on_update_f, NULL, NULL);
> +	raft_on_update(box_raft(), &box_raft_on_update);
> +}
> +
> +void
> +box_raft_free(void)
> +{
> +	/*
> +	 * Can't join the fiber, because the event loop is stopped already, and
> +	 * yields are not allowed.
> +	 */
> +	box_raft_global.worker = NULL;
> +	raft_destroy(&box_raft_global);
> +	/*
> +	 * Invalidate so as box_raft() would fail if any usage attempt happens.
> +	 */
> +	box_raft_global.state = 0;
> +}
> diff --git a/src/box/raft.h b/src/box/raft.h
> new file mode 100644
> index 000000000..fe0f073dc
> --- /dev/null
> +++ b/src/box/raft.h
> @@ -0,0 +1,59 @@
> +#pragma once
> +/*
> + * Copyright 2010-2020, Tarantool AUTHORS, please see AUTHORS file.
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above
> + *    copyright notice, this list of conditions and the
> + *    following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above
> + *    copyright notice, this list of conditions and the following
> + *    disclaimer in the documentation and/or other materials
> + *    provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +#include "raftlib.h"
> +
> +#if defined(__cplusplus)
> +extern "C" {
> +#endif
> +
> +/** Raft state of this instance. */
> +static inline struct raft *
> +box_raft(void)
> +{
> +	extern struct raft box_raft_global;
> +	/**
> +	 * Ensure the raft node can be used. I.e. that it is properly
> +	 * initialized. Entirely for debug purposes.
> +	 */
> +	assert(box_raft_global.state != 0);
> +	return &box_raft_global;
> +}
> +
> +void
> +box_raft_init(void);
> +
> +void
> +box_raft_free(void);
> +
> +#if defined(__cplusplus)
> +}
> +#endif
> diff --git a/src/box/raftlib.c b/src/box/raftlib.c
> index ff664a4d1..3867c63e0 100644
> --- a/src/box/raftlib.c
> +++ b/src/box/raftlib.c
> @@ -44,14 +44,6 @@
>    */
>   #define RAFT_RANDOM_ELECTION_FACTOR 0.1
>   
> -struct raft box_raft_global = {
> -	/*
> -	 * Set an invalid state to validate in runtime the global raft node is
> -	 * not used before initialization.
> -	 */
> -	.state = 0,
> -};
> -
>   /**
>    * When decoding we should never trust that there is
>    * a valid data incomes.
> @@ -1106,24 +1098,3 @@ raft_destroy(struct raft *raft)
>   		raft->worker = NULL;
>   	}
>   }
> -
> -void
> -box_raft_init(void)
> -{
> -	raft_create(&box_raft_global);
> -}
> -
> -void
> -box_raft_free(void)
> -{
> -	/*
> -	 * Can't join the fiber, because the event loop is stopped already, and
> -	 * yields are not allowed.
> -	 */
> -	box_raft_global.worker = NULL;
> -	raft_destroy(&box_raft_global);
> -	/*
> -	 * Invalidate so as box_raft() would fail if any usage attempt happens.
> -	 */
> -	box_raft_global.state = 0;
> -}
> diff --git a/src/box/raftlib.h b/src/box/raftlib.h
> index 3088fba23..805f69d64 100644
> --- a/src/box/raftlib.h
> +++ b/src/box/raftlib.h
> @@ -271,25 +271,6 @@ raft_create(struct raft *raft);
>   void
>   raft_destroy(struct raft *raft);
>   
> -/** Raft state of this instance. */
> -static inline struct raft *
> -box_raft(void)
> -{
> -	extern struct raft box_raft_global;
> -	/**
> -	 * Ensure the raft node can be used. I.e. that it is properly
> -	 * initialized. Entirely for debug purposes.
> -	 */
> -	assert(box_raft_global.state != 0);
> -	return &box_raft_global;
> -}
> -
> -void
> -box_raft_init(void);
> -
> -void
> -box_raft_free(void);
> -
>   #if defined(__cplusplus)
>   }
>   #endif

-- 
Serge Petrenko