From 74fd334a6ebe023c1c4b6d80d298857e2d4fe7bf Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 27 Jul 2018 20:30:13 +0800 Subject: [PATCH] crimson/common: write configs synchronously on shard.0 to avoid potential racings on the same shard. before this change, we apply the change in async. after this change, all changes happens on the owner shard (i.e. shard.0), and the changes are applied synchronously. Signed-off-by: Kefu Chai --- src/crimson/common/config_proxy.h | 35 +++++++++++++++++++------------ src/test/crimson/test_config.cc | 14 ++++++------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/crimson/common/config_proxy.h b/src/crimson/common/config_proxy.h index 32b2828af26..807ca958d5e 100644 --- a/src/crimson/common/config_proxy.h +++ b/src/crimson/common/config_proxy.h @@ -38,21 +38,30 @@ class ConfigProxy : public seastar::peering_sharded_service // @param func a functor which accepts @c "ConfigValues&" template seastar::future<> do_change(Func&& func) { - auto new_values = seastar::make_lw_shared(*values); - func(*new_values); - return seastar::do_with(seastar::make_foreign(new_values), - [this](auto& foreign_values) { - return container().invoke_on_all([&foreign_values](ConfigProxy& proxy) { - return foreign_values.copy().then([&proxy](auto foreign_values) { - proxy.values.reset(); - proxy.values = std::move(foreign_values); - proxy.obs_mgr.apply_changes(proxy.values->changed, - proxy, nullptr); + return container().invoke_on(values.get_owner_shard(), + [func = std::move(func)](ConfigProxy& owner) { + // apply the changes to a copy of the values + auto new_values = seastar::make_lw_shared(*owner.values); + new_values->changed.clear(); + func(*new_values); + + // always apply the new settings synchronously on the owner shard, to + // avoid racings with other do_change() calls in parallel. + owner.values.reset(new_values); + + return seastar::parallel_for_each(boost::irange(1u, seastar::smp::count), + [&owner, new_values] (auto cpu) { + return owner.container().invoke_on(cpu, + [foreign_values = seastar::make_foreign(new_values)](ConfigProxy& proxy) mutable { + proxy.values.reset(); + proxy.values = std::move(foreign_values); + proxy.obs_mgr.apply_changes(proxy.values->changed, + proxy, nullptr); + }); + }).finally([new_values] { + new_values->changed.clear(); }); }); - }).then([this] { - values->changed.clear(); - }); } public: ConfigProxy(); diff --git a/src/test/crimson/test_config.cc b/src/test/crimson/test_config.cc index eaf9261c30a..5d0c104b8fc 100644 --- a/src/test/crimson/test_config.cc +++ b/src/test/crimson/test_config.cc @@ -11,17 +11,15 @@ static seastar::future<> test_config() return ceph::common::sharded_conf().start().then([] { return ceph::common::sharded_conf().invoke_on(0, &Config::start); }).then([] { - return ceph::common::sharded_conf().invoke_on_all([](auto& config) { + return ceph::common::sharded_conf().invoke_on_all([](Config& config) { return config.set_val("osd_tracing", "true"); }); }).then([] { - return ceph::common::local_conf().get_val("osd_tracing"); - }).then([](bool osd_tracing) { - if (osd_tracing) { - return seastar::make_ready_future<>(); - } else { - throw std::runtime_error("run osd_tracing"); - } + return ceph::common::sharded_conf().invoke_on_all([](Config& config) { + if (!config.get_val("osd_tracing")) { + throw std::runtime_error("run osd_tracing"); + } + }); }).then([] { return ceph::common::sharded_conf().stop(); }); -- 2.39.5