mgr/DaemonState: add per DaemonState lock

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc
index 60fb2cc2095b4dadc46b4053fdfa507c8b3959f0..c8b720911a80cb7417aedf5bde468162b239462e 100644 (file)
--- 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<DaemonState>(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 2c7f52f30fa75cae056d694c819c49661fa4d6c9..93fe130190eb6d72b52eb9d59b7a131f954fbf36 100644 (file)
--- 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 e75b968425e977c0532686b500846eb592ea7cab..98853a2ec2cb628985b028891c7400997c479467 100644 (file)
--- 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 09082505e0fe02621445ab07bcb1dc0ae986c63c..092b71fdb9fac8a872beac43d87167d996fed989 100644 (file)
--- 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 096462e7d801399eb72d75b63de59bf11caa83d5..74628dbbf1c7b8f71fbf86c5920b7f9fb71f1d0f 100644 (file)
--- 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());