[Tarantool-patches] [PATCH v2 02/10] box: introduce summary RO flag
Serge Petrenko
sergepetrenko at tarantool.org
Fri Sep 4 11:17:31 MSK 2020
04.09.2020 01:51, Vladislav Shpilevoy пишет:
> An instance is writable if box.cfg.read_only is false, and it is
> not orphan. Update of the final read-only state of the instance
> needs to fire read-only update triggers, and notify the engines.
> These 2 flags were easy and cheap to check on each operation, and
> the triggers were easy to use since both flags are stored and
> updated inside box.cc.
>
> That is going to change when Raft is introduced. Raft will add 2
> more checks:
>
> - A flag if Raft is enabled on the node. If it is not, then Raft
> state won't affect whether the instance is writable;
>
> - When Raft is enabled, it will allow writes on a leader only.
>
> It means a check for being read-only would look like this:
>
> is_ro || is_orphan || (raft_is_enabled() && !raft_is_leader())
>
> This is significantly slower. Besides, Raft somehow needs to
> access the read-only triggers and engine API - this looks wrong.
>
> The patch introduces a new flag is_ro_summary. The flag
> incorporates all the read-only conditions into one flag. When some
> subsystem may change read-only state of the instance, it needs to
> call box_update_ro_summary(), and the function takes care of
> updating the summary flag, running the triggers, and notifying the
> engines.
>
> Raft will use this function when its state or config will change.
>
> Needed for #1146
Thanks for the patch! LGTM.
> ---
> src/box/box.cc | 44 +++++++++++++++++++++++++++-----------------
> src/box/box.h | 6 ++++++
> 2 files changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/src/box/box.cc b/src/box/box.cc
> index faffd5769..0813603c0 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -129,6 +129,14 @@ static bool is_local_recovery = false;
> */
> static bool is_orphan;
>
> +/**
> + * Summary flag incorporating all the instance attributes,
> + * affecting ability to write. Currently these are:
> + * - is_ro;
> + * - is_orphan;
> + */
> +static bool is_ro_summary = true;
> +
> /**
> * The pool of fibers in the transaction processor thread
> * working on incoming messages from net, wal and other
> @@ -144,11 +152,24 @@ static struct fiber_pool tx_fiber_pool;
> */
> static struct cbus_endpoint tx_prio_endpoint;
>
> +void
> +box_update_ro_summary(void)
> +{
> + bool old_is_ro_summary = is_ro_summary;
> + is_ro_summary = is_ro || is_orphan;
> + /* In 99% nothing changes. Filter this out first. */
> + if (is_ro_summary == old_is_ro_summary)
> + return;
> +
> + if (is_ro_summary)
> + engine_switch_to_ro();
> + fiber_cond_broadcast(&ro_cond);
> +}
> +
> static int
> box_check_writable(void)
> {
> - /* box is only writable if box.cfg.read_only == false and */
> - if (is_ro || is_orphan) {
> + if (is_ro_summary) {
> diag_set(ClientError, ER_READONLY);
> diag_log();
> return -1;
> @@ -253,20 +274,14 @@ box_check_ro(void);
> void
> box_set_ro(void)
> {
> - bool ro = box_check_ro();
> - if (ro == is_ro)
> - return; /* nothing to do */
> - if (ro)
> - engine_switch_to_ro();
> -
> - is_ro = ro;
> - fiber_cond_broadcast(&ro_cond);
> + is_ro = box_check_ro();
> + box_update_ro_summary();
> }
>
> bool
> box_is_ro(void)
> {
> - return is_ro || is_orphan;
> + return is_ro_summary;
> }
>
> bool
> @@ -293,13 +308,8 @@ box_wait_ro(bool ro, double timeout)
> void
> box_do_set_orphan(bool orphan)
> {
> - if (is_orphan == orphan)
> - return; /* nothing to do */
> - if (orphan)
> - engine_switch_to_ro();
> -
> is_orphan = orphan;
> - fiber_cond_broadcast(&ro_cond);
> + box_update_ro_summary();
> }
>
> void
> diff --git a/src/box/box.h b/src/box/box.h
> index f9bd8b98d..5988264a5 100644
> --- a/src/box/box.h
> +++ b/src/box/box.h
> @@ -137,6 +137,12 @@ box_set_orphan(bool orphan);
> void
> box_do_set_orphan(bool orphan);
>
> +/**
> + * Update the final RO flag based on the instance flags and state.
> + */
> +void
> +box_update_ro_summary(void);
> +
> /**
> * Iterate over all spaces and save them to the
> * snapshot file.
--
Serge Petrenko
More information about the Tarantool-patches
mailing list