common: mark events of TrackedOp outside its constructor

Avoid marking events when TrackedOp::tracking_start() isn't invoked yet. Fixes: http://tracker.ceph.com/issues/22608 Signed-off-by: Xuehan Xu <xuxuehan@360.cn>
ceph · Jan 13, 2018 · a74e554 · a74e554
1 parent 1f32d63
commit a74e554
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 26 deletions.
diff --git a/src/common/TrackedOp.h b/src/common/TrackedOp.h
@@ -146,6 +146,14 @@ class OpTracker {
   {
     typename T::Ref retval(new T(params, this));
     retval->tracking_start();
+
+    if (is_tracking()) {
+      retval->mark_event("header_read", params->get_recv_stamp());
+      retval->mark_event("throttled", params->get_throttle_stamp());
+      retval->mark_event("all_read", params->get_recv_complete_stamp());
+      retval->mark_event("dispatched", params->get_dispatch_stamp());
+    }
+
     return retval;
   }
 };

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
@@ -9048,7 +9048,7 @@ MDRequestRef MDCache::request_start(MClientRequest *req)
   params.dispatched = req->get_dispatch_stamp();
 
   MDRequestRef mdr =
-      mds->op_tracker.create_request<MDRequestImpl,MDRequestImpl::Params>(params);
+      mds->op_tracker.create_request<MDRequestImpl,MDRequestImpl::Params*>(&params);
   active_requests[params.reqid] = mdr;
   mdr->set_op_stamp(req->get_stamp());
   dout(7) << "request_start " << *mdr << dendl;
@@ -9068,7 +9068,7 @@ MDRequestRef MDCache::request_start_slave(metareqid_t ri, __u32 attempt, Message
   params.all_read = m->get_recv_complete_stamp();
   params.dispatched = m->get_dispatch_stamp();
   MDRequestRef mdr =
-      mds->op_tracker.create_request<MDRequestImpl,MDRequestImpl::Params>(params);
+      mds->op_tracker.create_request<MDRequestImpl,MDRequestImpl::Params*>(&params);
   assert(active_requests.count(mdr->reqid) == 0);
   active_requests[mdr->reqid] = mdr;
   dout(7) << "request_start_slave " << *mdr << " by mds." << by << dendl;
@@ -9083,7 +9083,7 @@ MDRequestRef MDCache::request_start_internal(int op)
   params.initiated = ceph_clock_now();
   params.internal_op = op;
   MDRequestRef mdr =
-      mds->op_tracker.create_request<MDRequestImpl,MDRequestImpl::Params>(params);
+      mds->op_tracker.create_request<MDRequestImpl,MDRequestImpl::Params*>(&params);
 
   assert(active_requests.count(mdr->reqid) == 0);
   active_requests[mdr->reqid] = mdr;

diff --git a/src/mds/Mutation.h b/src/mds/Mutation.h
@@ -293,24 +293,30 @@ struct MDRequestImpl : public MutationImpl {
     // keep these default values synced to MutationImpl's
     Params() : attempt(0), client_req(NULL),
         triggering_slave_req(NULL), slave_to(MDS_RANK_NONE), internal_op(-1) {}
+    const utime_t& get_recv_stamp() const {
+      return initiated;
+    }
+    const utime_t& get_throttle_stamp() const {
+      return throttled;
+    }
+    const utime_t& get_recv_complete_stamp() const {
+      return all_read;
+    }
+    const utime_t& get_dispatch_stamp() const {
+      return dispatched;
+    }
   };
-  MDRequestImpl(const Params& params, OpTracker *tracker) :
-    MutationImpl(tracker, params.initiated,
-		 params.reqid, params.attempt, params.slave_to),
+  MDRequestImpl(const Params* params, OpTracker *tracker) :
+    MutationImpl(tracker, params->initiated,
+		 params->reqid, params->attempt, params->slave_to),
     session(NULL), item_session_request(this),
-    client_request(params.client_req), straydn(NULL), snapid(CEPH_NOSNAP),
+    client_request(params->client_req), straydn(NULL), snapid(CEPH_NOSNAP),
     tracei(NULL), tracedn(NULL), alloc_ino(0), used_prealloc_ino(0),
-    slave_request(NULL), internal_op(params.internal_op), internal_op_finish(NULL),
+    slave_request(NULL), internal_op(params->internal_op), internal_op_finish(NULL),
     internal_op_private(NULL),
     retry(0),
     waited_for_osdmap(false), _more(NULL) {
     in[0] = in[1] = NULL;
-    if (!params.throttled.is_zero())
-      mark_event("throttled", params.throttled);
-    if (!params.all_read.is_zero())
-      mark_event("all_read", params.all_read);
-    if (!params.dispatched.is_zero())
-      mark_event("dispatched", params.dispatched);
   }
   ~MDRequestImpl() override;
 

diff --git a/src/mon/MonOpRequest.h b/src/mon/MonOpRequest.h
@@ -95,11 +95,6 @@ struct MonOpRequest : public TrackedOp {
     forwarded_to_leader(false),
     op_type(OP_TYPE_NONE)
   {
-    mark_event("header_read", request->get_recv_stamp());
-    mark_event("throttled", request->get_throttle_stamp());
-    mark_event("all_read", request->get_recv_complete_stamp());
-    mark_event("dispatched", request->get_dispatch_stamp());
-
     if (req) {
       con = req->get_connection();
       if (con) {

diff --git a/src/osd/OpRequest.cc b/src/osd/OpRequest.cc
@@ -39,13 +39,7 @@ OpRequest::OpRequest(Message *req, OpTracker *tracker) :
   } else if (req->get_type() == MSG_OSD_REPOPREPLY) {
     reqid = static_cast<MOSDRepOpReply*>(req)->reqid;
   }
-  if (tracker->is_tracking()) {
-    req_src_inst = req->get_source_inst();
-    mark_event("header_read", request->get_recv_stamp());
-    mark_event("throttled", request->get_throttle_stamp());
-    mark_event("all_read", request->get_recv_complete_stamp());
-    mark_event("dispatched", request->get_dispatch_stamp());
-  }
+  req_src_inst = req->get_source_inst();
 }
 
 void OpRequest::_dump(Formatter *f) const