vli02 commented on code in PR #5023:
URL: https://github.com/apache/hbase/pull/5023#discussion_r1113571945
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java:
##########
@@ -638,16 +641,23 @@ private void shutdown() {
}
/** 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);
}
// 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();
+ if (e instanceof RemoteWithExtrasException) {
+ getMetric(REMOTE_EXCEPTION_CNT_BASE + ((RemoteWithExtrasException)
e).getClassName(),
+ rpcCounters, counterFactory).inc();
Review Comment:
Yes, it makes more sense.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java:
##########
@@ -638,16 +641,23 @@ private void shutdown() {
}
/** 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);
}
// 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();
+ if (e instanceof RemoteWithExtrasException) {
+ getMetric(REMOTE_EXCEPTION_CNT_BASE + ((RemoteWithExtrasException)
e).getClassName(),
Review Comment:
Your are right, the `className` in `RemoteException` should be the full
exception class name. I am going to use substring to get the last part of the
name.
##########
hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java:
##########
@@ -150,51 +152,71 @@ public void testStaticMetrics() throws IOException {
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("IOException", null, false, false));
Review Comment:
Good catch, it should be this form from server side response.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java:
##########
@@ -638,16 +641,23 @@ private void shutdown() {
}
/** 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);
}
// 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();
+ if (e instanceof RemoteWithExtrasException) {
Review Comment:
Make sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
