From f98085e44e82083327b33eded0b8d9e52ceba099 Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Fri, 22 Nov 2019 16:11:37 -0800 Subject: [PATCH] HBASE-23333 Include Call.toShortString() in sendCall exceptions --- .../hbase/ipc/BlockingRpcConnection.java | 10 ++--- .../org/apache/hadoop/hbase/ipc/Call.java | 38 +++++++++++++------ .../org/apache/hadoop/hbase/ipc/IPCUtil.java | 2 +- .../hadoop/hbase/ipc/RpcConnection.java | 2 +- .../hadoop/hbase/util/TestFutureUtils.java | 2 + 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java index 6df2a3946fd..c5b1e5766ac 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java @@ -149,7 +149,7 @@ class BlockingRpcConnection extends RpcConnection implements Runnable { public void sendCall(final Call call) throws IOException { if (callsToWrite.size() >= maxQueueSize) { - throw new IOException("Can't add the call " + call.id + throw new IOException("Can't add " + call.toShortString() + " to the write queue. callsToWrite.size()=" + callsToWrite.size()); } callsToWrite.offer(call); @@ -161,7 +161,7 @@ class BlockingRpcConnection extends RpcConnection implements Runnable { // By removing the call from the expected call list, we make the list smaller, but // it means as well that we don't know how many calls we cancelled. calls.remove(call.id); - call.setException(new CallCancelledException("Call id=" + call.id + ", waitTime=" + call.setException(new CallCancelledException(call.toShortString() + ", waitTime=" + (EnvironmentEdgeManager.currentTime() - call.getStartTime()) + ", rpcTimeout=" + call.timeout)); } @@ -193,9 +193,7 @@ class BlockingRpcConnection extends RpcConnection implements Runnable { } catch (IOException e) { // exception here means the call has not been added to the pendingCalls yet, so we need // to fail it by our own. - if (LOG.isDebugEnabled()) { - LOG.debug("call write error for call #" + call.id, e); - } + LOG.debug("call write error for {}", call.toShortString()); call.setException(e); closeConn(e); } @@ -628,7 +626,7 @@ class BlockingRpcConnection extends RpcConnection implements Runnable { call.callStats.setRequestSizeBytes(write(this.out, requestHeader, call.param, cellBlock)); } catch (Throwable t) { if(LOG.isTraceEnabled()) { - LOG.trace("Error while writing call, call_id:" + call.id, t); + LOG.trace("Error while writing {}", call.toShortString()); } IOException e = IPCUtil.toIOE(t); closeConn(e); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/Call.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/Call.java index e39aba073bc..ccd92cc7472 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/Call.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/Call.java @@ -17,21 +17,21 @@ */ package org.apache.hadoop.hbase.ipc; -import org.apache.hbase.thirdparty.io.netty.util.Timeout; - -import org.apache.hbase.thirdparty.com.google.protobuf.Descriptors; -import org.apache.hbase.thirdparty.com.google.protobuf.Message; -import org.apache.hbase.thirdparty.com.google.protobuf.RpcCallback; -import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; - import java.io.IOException; - +import java.util.Optional; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; import org.apache.hadoop.hbase.CellScanner; -import org.apache.yetus.audience.InterfaceAudience; import org.apache.hadoop.hbase.client.MetricsConnection; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.htrace.core.Span; import org.apache.htrace.core.Tracer; +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.hbase.thirdparty.com.google.protobuf.Descriptors; +import org.apache.hbase.thirdparty.com.google.protobuf.Message; +import org.apache.hbase.thirdparty.com.google.protobuf.RpcCallback; +import org.apache.hbase.thirdparty.io.netty.util.Timeout; +import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; /** A call waiting for a value. */ @InterfaceAudience.Private @@ -56,7 +56,7 @@ class Call { final int timeout; // timeout in millisecond for this call; 0 means infinite. final int priority; final MetricsConnection.CallStats callStats; - final RpcCallback callback; + private final RpcCallback callback; final Span span; Timeout timeoutTask; @@ -76,10 +76,24 @@ class Call { this.span = Tracer.getCurrentSpan(); } + /** + * Builds a simplified {@link #toString()} that includes just the id and method name. + */ + public String toShortString() { + return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) + .append("id", id) + .append("methodName", md.getName()) + .toString(); + } + @Override public String toString() { - return "callId: " + this.id + " methodName: " + this.md.getName() + " param {" - + (this.param != null ? ProtobufUtil.getShortTextFormat(this.param) : "") + "}"; + return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) + .appendSuper(toShortString()) + .append("param", Optional.ofNullable(param) + .map(ProtobufUtil::getShortTextFormat) + .orElse("")) + .toString(); } /** diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/IPCUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/IPCUtil.java index e9981b3ed7a..8e926409ad4 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/IPCUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/IPCUtil.java @@ -230,7 +230,7 @@ class IPCUtil { } static void setCancelled(Call call) { - call.setException(new CallCancelledException("Call id=" + call.id + ", waitTime=" + call.setException(new CallCancelledException(call.toShortString() + ", waitTime=" + (EnvironmentEdgeManager.currentTime() - call.getStartTime()) + ", rpcTimeout=" + call.timeout)); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcConnection.java index a935bb41a42..1fffebbf5b0 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcConnection.java @@ -196,7 +196,7 @@ abstract class RpcConnection { @Override public void run(Timeout timeout) throws Exception { - call.setTimeout(new CallTimeoutException("Call id=" + call.id + ", waitTime=" + call.setTimeout(new CallTimeoutException(call.toShortString() + ", waitTime=" + (EnvironmentEdgeManager.currentTime() - call.getStartTime()) + ", rpcTimeout=" + call.timeout)); callTimeout(call); diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestFutureUtils.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestFutureUtils.java index 5245f93b24c..d560b24f61e 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestFutureUtils.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestFutureUtils.java @@ -80,6 +80,8 @@ public class TestFutureUtils { startsWith("org.apache.hadoop.hbase.util.TestFutureUtils.testRecordStackTrace")); assertTrue(Stream.of(elements) .anyMatch(element -> element.toString().contains("--------Future.get--------"))); + } catch (Throwable t) { + throw new AssertionError("Caught unexpected Throwable", t); } } }