From f236b5e78338f85a0ac82440ae44cdf4db83a04f Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 19 Jul 2017 17:28:48 -0400 Subject: [PATCH] mgr/DaemonState: add per DaemonState lock Signed-off-by: Sage Weil --- src/mgr/DaemonServer.cc | 14 ++++++++++---- src/mgr/DaemonState.cc | 3 ++- src/mgr/DaemonState.h | 2 ++ src/mgr/Mgr.cc | 4 +++- src/mgr/PyModules.cc | 8 ++++++-- 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc index 60fb2cc2095..c8b720911a8 100644 --- a/src/mgr/DaemonServer.cc +++ b/src/mgr/DaemonServer.cc @@ -312,16 +312,18 @@ bool DaemonServer::handle_open(MMgrOpen *m) configure->stats_period = g_conf->mgr_stats_period; m->get_connection()->send_message(configure); + DaemonStatePtr daemon; if (daemon_state.exists(key)) { + daemon = daemon_state.get(key); + } + if (daemon) { dout(20) << "updating existing DaemonState for " << m->daemon_name << dendl; + Mutex::Locker l(daemon->lock); daemon_state.get(key)->perf_counters.clear(); } if (m->service_daemon) { - DaemonStatePtr daemon; - if (daemon_state.exists(key)) { - daemon = daemon_state.get(key); - } else { + if (!daemon) { dout(4) << "constructing new DaemonState for " << key << dendl; daemon = std::make_shared(daemon_state.types); daemon->key = key; @@ -330,6 +332,7 @@ bool DaemonServer::handle_open(MMgrOpen *m) } daemon_state.insert(daemon); } + Mutex::Locker l(daemon->lock); daemon->service_daemon = true; daemon->metadata = m->daemon_metadata; daemon->service_status = m->daemon_status; @@ -389,6 +392,7 @@ bool DaemonServer::handle_report(MMgrReport *m) // always contains metadata. } assert(daemon != nullptr); + Mutex::Locker l(daemon->lock); auto &daemon_counters = daemon->perf_counters; daemon_counters.update(m); // if there are any schema updates, notify the python modules @@ -674,6 +678,7 @@ bool DaemonServer::handle_command(MCommand *m) DaemonKey key(p.first, q.first); assert(daemon_state.exists(key)); auto daemon = daemon_state.get(key); + Mutex::Locker l(daemon->lock); f->dump_stream("status_stamp") << daemon->service_status_stamp; f->dump_stream("last_beacon") << daemon->last_service_beacon; f->open_object_section("status"); @@ -972,6 +977,7 @@ void DaemonServer::_prune_pending_service_map() continue; } auto daemon = daemon_state.get(key); + Mutex::Locker l(daemon->lock); if (daemon->last_service_beacon == utime_t()) { // we must have just restarted; assume they are alive now. daemon->last_service_beacon = ceph_clock_now(); diff --git a/src/mgr/DaemonState.cc b/src/mgr/DaemonState.cc index 2c7f52f30fa..93fe130190e 100644 --- a/src/mgr/DaemonState.cc +++ b/src/mgr/DaemonState.cc @@ -62,7 +62,8 @@ DaemonStateCollection DaemonStateIndex::get_by_service( return result; } -DaemonStateCollection DaemonStateIndex::get_by_server(const std::string &hostname) const +DaemonStateCollection DaemonStateIndex::get_by_server( + const std::string &hostname) const { Mutex::Locker l(lock); diff --git a/src/mgr/DaemonState.h b/src/mgr/DaemonState.h index e75b968425e..98853a2ec2c 100644 --- a/src/mgr/DaemonState.h +++ b/src/mgr/DaemonState.h @@ -93,6 +93,8 @@ class DaemonPerfCounters class DaemonState { public: + Mutex lock = {"DaemonState::lock"}; + DaemonKey key; // The hostname where daemon was last seen running (extracted diff --git a/src/mgr/Mgr.cc b/src/mgr/Mgr.cc index 09082505e0f..092b71fdb9f 100644 --- a/src/mgr/Mgr.cc +++ b/src/mgr/Mgr.cc @@ -118,7 +118,7 @@ public: DaemonStatePtr state; if (daemon_state.exists(key)) { state = daemon_state.get(key); - // TODO lock state + Mutex::Locker l(state->lock); daemon_meta.erase("name"); daemon_meta.erase("hostname"); state->metadata.clear(); @@ -415,6 +415,7 @@ void Mgr::handle_osd_map() if (daemon_state.exists(k)) { auto metadata = daemon_state.get(k); + Mutex::Locker l(metadata->lock); auto addr_iter = metadata->metadata.find("front_addr"); if (addr_iter != metadata->metadata.end()) { const std::string &metadata_addr = addr_iter->second; @@ -552,6 +553,7 @@ void Mgr::handle_fs_map(MFSMap* m) bool update = false; if (daemon_state.exists(k)) { auto metadata = daemon_state.get(k); + Mutex::Locker l(metadata->lock); if (metadata->metadata.empty() || metadata->metadata.count("addr") == 0) { update = true; diff --git a/src/mgr/PyModules.cc b/src/mgr/PyModules.cc index 096462e7d80..74628dbbf1c 100644 --- a/src/mgr/PyModules.cc +++ b/src/mgr/PyModules.cc @@ -57,6 +57,7 @@ void PyModules::dump_server(const std::string &hostname, std::string ceph_version; for (const auto &i : dmc) { + Mutex::Locker l(i.second->lock); const auto &key = i.first; const std::string &str_type = key.first; const std::string &svc_name = key.second; @@ -122,6 +123,7 @@ PyObject *PyModules::get_metadata_python( const std::string &svc_id) { auto metadata = daemon_state.get(DaemonKey(svc_name, svc_id)); + Mutex::Locker l(metadata->lock); PyFormatter f; f.dump_string("hostname", metadata->hostname); for (const auto &i : metadata->metadata) { @@ -137,6 +139,7 @@ PyObject *PyModules::get_daemon_status_python( const std::string &svc_id) { auto metadata = daemon_state.get(DaemonKey(svc_name, svc_id)); + Mutex::Locker l(metadata->lock); PyFormatter f; for (const auto &i : metadata->service_status) { f.dump_string(i.first.c_str(), i.second); @@ -199,6 +202,7 @@ PyObject *PyModules::get_python(const std::string &what) PyFormatter f; auto dmc = daemon_state.get_by_service("osd"); for (const auto &i : dmc) { + Mutex::Locker l(i.second->lock); f.open_object_section(i.first.second.c_str()); f.dump_string("hostname", i.second->hostname); for (const auto &j : i.second->metadata) { @@ -669,8 +673,7 @@ PyObject* PyModules::get_counter_python( auto metadata = daemon_state.get(DaemonKey(svc_name, svc_id)); - // FIXME: this is unsafe, I need to either be inside DaemonStateIndex's - // lock or put a lock on individual DaemonStates + Mutex::Locker l2(metadata->lock); if (metadata) { if (metadata->perf_counters.instances.count(path)) { auto counter_instance = metadata->perf_counters.instances.at(path); @@ -731,6 +734,7 @@ PyObject* PyModules::get_perf_schema_python( std::ostringstream daemon_name; auto key = statepair.first; auto state = statepair.second; + Mutex::Locker l(state->lock); daemon_name << key.first << "." << key.second; f.open_object_section(daemon_name.str().c_str()); -- 2.39.5