This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new ad3936521 Minor refactoring on Op class
ad3936521 is described below

commit ad3936521af034ffcac637f97cd8c932f6289b4f
Author: kedeng <kdeng...@gmail.com>
AuthorDate: Mon Apr 22 12:30:46 2024 +0800

    Minor refactoring on Op class
    
    We have derived various types of operations based on the Op class.
    Now, if we need to add time statistics for these operations, it
    would be repetitive to add an initial timestamp separately for
    each derived class.
    
    In this patch, I moved the 'start_time_' from the WriteOp class to
    the OpState class, making it easier for subsequent derived classes
    to use.
    
    Since there are no logical changes, no additional unit tests have
    been added.
    
    Change-Id: Ie391d4a55b8da08a62025a05cc466fc2b947099c
    Reviewed-on: http://gerrit.cloudera.org:8080/21342
    Reviewed-by: Yingchun Lai <laiyingc...@apache.org>
    Tested-by: Yingchun Lai <laiyingc...@apache.org>
    Reviewed-by: Alexey Serbin <ale...@apache.org>
---
 src/kudu/tablet/ops/op.h        | 10 ++++++++++
 src/kudu/tablet/ops/write_op.cc |  8 ++++----
 src/kudu/tablet/ops/write_op.h  |  4 ----
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/kudu/tablet/ops/op.h b/src/kudu/tablet/ops/op.h
index 63b8ed017..5c184e823 100644
--- a/src/kudu/tablet/ops/op.h
+++ b/src/kudu/tablet/ops/op.h
@@ -40,6 +40,7 @@
 #include "kudu/tserver/tserver.pb.h"
 #include "kudu/util/countdown_latch.h"
 #include "kudu/util/memory/arena.h"
+#include "kudu/util/monotime.h"
 #include "kudu/util/status.h"
 
 namespace google {
@@ -267,6 +268,11 @@ class OpState {
     return request_id_;
   }
 
+  // Get the startup time of this op.
+  MonoTime start_time() const { return start_time_; }
+  // Set the startup time of this op.
+  void set_start_time(MonoTime start_time) { start_time_ = start_time; }
+
  protected:
   explicit OpState(TabletReplica* tablet_replica);
   virtual ~OpState();
@@ -306,6 +312,10 @@ class OpState {
 
   // The defined consistency mode for this op.
   ExternalConsistencyMode external_consistency_mode_;
+
+  // Use to record the op's start time.
+  // 'set_start_time()' needs to be called at the beginning of the op to 
initialize it.
+  MonoTime start_time_;
 };
 
 // A parent class for the callback that gets called when ops
diff --git a/src/kudu/tablet/ops/write_op.cc b/src/kudu/tablet/ops/write_op.cc
index d53ce15b0..0e2a90cc9 100644
--- a/src/kudu/tablet/ops/write_op.cc
+++ b/src/kudu/tablet/ops/write_op.cc
@@ -63,6 +63,7 @@
 #include "kudu/util/flag_tags.h"
 #include "kudu/util/memory/arena.h"
 #include "kudu/util/metrics.h"
+#include "kudu/util/monotime.h"
 #include "kudu/util/pb_util.h"
 #include "kudu/util/slice.h"
 #include "kudu/util/trace.h"
@@ -157,7 +158,6 @@ Status WriteAuthorizationContext::CheckPrivileges() const {
 WriteOp::WriteOp(unique_ptr<WriteOpState> state, DriverType type)
   : Op(type, Op::WRITE_OP),
   state_(std::move(state)) {
-  start_time_ = MonoTime::Now();
 }
 
 void WriteOp::NewReplicateMsg(unique_ptr<ReplicateMsg>* replicate_msg) {
@@ -270,6 +270,7 @@ Status WriteOp::Start() {
   TRACE("Start()");
   DCHECK(!state_->has_timestamp());
   DCHECK(state_->consensus_round()->replicate_msg()->has_timestamp());
+  state_->set_start_time(MonoTime::Now());
   
state_->set_timestamp(Timestamp(state_->consensus_round()->replicate_msg()->timestamp()));
   state_->tablet_replica()->tablet()->StartOp(state_.get());
   TRACE("Timestamp: $0", 
state_->tablet_replica()->clock()->Stringify(state_->timestamp()));
@@ -358,7 +359,7 @@ void WriteOp::Finish(OpResult result) {
         
metrics->commit_wait_duration->Increment(op_m.commit_wait_duration_usec);
       }
       uint64_t op_duration_usec =
-          (MonoTime::Now() - start_time_).ToMicroseconds();
+          (MonoTime::Now() - state_->start_time()).ToMicroseconds();
       switch (state()->external_consistency_mode()) {
         case CLIENT_PROPAGATED:
           
metrics->write_op_duration_client_propagated_consistency->Increment(op_duration_usec);
@@ -374,8 +375,7 @@ void WriteOp::Finish(OpResult result) {
 }
 
 string WriteOp::ToString() const {
-  MonoTime now(MonoTime::Now());
-  MonoDelta d = now - start_time_;
+  MonoDelta d = MonoTime::Now() - state_->start_time();
   WallTime abs_time = WallTime_Now() - d.ToSeconds();
   string abs_time_formatted;
   StringAppendStrftime(&abs_time_formatted, "%Y-%m-%d %H:%M:%S", 
(time_t)abs_time, true);
diff --git a/src/kudu/tablet/ops/write_op.h b/src/kudu/tablet/ops/write_op.h
index c794a8a49..f1b25b55d 100644
--- a/src/kudu/tablet/ops/write_op.h
+++ b/src/kudu/tablet/ops/write_op.h
@@ -41,7 +41,6 @@
 #include "kudu/tserver/tserver.pb.h"
 #include "kudu/util/bitset.h"
 #include "kudu/util/locks.h"
-#include "kudu/util/monotime.h"
 #include "kudu/util/status.h"
 
 namespace kudu {
@@ -400,9 +399,6 @@ class WriteOp : public Op {
   // successfully written rows, the latter is for failed ones.
   void UpdatePerRowMetricsAndErrors();
 
-  // this op's start time
-  MonoTime start_time_;
-
   std::unique_ptr<WriteOpState> state_;
 
  private:

Reply via email to