[Tarantool-patches] [PATCH v2 03/10] wal: don't touch box.cfg.wal_dir more than once
Serge Petrenko
sergepetrenko at tarantool.org
Fri Sep 4 11:20:40 MSK 2020
04.09.2020 01:51, Vladislav Shpilevoy пишет:
> Relay.cc and box.cc obtained box.cfg.wal_dir value using
> cfg_gets() call. To initialize WAL and create struct recovery
> objects.
>
> That is not only a bit dangerous (cfg_gets() uses Lua API and can
> throw a Lua error) and slow, but also not necessary - wal_dir
> parameter is constant, it can't be changed after instance start.
>
> It means, the value can be stored somewhere one time and then used
> without Lua.
>
> Main motivation is that the WAL directory path will be needed
> inside relay threads to restart their recovery iterators in the
> Raft patch. They can't use cfg_gets(), because Lua lives in TX
> thread. But can access a constant global variable, introduced in
> this patch (it existed before, but now has a method to get it).
>
> Needed for #1146
LGTM
> ---
> src/box/box.cc | 9 ++++-----
> src/box/relay.cc | 7 ++-----
> src/box/wal.c | 6 ++++++
> src/box/wal.h | 7 +++++++
> 4 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 0813603c0..eeb00d5e2 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -2395,8 +2395,7 @@ local_recovery(const struct tt_uuid *instance_uuid,
> wal_stream_create(&wal_stream);
>
> struct recovery *recovery;
> - recovery = recovery_new(cfg_gets("wal_dir"),
> - cfg_geti("force_recovery"),
> + recovery = recovery_new(wal_dir(), cfg_geti("force_recovery"),
> checkpoint_vclock);
>
> /*
> @@ -2469,7 +2468,7 @@ local_recovery(const struct tt_uuid *instance_uuid,
> recovery_follow_local(recovery, &wal_stream.base, "hot_standby",
> cfg_getd("wal_dir_rescan_delay"));
> while (true) {
> - if (path_lock(cfg_gets("wal_dir"), &wal_dir_lock))
> + if (path_lock(wal_dir(), &wal_dir_lock))
> diag_raise();
> if (wal_dir_lock >= 0)
> break;
> @@ -2616,7 +2615,7 @@ box_cfg_xc(void)
> * Lock the write ahead log directory to avoid multiple
> * instances running in the same dir.
> */
> - if (path_lock(cfg_gets("wal_dir"), &wal_dir_lock) < 0)
> + if (path_lock(wal_dir(), &wal_dir_lock) < 0)
> diag_raise();
> if (wal_dir_lock < 0) {
> /**
> @@ -2625,7 +2624,7 @@ box_cfg_xc(void)
> * WAL dir must contain at least one xlog.
> */
> if (!cfg_geti("hot_standby") || checkpoint == NULL)
> - tnt_raise(ClientError, ER_ALREADY_RUNNING, cfg_gets("wal_dir"));
> + tnt_raise(ClientError, ER_ALREADY_RUNNING, wal_dir());
> }
>
> struct journal bootstrap_journal;
> diff --git a/src/box/relay.cc b/src/box/relay.cc
> index a7843a8c2..124b0f52f 100644
> --- a/src/box/relay.cc
> +++ b/src/box/relay.cc
> @@ -34,7 +34,6 @@
> #include "tt_static.h"
> #include "scoped_guard.h"
> #include "cbus.h"
> -#include "cfg.h"
> #include "errinj.h"
> #include "fiber.h"
> #include "say.h"
> @@ -369,8 +368,7 @@ relay_final_join(int fd, uint64_t sync, struct vclock *start_vclock,
> relay_delete(relay);
> });
>
> - relay->r = recovery_new(cfg_gets("wal_dir"), false,
> - start_vclock);
> + relay->r = recovery_new(wal_dir(), false, start_vclock);
> vclock_copy(&relay->stop_vclock, stop_vclock);
>
> int rc = cord_costart(&relay->cord, "final_join",
> @@ -731,8 +729,7 @@ relay_subscribe(struct replica *replica, int fd, uint64_t sync,
> });
>
> vclock_copy(&relay->local_vclock_at_subscribe, &replicaset.vclock);
> - relay->r = recovery_new(cfg_gets("wal_dir"), false,
> - replica_clock);
> + relay->r = recovery_new(wal_dir(), false, replica_clock);
> vclock_copy(&relay->tx.vclock, replica_clock);
> relay->version_id = replica_version_id;
>
> diff --git a/src/box/wal.c b/src/box/wal.c
> index 045006b60..e181e58d9 100644
> --- a/src/box/wal.c
> +++ b/src/box/wal.c
> @@ -201,6 +201,12 @@ wal_mode(void)
> return wal_writer_singleton.wal_mode;
> }
>
> +const char *
> +wal_dir(void)
> +{
> + return wal_writer_singleton.wal_dir.dirname;
> +}
> +
> static void
> wal_write_to_disk(struct cmsg *msg);
>
> diff --git a/src/box/wal.h b/src/box/wal.h
> index 9d0cada46..581306fe9 100644
> --- a/src/box/wal.h
> +++ b/src/box/wal.h
> @@ -98,6 +98,13 @@ wal_enable(void);
> void
> wal_free(void);
>
> +/**
> + * Get WAL directory path. The value never changes after box is
> + * configured first time. Safe to use from multiple threads.
> + */
> +const char *
> +wal_dir(void);
> +
> struct wal_watcher_msg {
> struct cmsg cmsg;
> struct wal_watcher *watcher;
--
Serge Petrenko
More information about the Tarantool-patches
mailing list