From a56df5f8a02b466a685fca9579ad325f7ef08006 Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Fri, 22 Nov 2019 16:11:37 -0800 Subject: [PATCH] HBASE-23333 include simple Call.toString() in sendCall exceptions --- .../hbase/ipc/BlockingRpcConnection.java | 8 ++--- .../org/apache/hadoop/hbase/ipc/Call.java | 29 +++++++++++-------- .../org/apache/hadoop/hbase/ipc/IPCUtil.java | 2 +- .../hadoop/hbase/ipc/RpcConnection.java | 2 +- .../hadoop/hbase/util/TestFutureUtils.java | 2 ++ 5 files changed, 25 insertions(+), 18 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 6df2a3946fd2..4c4d555c4a90 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 @@ public CallSender(String name, Configuration conf) { 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 + " to the write queue. callsToWrite.size()=" + callsToWrite.size()); } callsToWrite.offer(call); @@ -161,7 +161,7 @@ public void remove(Call call) { // 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 + ", waitTime=" + (EnvironmentEdgeManager.currentTime() - call.getStartTime()) + ", rpcTimeout=" + call.timeout)); } @@ -194,7 +194,7 @@ public void run() { // 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, e); } call.setException(e); closeConn(e); @@ -628,7 +628,7 @@ private void writeRequest(Call call) throws IOException { 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, t); } 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 e39aba073bc1..76f7a47b3a81 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; @@ -78,8 +78,13 @@ protected Call(int id, final Descriptors.MethodDescriptor md, Message param, @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) + .append("id", id) + .append("methodName", md.getName()) + .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 e9981b3ed7a2..911145872d95 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 @@ static IOException wrapException(InetSocketAddress addr, Throwable error) { } static void setCancelled(Call call) { - call.setException(new CallCancelledException("Call id=" + call.id + ", waitTime=" + call.setException(new CallCancelledException(call + ", 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 a935bb41a420..fbcd6725937e 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 @@ protected void scheduleTimeoutTask(final Call call) { @Override public void run(Timeout timeout) throws Exception { - call.setTimeout(new CallTimeoutException("Call id=" + call.id + ", waitTime=" + call.setTimeout(new CallTimeoutException(call + ", 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 5245f93b24cc..d560b24f61ee 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 void testRecordStackTrace() throws IOException { 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); } } }