From d328a28cc67e5bebc0496204d6195e1abf2e962a Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@inktank.com>
Date: Fri, 31 Aug 2012 18:23:20 -0700
Subject: [PATCH] mon: send 'null' reply to requests we won't reply to

This is a no-op if the client was talking to us, but in the forwarded
request case will clean up the request state (and request message) on the
forwarding monitor.  Otherwise, MOSDFailure messages (and probably others)
can accumulate on the non-leader mon indefinitely.

Signed-off-by: Sage Weil <sage@inktank.com>
---
 src/include/ceph_features.h |  4 +++-
 src/messages/MRoute.h       |  8 +++++---
 src/mon/Monitor.cc          | 40 ++++++++++++++++++++++++++++++-------
 src/mon/Monitor.h           |  1 +
 4 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/src/include/ceph_features.h b/src/include/ceph_features.h
index d00ae7c938b01..ad2ef4ecec56c 100644
--- a/src/include/ceph_features.h
+++ b/src/include/ceph_features.h
@@ -24,6 +24,7 @@
 #define CEPH_FEATURE_INDEP_PG_MAP   (1<<17)
 #define CEPH_FEATURE_CRUSH_TUNABLES (1<<18)
 #define CEPH_FEATURE_CHUNKY_SCRUB   (1<<19)
+#define CEPH_FEATURE_MON_NULLROUTE  (1<<20)
 
 /*
  * Features supported.  Should be everything above.
@@ -48,7 +49,8 @@
 	 CEPH_FEATURE_MONENC |		 \
 	 CEPH_FEATURE_INDEP_PG_MAP |	 \
 	 CEPH_FEATURE_CRUSH_TUNABLES |	 \
-	 CEPH_FEATURE_CHUNKY_SCRUB)
+	 CEPH_FEATURE_CHUNKY_SCRUB |	 \
+	 CEPH_FEATURE_MON_NULLROUTE)
 
 #define CEPH_FEATURES_SUPPORTED_DEFAULT  CEPH_FEATURES_ALL
 
diff --git a/src/messages/MRoute.h b/src/messages/MRoute.h
index 7efa13683adda..cc1b3bdf901f4 100644
--- a/src/messages/MRoute.h
+++ b/src/messages/MRoute.h
@@ -43,12 +43,14 @@ public:
     bufferlist::iterator p = payload.begin();
     ::decode(session_mon_tid, p);
     ::decode(dest, p);
-    msg = decode_message(NULL, p);
+    if (!p.end())
+      msg = decode_message(NULL, p);
   }
   void encode_payload(uint64_t features) {
     ::encode(session_mon_tid, payload);
     ::encode(dest, payload);
-    encode_message(msg, features, payload);
+    if (msg)
+      encode_message(msg, features, payload);
   }
 
   const char *get_type_name() const { return "route"; }
@@ -56,7 +58,7 @@ public:
     if (msg)
       o << "route(" << *msg;
     else
-      o << "route(???";
+      o << "route(no-reply";
     if (session_mon_tid)
       o << " tid " << session_mon_tid << ")";
     else
diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index e51e85231e12a..6700ba515c113 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -1566,6 +1566,26 @@ void Monitor::send_reply(PaxosServiceMessage *req, Message *reply)
   session->put();
 }
 
+void Monitor::no_reply(PaxosServiceMessage *req)
+{
+  MonSession *session = (MonSession*)req->get_connection()->get_priv();
+  assert(session);
+  if (session->proxy_con) {
+    if (get_quorum_features() & CEPH_FEATURE_MON_NULLROUTE) {
+      dout(10) << "no_reply to " << req->get_source_inst() << " via mon." << req->session_mon
+	       << " for request " << *req << dendl;
+      messenger->send_message(new MRoute(session->proxy_tid, NULL),
+			      session->proxy_con);
+    } else {
+      dout(10) << "no_reply no quorum nullroute feature for " << req->get_source_inst() << " via mon." << req->session_mon
+	       << " for request " << *req << dendl;
+    }
+  } else {
+    dout(10) << "no_reply to " << req->get_source_inst() << " " << *req << dendl;
+  }
+  session->put();
+}
+
 void Monitor::handle_route(MRoute *m)
 {
   MonSession *session = (MonSession *)m->get_connection()->get_priv();
@@ -1577,7 +1597,10 @@ void Monitor::handle_route(MRoute *m)
     m->put();
     return;
   }
-  dout(10) << "handle_route " << *m->msg << " to " << m->dest << dendl;
+  if (m->msg)
+    dout(10) << "handle_route " << *m->msg << " to " << m->dest << dendl;
+  else
+    dout(10) << "handle_route null to " << m->dest << dendl;
   
   // look it up
   if (m->session_mon_tid) {
@@ -1585,10 +1608,11 @@ void Monitor::handle_route(MRoute *m)
       RoutedRequest *rr = routed_requests[m->session_mon_tid];
 
       // reset payload, in case encoding is dependent on target features
-      m->msg->clear_payload();
-
-      messenger->send_message(m->msg, rr->session->inst);
-      m->msg = NULL;
+      if (m->msg) {
+	m->msg->clear_payload();
+	messenger->send_message(m->msg, rr->session->inst);
+	m->msg = NULL;
+      }
       routed_requests.erase(m->session_mon_tid);
       rr->session->routed_request_tids.insert(rr->tid);
       delete rr;
@@ -1597,8 +1621,10 @@ void Monitor::handle_route(MRoute *m)
     }
   } else {
     dout(10) << " not a routed request, trying to send anyway" << dendl;
-    messenger->lazy_send_message(m->msg, m->dest);
-    m->msg = NULL;
+    if (m->msg) {
+      messenger->lazy_send_message(m->msg, m->dest);
+      m->msg = NULL;
+    }
   }
   m->put();
   if (session)
diff --git a/src/mon/Monitor.h b/src/mon/Monitor.h
index 9e92de6be7f06..6b80ca2859cfc 100644
--- a/src/mon/Monitor.h
+++ b/src/mon/Monitor.h
@@ -351,6 +351,7 @@ public:
   void handle_forward(MForward *m);
   void try_send_message(Message *m, entity_inst_t to);
   void send_reply(PaxosServiceMessage *req, Message *reply);
+  void no_reply(PaxosServiceMessage *req);
   void resend_routed_requests();
   void remove_session(MonSession *s);
 
-- 
2.39.5