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: