[kudu] branch master updated: [tablet] don't call UpdateMetricsForOp() for rows with errors

[al...@apache.org]

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 <al...@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 <as...@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_;