OSD: share map updates in the op_tp threads instead of the main dispatch thread

Sharing maps can require disk accesses and things. We don't want to do that
in our fast path, so do it in OSD::dequeue_op instead of OSD::handle_op. We're
cheating slightly and still doing it in handle_op if no op actually gets queued,
but we're going to put those into a separate work queue next. We'll also be
moving all the functions necessary for this into OSDService so that our completion
struct doesn't need to be a friend to OSD.
To make this easier, we're adding send_map_update and sent_epoch members to
OpRequest.

Signed-off-by: Greg Farnum <greg@inktank.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 3f1e23d53226b072f157c145d349bb43727fa3cf..23b94090cfff6e90b39c2b04272ae7efe8ce624e 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -7466,6 +7466,39 @@ void OSDService::handle_misdirected_op(PG *pg, OpRequestRef op)
   reply_op_error(op, -ENXIO);
 }
 
+struct send_map_on_destruct {
+  OSD *osd;
+  Message *m;
+  ConnectionRef con;
+  OSDMapRef osdmap;
+  epoch_t map_epoch;
+  bool should_send;
+  send_map_on_destruct(OSD *osd, Message *m,
+                       OSDMapRef& osdmap, epoch_t map_epoch) :
+                         osd(osd), m(m), con(m->get_connection()),
+                         osdmap(osdmap), map_epoch(map_epoch),
+                         should_send(true) {}
+  ~send_map_on_destruct() {
+    if (!should_send)
+      return;
+    OSD::Session *client_session = static_cast<OSD::Session *>(
+        con->get_priv());
+    if (client_session) {
+      client_session->sent_epoch_lock.Lock();
+    }
+    osd->_share_map_incoming(
+        m->get_source(),
+        con.get(),
+        map_epoch,
+        osdmap,
+        client_session);
+    if (client_session) {
+      client_session->sent_epoch_lock.Unlock();
+      client_session->put();
+    }
+  }
+};
+
 void OSD::handle_op(OpRequestRef op, OSDMapRef osdmap)
 {
   MOSDOp *m = static_cast<MOSDOp*>(op->get_req());
@@ -7493,22 +7526,8 @@ void OSD::handle_op(OpRequestRef op, OSDMapRef osdmap)
     return;
   }
 
-  // share our map with sender, if they're old
-  Session *client_session = static_cast<Session *>(m->get_connection()->get_priv());
-  if (client_session) {
-    client_session->sent_epoch_lock.Lock();
-  }
-  _share_map_incoming(
-      m->get_source(),
-      m->get_connection().get(),
-      m->get_map_epoch(),
-      osdmap,
-      client_session
-  );
-  if (client_session) {
-    client_session->sent_epoch_lock.Unlock();
-    client_session->put();
-  }
+  // set up a map send if the Op gets blocked for some reason
+  send_map_on_destruct share_map(this, m, osdmap, m->get_map_epoch());
 
   if (op->rmw_flags == 0) {
     int r = init_op_flags(op);
@@ -7599,8 +7618,12 @@ void OSD::handle_op(OpRequestRef op, OSDMapRef osdmap)
   }
 
   PG *pg = get_pg_or_queue_for_pg(pgid, op);
-  if (pg)
+  if (pg) {
+    op->send_map_update = true;
+    op->sent_epoch = m->get_map_epoch();
     enqueue_op(pg, op);
+    share_map.should_send = false;
+  }
 }
 
 template<typename T, int MSGTYPE>
@@ -7768,6 +7791,27 @@ void OSD::dequeue_op(
           << " latency " << latency
           << " " << *(op->get_req())
           << " pg " << *pg << dendl;
+
+  // share our map with sender, if they're old
+  if (op->send_map_update) {
+    Message *m = op->get_req();
+    Session *session = static_cast<Session *>(m->get_connection()->get_priv());
+    if (session) {
+      session->sent_epoch_lock.Lock();
+    }
+    _share_map_incoming(
+        m->get_source(),
+        m->get_connection().get(),
+        op->sent_epoch,
+        osdmap,
+        session
+    );
+    if (session) {
+      session->sent_epoch_lock.Unlock();
+      session->put();
+    }
+  }
+
   if (pg->deleting)
     return;
 

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index 808020bcadc69dcc332fccfdb6f1391041eb8bb8..aedd5f6f77fb68923858318847f6986c612f63d5 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1341,6 +1341,7 @@ private:
   epoch_t note_peer_epoch(int p, epoch_t e);
   void forget_peer_epoch(int p, epoch_t e);
 
+  friend struct send_map_on_destruct;
   bool _should_share_map(entity_name_t name, Connection *con, epoch_t epoch,
                          OSDMapRef& osdmap, Session *session);
   void _share_map_incoming(entity_name_t name, Connection *con, epoch_t epoch,

diff --git a/src/osd/OpRequest.cc b/src/osd/OpRequest.cc
index 33e7fbdb9d25f354fa408ae0f40cffa6d0bf3cf9..ef33d155c9ccb56cba1c5bc9421a4e64bab58d29 100644 (file)
--- a/src/osd/OpRequest.cc
+++ b/src/osd/OpRequest.cc
@@ -17,7 +17,8 @@
 OpRequest::OpRequest(Message *req, OpTracker *tracker) :
   TrackedOp(req, tracker),
   rmw_flags(0),
-  hit_flag_points(0), latest_flag_point(0) {
+  hit_flag_points(0), latest_flag_point(0),
+  send_map_update(false), sent_epoch(0) {
   if (req->get_priority() < tracker->cct->_conf->osd_client_op_priority) {
     // don't warn as quickly for low priority ops
     warn_interval_multiplier = tracker->cct->_conf->osd_recovery_op_warn_multiple;

diff --git a/src/osd/OpRequest.h b/src/osd/OpRequest.h
index 569b6fc581ea37041592ea5fc177a35b2a5f3040..dc451c112358a3eb73a9da9ecd56f1df7a79470a 100644 (file)
--- a/src/osd/OpRequest.h
+++ b/src/osd/OpRequest.h
@@ -89,6 +89,8 @@ private:
   OpRequest(Message *req, OpTracker *tracker);
 
 public:
+  bool send_map_update;
+  epoch_t sent_epoch;
   bool been_queued_for_pg() { return hit_flag_points & flag_queued_for_pg; }
   bool been_reached_pg() { return hit_flag_points & flag_reached_pg; }
   bool been_delayed() { return hit_flag_points & flag_delayed; }