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 a2ba855 [tablet] don't call UpdateMetricsForOp() for rows with errors a2ba855 is described below commit a2ba855deaba72846362c95d3614bdb3dd282a87 Author: Alexey Serbin <ale...@apache.org> AuthorDate: Mon Feb 28 10:16:28 2022 -0800 [tablet] don't call UpdateMetricsForOp() for rows with errors This patch updates the control path in WriteOp::UpdatePerRowErrors() to avoid calling UpdateMetricsForOp() for a row that contains an error since that's effectively a no-op. Also, renamed UpdatePerRowErrors() into UpdatePerRowMetricsAndErrors(). Change-Id: Ic1f57ee7d1b0064569a34ba93d35979426f76812 Reviewed-on: http://gerrit.cloudera.org:8080/18281 Tested-by: Alexey Serbin <aser...@cloudera.com> Reviewed-by: Andrew Wong <aw...@cloudera.com> --- src/kudu/tablet/ops/write_op.cc | 21 ++++++++++----------- src/kudu/tablet/ops/write_op.h | 7 +++++-- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/kudu/tablet/ops/write_op.cc b/src/kudu/tablet/ops/write_op.cc index 83097f9..7f1defb 100644 --- a/src/kudu/tablet/ops/write_op.cc +++ b/src/kudu/tablet/ops/write_op.cc @@ -243,19 +243,20 @@ Status WriteOp::Start() { return Status::OK(); } -void WriteOp::UpdatePerRowErrors() { - // Add per-row errors to the result, update metrics. - for (int i = 0; i < state()->row_ops().size(); ++i) { - const RowOp* op = state()->row_ops()[i]; +void WriteOp::UpdatePerRowMetricsAndErrors() { + // Update metrics or add per-row errors to the result. + size_t idx = 0; + for (const auto* op : state()->row_ops()) { if (op->result->has_failed_status()) { // Replicas disregard the per row errors, for now // TODO(unknown): check the per-row errors against the leader's, at least in debug mode WriteResponsePB::PerRowErrorPB* error = state()->response()->add_per_row_errors(); - error->set_row_index(i); + error->set_row_index(idx); error->mutable_error()->CopyFrom(op->result->failed_status()); + } else { + state()->UpdateMetricsForOp(*op); } - - state()->UpdateMetricsForOp(*op); + ++idx; } } @@ -276,7 +277,7 @@ Status WriteOp::Apply(CommitMsg** commit_msg) { RETURN_NOT_OK(tablet->ApplyRowOperations(state())); TRACE("APPLY: Finished."); - UpdatePerRowErrors(); + UpdatePerRowMetricsAndErrors(); // Create the Commit message *commit_msg = google::protobuf::Arena::CreateMessage<CommitMsg>(state_->pb_arena()); @@ -477,9 +478,7 @@ void WriteOpState::ReleaseTxResultPB(TxResultPB* result) const { } void WriteOpState::UpdateMetricsForOp(const RowOp& op) { - if (op.result->has_failed_status()) { - return; - } + DCHECK(!op.result->has_failed_status()); switch (op.decoded_op.type) { case RowOperationsPB::INSERT: DCHECK(!op.error_ignored); diff --git a/src/kudu/tablet/ops/write_op.h b/src/kudu/tablet/ops/write_op.h index c8621fa..8c14176 100644 --- a/src/kudu/tablet/ops/write_op.h +++ b/src/kudu/tablet/ops/write_op.h @@ -29,7 +29,7 @@ #include <google/protobuf/stubs/port.h> #include "kudu/common/row_operations.h" -#include "kudu/common/wire_protocol.pb.h" +#include "kudu/common/row_operations.pb.h" #include "kudu/consensus/consensus.pb.h" #include "kudu/gutil/macros.h" #include "kudu/gutil/ref_counted.h" @@ -385,7 +385,10 @@ class WriteOp : public Op { std::string ToString() const override; private: - void UpdatePerRowErrors(); + // For each row of this write operation, update corresponding metrics or set + // corresponding error information in the response. The former is for + // successfully written rows, the latter is for failed ones. + void UpdatePerRowMetricsAndErrors(); // this op's start time MonoTime start_time_;