You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2023/02/25 00:36:37 UTC

[hbase] branch branch-2.5 updated: HBASE-15242: add client side metrics for timeout and remote exceptions. (#5023)

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

apurtell pushed a commit to branch branch-2.5
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2.5 by this push:
     new 7cb4a58bb1b HBASE-15242: add client side metrics for timeout and remote exceptions. (#5023)
7cb4a58bb1b is described below

commit 7cb4a58bb1b7c632930545af7b2547deea5260c4
Author: Victor <vl...@gmail.com>
AuthorDate: Fri Feb 24 16:25:01 2023 -0800

    HBASE-15242: add client side metrics for timeout and remote exceptions. (#5023)
    
    Signed-off-by: Andrew Purtell <ap...@apache.org>
---
 .../hadoop/hbase/client/MetricsConnection.java     | 19 ++++++++-
 .../apache/hadoop/hbase/ipc/AbstractRpcClient.java |  7 ++--
 .../hadoop/hbase/client/TestMetricsConnection.java | 48 +++++++++++++++++-----
 3 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
index 302cb283225..0f9b5a5ee86 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
@@ -37,6 +37,7 @@ import java.util.function.Supplier;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.ipc.RemoteException;
 import org.apache.yetus.audience.InterfaceAudience;
 
 import org.apache.hbase.thirdparty.com.google.protobuf.Descriptors.MethodDescriptor;
@@ -118,6 +119,9 @@ public final class MetricsConnection implements StatisticTrackable {
 
   private static final String CNT_BASE = "rpcCount_";
   private static final String FAILURE_CNT_BASE = "rpcFailureCount_";
+  private static final String TOTAL_EXCEPTION_CNT = "rpcTotalExceptions";
+  private static final String LOCAL_EXCEPTION_CNT_BASE = "rpcLocalExceptions_";
+  private static final String REMOTE_EXCEPTION_CNT_BASE = "rpcRemoteExceptions_";
   private static final String DRTN_BASE = "rpcCallDurationMs_";
   private static final String REQ_BASE = "rpcCallRequestSizeBytes_";
   private static final String RESP_BASE = "rpcCallResponseSizeBytes_";
@@ -638,7 +642,7 @@ public final class MetricsConnection implements StatisticTrackable {
   }
 
   /** Report RPC context to metrics system. */
-  public void updateRpc(MethodDescriptor method, Message param, CallStats stats, boolean failed) {
+  public void updateRpc(MethodDescriptor method, Message param, CallStats stats, Throwable e) {
     int callsPerServer = stats.getConcurrentCallsPerServer();
     if (callsPerServer > 0) {
       concurrentCallsPerServerHist.update(callsPerServer);
@@ -646,8 +650,19 @@ public final class MetricsConnection implements StatisticTrackable {
     // Update the counter that tracks RPCs by type.
     final String methodName = method.getService().getName() + "_" + method.getName();
     getMetric(CNT_BASE + methodName, rpcCounters, counterFactory).inc();
-    if (failed) {
+    if (e != null) {
       getMetric(FAILURE_CNT_BASE + methodName, rpcCounters, counterFactory).inc();
+      getMetric(TOTAL_EXCEPTION_CNT, rpcCounters, counterFactory).inc();
+      if (e instanceof RemoteException) {
+        String fullClassName = ((RemoteException) e).getClassName();
+        String simpleClassName = (fullClassName != null)
+          ? fullClassName.substring(fullClassName.lastIndexOf(".") + 1)
+          : "unknown";
+        getMetric(REMOTE_EXCEPTION_CNT_BASE + simpleClassName, rpcCounters, counterFactory).inc();
+      } else {
+        getMetric(LOCAL_EXCEPTION_CNT_BASE + e.getClass().getSimpleName(), rpcCounters,
+          counterFactory).inc();
+      }
     }
     // this implementation is tied directly to protobuf implementation details. would be better
     // if we could dispatch based on something static, ie, request Message type.
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AbstractRpcClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AbstractRpcClient.java
index e0dbb509575..35ecb8b2435 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AbstractRpcClient.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AbstractRpcClient.java
@@ -375,16 +375,15 @@ public abstract class AbstractRpcClient<T extends RpcConnection> implements RpcC
   private void onCallFinished(Call call, HBaseRpcController hrc, Address addr,
     RpcCallback<Message> callback) {
     call.callStats.setCallTimeMs(EnvironmentEdgeManager.currentTime() - call.getStartTime());
-    final boolean failed = (call.error != null) ? true : false;
     if (metrics != null) {
-      metrics.updateRpc(call.md, call.param, call.callStats, failed);
+      metrics.updateRpc(call.md, call.param, call.callStats, call.error);
     }
     if (LOG.isTraceEnabled()) {
       LOG.trace("CallId: {}, call: {}, startTime: {}ms, callTime: {}ms, status: {}", call.id,
         call.md.getName(), call.getStartTime(), call.callStats.getCallTimeMs(),
-        failed ? "failed" : "successful");
+        call.error != null ? "failed" : "successful");
     }
-    if (failed) {
+    if (call.error != null) {
       if (call.error instanceof RemoteException) {
         call.error.fillInStackTrace();
         hrc.setFailed(call.error);
diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
index 498cf6c3d5e..66d81599427 100644
--- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
+++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
@@ -33,6 +33,8 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadPoolExecutor;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.ipc.CallTimeoutException;
+import org.apache.hadoop.hbase.ipc.RemoteWithExtrasException;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.testclassification.ClientTests;
 import org.apache.hadoop.hbase.testclassification.MetricsTests;
@@ -177,51 +179,77 @@ public class TestMetricsConnection {
 
     for (int i = 0; i < loop; i++) {
       METRICS.updateRpc(ClientService.getDescriptor().findMethodByName("Get"),
-        GetRequest.getDefaultInstance(), MetricsConnection.newCallStats(), false);
+        GetRequest.getDefaultInstance(), MetricsConnection.newCallStats(), null);
       METRICS.updateRpc(ClientService.getDescriptor().findMethodByName("Scan"),
-        ScanRequest.getDefaultInstance(), MetricsConnection.newCallStats(), false);
+        ScanRequest.getDefaultInstance(), MetricsConnection.newCallStats(),
+        new RemoteWithExtrasException("java.io.IOException", null, false, false));
       METRICS.updateRpc(ClientService.getDescriptor().findMethodByName("Multi"),
-        MultiRequest.getDefaultInstance(), MetricsConnection.newCallStats(), true);
+        MultiRequest.getDefaultInstance(), MetricsConnection.newCallStats(),
+        new CallTimeoutException("test with CallTimeoutException"));
       METRICS.updateRpc(ClientService.getDescriptor().findMethodByName("Mutate"),
         MutateRequest.newBuilder()
           .setMutation(ProtobufUtil.toMutation(MutationType.APPEND, new Append(foo)))
           .setRegion(region).build(),
-        MetricsConnection.newCallStats(), false);
+        MetricsConnection.newCallStats(), null);
       METRICS.updateRpc(ClientService.getDescriptor().findMethodByName("Mutate"),
         MutateRequest.newBuilder()
           .setMutation(ProtobufUtil.toMutation(MutationType.DELETE, new Delete(foo)))
           .setRegion(region).build(),
-        MetricsConnection.newCallStats(), false);
+        MetricsConnection.newCallStats(), null);
       METRICS.updateRpc(ClientService.getDescriptor().findMethodByName("Mutate"),
         MutateRequest.newBuilder()
           .setMutation(ProtobufUtil.toMutation(MutationType.INCREMENT, new Increment(foo)))
           .setRegion(region).build(),
-        MetricsConnection.newCallStats(), false);
+        MetricsConnection.newCallStats(), null);
       METRICS.updateRpc(ClientService.getDescriptor().findMethodByName("Mutate"),
         MutateRequest.newBuilder()
           .setMutation(ProtobufUtil.toMutation(MutationType.PUT, new Put(foo))).setRegion(region)
           .build(),
-        MetricsConnection.newCallStats(), false);
+        MetricsConnection.newCallStats(), null);
     }
+
     final String rpcCountPrefix = "rpcCount_" + ClientService.getDescriptor().getName() + "_";
     final String rpcFailureCountPrefix =
       "rpcFailureCount_" + ClientService.getDescriptor().getName() + "_";
     String metricKey;
     long metricVal;
     Counter counter;
-    for (String method : new String[] { "Get", "Scan", "Mutate" }) {
+
+    for (String method : new String[] { "Get", "Scan", "Multi", "Mutate" }) {
       metricKey = rpcCountPrefix + method;
       metricVal = METRICS.getRpcCounters().get(metricKey).getCount();
       assertTrue("metric: " + metricKey + " val: " + metricVal, metricVal >= loop);
+
       metricKey = rpcFailureCountPrefix + method;
       counter = METRICS.getRpcCounters().get(metricKey);
       metricVal = (counter != null) ? counter.getCount() : 0;
-      assertTrue("metric: " + metricKey + " val: " + metricVal, metricVal == 0);
+      if (method.equals("Get") || method.equals("Mutate")) {
+        // no failure
+        assertTrue("metric: " + metricKey + " val: " + metricVal, metricVal == 0);
+      } else {
+        // has failure
+        assertTrue("metric: " + metricKey + " val: " + metricVal, metricVal == loop);
+      }
     }
-    metricKey = rpcFailureCountPrefix + "Multi";
+
+    // remote exception
+    metricKey = "rpcRemoteExceptions_IOException";
+    counter = METRICS.getRpcCounters().get(metricKey);
+    metricVal = (counter != null) ? counter.getCount() : 0;
+    assertTrue("metric: " + metricKey + " val: " + metricVal, metricVal == loop);
+
+    // local exception
+    metricKey = "rpcLocalExceptions_CallTimeoutException";
     counter = METRICS.getRpcCounters().get(metricKey);
     metricVal = (counter != null) ? counter.getCount() : 0;
     assertTrue("metric: " + metricKey + " val: " + metricVal, metricVal == loop);
+
+    // total exception
+    metricKey = "rpcTotalExceptions";
+    counter = METRICS.getRpcCounters().get(metricKey);
+    metricVal = (counter != null) ? counter.getCount() : 0;
+    assertTrue("metric: " + metricKey + " val: " + metricVal, metricVal == loop * 2);
+
     for (MetricsConnection.CallTracker t : new MetricsConnection.CallTracker[] {
       METRICS.getGetTracker(), METRICS.getScanTracker(), METRICS.getMultiTracker(),
       METRICS.getAppendTracker(), METRICS.getDeleteTracker(), METRICS.getIncrementTracker(),