HBASE-21862 IPCUtil.wrapException should keep the original exception types for all the connection exceptions
Signed-off-by: Michael Stack <stack@apache.org>
This commit is contained in:
parent
16f27d7699
commit
54f9afba04
|
@ -26,6 +26,7 @@ import java.lang.reflect.UndeclaredThrowableException;
|
|||
import java.net.ConnectException;
|
||||
import java.net.SocketTimeoutException;
|
||||
import java.nio.channels.ClosedChannelException;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.TimeoutException;
|
||||
|
||||
import org.apache.hadoop.hbase.CallDroppedException;
|
||||
|
@ -37,6 +38,10 @@ import org.apache.hadoop.hbase.RegionTooBusyException;
|
|||
import org.apache.hadoop.hbase.RetryImmediatelyException;
|
||||
import org.apache.yetus.audience.InterfaceAudience;
|
||||
import org.apache.yetus.audience.InterfaceStability;
|
||||
|
||||
import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
|
||||
import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet;
|
||||
|
||||
import org.apache.hadoop.hbase.ipc.CallTimeoutException;
|
||||
import org.apache.hadoop.hbase.ipc.FailedServerException;
|
||||
import org.apache.hadoop.hbase.quotas.RpcThrottlingException;
|
||||
|
@ -131,10 +136,28 @@ public final class ClientExceptionsUtil {
|
|||
return (t instanceof CallDroppedException);
|
||||
}
|
||||
|
||||
// This list covers most connectivity exceptions but not all.
|
||||
// For example, in SocketOutputStream a plain IOException is thrown at times when the channel is
|
||||
// closed.
|
||||
private static final ImmutableSet<Class<? extends Throwable>> CONNECTION_EXCEPTION_TYPES =
|
||||
ImmutableSet.of(SocketTimeoutException.class, ConnectException.class,
|
||||
ClosedChannelException.class, SyncFailedException.class, EOFException.class,
|
||||
TimeoutException.class, TimeoutIOException.class, CallTimeoutException.class,
|
||||
ConnectionClosingException.class, FailedServerException.class,
|
||||
ConnectionClosedException.class);
|
||||
|
||||
/**
|
||||
* Check if the exception is something that indicates that we cannot
|
||||
* contact/communicate with the server.
|
||||
*
|
||||
* For test only. Usually you should use the {@link #isConnectionException(Throwable)} method
|
||||
* below.
|
||||
*/
|
||||
@VisibleForTesting
|
||||
public static Set<Class<? extends Throwable>> getConnectionExceptionTypes() {
|
||||
return CONNECTION_EXCEPTION_TYPES;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the exception is something that indicates that we cannot contact/communicate with the
|
||||
* server.
|
||||
* @param e exception to check
|
||||
* @return true when exception indicates that the client wasn't able to make contact with server
|
||||
*/
|
||||
|
@ -142,14 +165,12 @@ public final class ClientExceptionsUtil {
|
|||
if (e == null) {
|
||||
return false;
|
||||
}
|
||||
// This list covers most connectivity exceptions but not all.
|
||||
// For example, in SocketOutputStream a plain IOException is thrown
|
||||
// at times when the channel is closed.
|
||||
return (e instanceof SocketTimeoutException || e instanceof ConnectException
|
||||
|| e instanceof ClosedChannelException || e instanceof SyncFailedException
|
||||
|| e instanceof EOFException || e instanceof TimeoutException
|
||||
|| e instanceof CallTimeoutException || e instanceof ConnectionClosingException
|
||||
|| e instanceof FailedServerException || e instanceof ConnectionClosedException);
|
||||
for (Class<? extends Throwable> clazz : CONNECTION_EXCEPTION_TYPES) {
|
||||
if (clazz.isAssignableFrom(e.getClass())) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -17,28 +17,34 @@
|
|||
*/
|
||||
package org.apache.hadoop.hbase.ipc;
|
||||
|
||||
import org.apache.hadoop.hbase.exceptions.ConnectionClosedException;
|
||||
import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
|
||||
import org.apache.hbase.thirdparty.com.google.protobuf.CodedOutputStream;
|
||||
import org.apache.hbase.thirdparty.com.google.protobuf.Message;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.OutputStream;
|
||||
import java.lang.reflect.InvocationTargetException;
|
||||
import java.net.ConnectException;
|
||||
import java.net.InetSocketAddress;
|
||||
import java.net.SocketTimeoutException;
|
||||
import java.nio.ByteBuffer;
|
||||
|
||||
import java.nio.channels.ClosedChannelException;
|
||||
import java.util.concurrent.TimeoutException;
|
||||
import org.apache.hadoop.hbase.DoNotRetryIOException;
|
||||
import org.apache.hadoop.hbase.HBaseIOException;
|
||||
import org.apache.hadoop.hbase.HConstants;
|
||||
import org.apache.yetus.audience.InterfaceAudience;
|
||||
import org.apache.hadoop.hbase.exceptions.ClientExceptionsUtil;
|
||||
import org.apache.hadoop.hbase.exceptions.ConnectionClosedException;
|
||||
import org.apache.hadoop.hbase.exceptions.ConnectionClosingException;
|
||||
import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.CellBlockMeta;
|
||||
import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.ExceptionResponse;
|
||||
import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.RequestHeader;
|
||||
import org.apache.hadoop.hbase.exceptions.TimeoutIOException;
|
||||
import org.apache.hadoop.hbase.util.Bytes;
|
||||
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
|
||||
import org.apache.hadoop.ipc.RemoteException;
|
||||
import org.apache.yetus.audience.InterfaceAudience;
|
||||
|
||||
import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
|
||||
import org.apache.hbase.thirdparty.com.google.protobuf.CodedOutputStream;
|
||||
import org.apache.hbase.thirdparty.com.google.protobuf.Message;
|
||||
|
||||
import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.CellBlockMeta;
|
||||
import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.ExceptionResponse;
|
||||
import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.RequestHeader;
|
||||
|
||||
/**
|
||||
* Utility to help ipc'ing.
|
||||
|
@ -93,7 +99,7 @@ class IPCUtil {
|
|||
continue;
|
||||
}
|
||||
totalSize += m.getSerializedSize();
|
||||
totalSize += CodedOutputStream.computeRawVarint32Size(m.getSerializedSize());
|
||||
totalSize += CodedOutputStream.computeUInt32SizeNoTag(m.getSerializedSize());
|
||||
}
|
||||
Preconditions.checkArgument(totalSize < Integer.MAX_VALUE);
|
||||
return totalSize;
|
||||
|
@ -153,37 +159,74 @@ class IPCUtil {
|
|||
/**
|
||||
* Takes an Exception and the address we were trying to connect to and return an IOException with
|
||||
* the input exception as the cause. The new exception provides the stack trace of the place where
|
||||
* the exception is thrown and some extra diagnostics information. If the exception is
|
||||
* ConnectException or SocketTimeoutException, return a new one of the same type; Otherwise return
|
||||
* an IOException.
|
||||
* the exception is thrown and some extra diagnostics information.
|
||||
* <p/>
|
||||
* Notice that we will try our best to keep the original exception type when creating a new
|
||||
* exception, especially for the 'connection' exceptions, as it is used to determine whether this
|
||||
* is a network issue or the remote side tells us clearly what is wrong, which is very important
|
||||
* to decide whether to retry. If it is not possible to create a new exception with the same type,
|
||||
* for example, the {@code error} is not an {@link IOException}, an {@link IOException} will be
|
||||
* created.
|
||||
* @param addr target address
|
||||
* @param exception the relevant exception
|
||||
* @param error the relevant exception
|
||||
* @return an exception to throw
|
||||
* @see ClientExceptionsUtil#isConnectionException(Throwable)
|
||||
*/
|
||||
static IOException wrapException(InetSocketAddress addr, Exception exception) {
|
||||
if (exception instanceof ConnectException) {
|
||||
static IOException wrapException(InetSocketAddress addr, Throwable error) {
|
||||
if (error instanceof ConnectException) {
|
||||
// connection refused; include the host:port in the error
|
||||
return (ConnectException) new ConnectException(
|
||||
"Call to " + addr + " failed on connection exception: " + exception).initCause(exception);
|
||||
} else if (exception instanceof SocketTimeoutException) {
|
||||
return (SocketTimeoutException) new SocketTimeoutException(
|
||||
"Call to " + addr + " failed because " + exception).initCause(exception);
|
||||
} else if (exception instanceof ConnectionClosingException) {
|
||||
return (ConnectionClosingException) new ConnectionClosingException(
|
||||
"Call to " + addr + " failed on local exception: " + exception).initCause(exception);
|
||||
} else if (exception instanceof ServerTooBusyException) {
|
||||
return (IOException) new ConnectException(
|
||||
"Call to " + addr + " failed on connection exception: " + error).initCause(error);
|
||||
} else if (error instanceof SocketTimeoutException) {
|
||||
return (IOException) new SocketTimeoutException(
|
||||
"Call to " + addr + " failed because " + error).initCause(error);
|
||||
} else if (error instanceof ConnectionClosingException) {
|
||||
return (IOException) new ConnectionClosingException(
|
||||
"Call to " + addr + " failed on local exception: " + error).initCause(error);
|
||||
} else if (error instanceof ServerTooBusyException) {
|
||||
// we already have address in the exception message
|
||||
return (IOException) exception;
|
||||
} else if (exception instanceof DoNotRetryIOException) {
|
||||
return (IOException) error;
|
||||
} else if (error instanceof DoNotRetryIOException) {
|
||||
// try our best to keep the original exception type
|
||||
try {
|
||||
return (IOException) error.getClass().asSubclass(DoNotRetryIOException.class)
|
||||
.getConstructor(String.class)
|
||||
.newInstance("Call to " + addr + " failed on local exception: " + error).initCause(error);
|
||||
} catch (InstantiationException | IllegalAccessException | IllegalArgumentException
|
||||
| InvocationTargetException | NoSuchMethodException | SecurityException e) {
|
||||
// just ignore, will just new a DoNotRetryIOException instead below
|
||||
}
|
||||
return (IOException) new DoNotRetryIOException(
|
||||
"Call to " + addr + " failed on local exception: " + exception).initCause(exception);
|
||||
} else if (exception instanceof ConnectionClosedException) {
|
||||
return (ConnectionClosedException) exception;
|
||||
} else if (exception instanceof StoppedRpcClientException) {
|
||||
return (StoppedRpcClientException) exception;
|
||||
"Call to " + addr + " failed on local exception: " + error).initCause(error);
|
||||
} else if (error instanceof ConnectionClosedException) {
|
||||
return (IOException) new ConnectionClosedException(
|
||||
"Call to " + addr + " failed on local exception: " + error).initCause(error);
|
||||
} else if (error instanceof CallTimeoutException) {
|
||||
return (IOException) new CallTimeoutException(
|
||||
"Call to " + addr + " failed on local exception: " + error).initCause(error);
|
||||
} else if (error instanceof ClosedChannelException) {
|
||||
// ClosedChannelException does not have a constructor which takes a String but it is a
|
||||
// connection exception so we keep its original type
|
||||
return (IOException) error;
|
||||
} else if (error instanceof TimeoutException) {
|
||||
// TimeoutException is not an IOException, let's convert it to TimeoutIOException.
|
||||
return (IOException) new TimeoutIOException(
|
||||
"Call to " + addr + " failed on local exception: " + error).initCause(error);
|
||||
} else {
|
||||
return (IOException) new IOException(
|
||||
"Call to " + addr + " failed on local exception: " + exception).initCause(exception);
|
||||
// try our best to keep the original exception type
|
||||
if (error instanceof IOException) {
|
||||
try {
|
||||
return (IOException) error.getClass().asSubclass(IOException.class)
|
||||
.getConstructor(String.class)
|
||||
.newInstance("Call to " + addr + " failed on local exception: " + error)
|
||||
.initCause(error);
|
||||
} catch (InstantiationException | IllegalAccessException | IllegalArgumentException
|
||||
| InvocationTargetException | NoSuchMethodException | SecurityException e) {
|
||||
// just ignore, will just new an IOException instead below
|
||||
}
|
||||
}
|
||||
return (IOException) new HBaseIOException(
|
||||
"Call to " + addr + " failed on local exception: " + error).initCause(error);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -17,14 +17,18 @@
|
|||
*/
|
||||
package org.apache.hadoop.hbase.ipc;
|
||||
|
||||
import static org.apache.hadoop.hbase.ipc.IPCUtil.wrapException;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.hamcrest.CoreMatchers.instanceOf;
|
||||
import static org.junit.Assert.assertThat;
|
||||
|
||||
import java.net.ConnectException;
|
||||
import java.lang.reflect.Constructor;
|
||||
import java.lang.reflect.InvocationTargetException;
|
||||
import java.net.InetSocketAddress;
|
||||
import java.net.SocketTimeoutException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.TimeoutException;
|
||||
import org.apache.hadoop.hbase.HBaseClassTestRule;
|
||||
import org.apache.hadoop.hbase.exceptions.ConnectionClosingException;
|
||||
import org.apache.hadoop.hbase.exceptions.ClientExceptionsUtil;
|
||||
import org.apache.hadoop.hbase.exceptions.TimeoutIOException;
|
||||
import org.apache.hadoop.hbase.testclassification.ClientTests;
|
||||
import org.apache.hadoop.hbase.testclassification.SmallTests;
|
||||
import org.junit.ClassRule;
|
||||
|
@ -36,18 +40,66 @@ public class TestIPCUtil {
|
|||
|
||||
@ClassRule
|
||||
public static final HBaseClassTestRule CLASS_RULE =
|
||||
HBaseClassTestRule.forClass(TestIPCUtil.class);
|
||||
HBaseClassTestRule.forClass(TestIPCUtil.class);
|
||||
|
||||
private static Throwable create(Class<? extends Throwable> clazz) throws InstantiationException,
|
||||
IllegalAccessException, InvocationTargetException, NoSuchMethodException {
|
||||
try {
|
||||
Constructor<? extends Throwable> c = clazz.getDeclaredConstructor();
|
||||
c.setAccessible(true);
|
||||
return c.newInstance();
|
||||
} catch (NoSuchMethodException e) {
|
||||
// fall through
|
||||
}
|
||||
|
||||
try {
|
||||
Constructor<? extends Throwable> c = clazz.getDeclaredConstructor(String.class);
|
||||
c.setAccessible(true);
|
||||
return c.newInstance("error");
|
||||
} catch (NoSuchMethodException e) {
|
||||
// fall through
|
||||
}
|
||||
|
||||
try {
|
||||
Constructor<? extends Throwable> c = clazz.getDeclaredConstructor(Throwable.class);
|
||||
c.setAccessible(true);
|
||||
return c.newInstance(new Exception("error"));
|
||||
} catch (NoSuchMethodException e) {
|
||||
// fall through
|
||||
}
|
||||
|
||||
try {
|
||||
Constructor<? extends Throwable> c =
|
||||
clazz.getDeclaredConstructor(String.class, Throwable.class);
|
||||
c.setAccessible(true);
|
||||
return c.newInstance("error", new Exception("error"));
|
||||
} catch (NoSuchMethodException e) {
|
||||
// fall through
|
||||
}
|
||||
|
||||
Constructor<? extends Throwable> c =
|
||||
clazz.getDeclaredConstructor(Throwable.class, Throwable.class);
|
||||
c.setAccessible(true);
|
||||
return c.newInstance(new Exception("error"), "error");
|
||||
}
|
||||
|
||||
/**
|
||||
* See HBASE-21862, it is very important to keep the original exception type for connection
|
||||
* exceptions.
|
||||
*/
|
||||
@Test
|
||||
public void testWrapException() throws Exception {
|
||||
final InetSocketAddress address = InetSocketAddress.createUnresolved("localhost", 0);
|
||||
assertTrue(wrapException(address, new ConnectException()) instanceof ConnectException);
|
||||
assertTrue(
|
||||
wrapException(address, new SocketTimeoutException()) instanceof SocketTimeoutException);
|
||||
assertTrue(wrapException(address, new ConnectionClosingException(
|
||||
"Test AbstractRpcClient#wrapException")) instanceof ConnectionClosingException);
|
||||
assertTrue(
|
||||
wrapException(address, new CallTimeoutException("Test AbstractRpcClient#wrapException"))
|
||||
.getCause() instanceof CallTimeoutException);
|
||||
public void testWrapConnectionException() throws Exception {
|
||||
List<Throwable> exceptions = new ArrayList<>();
|
||||
for (Class<? extends Throwable> clazz : ClientExceptionsUtil.getConnectionExceptionTypes()) {
|
||||
exceptions.add(create(clazz));
|
||||
}
|
||||
InetSocketAddress addr = InetSocketAddress.createUnresolved("127.0.0.1", 12345);
|
||||
for (Throwable exception : exceptions) {
|
||||
if (exception instanceof TimeoutException) {
|
||||
assertThat(IPCUtil.wrapException(addr, exception), instanceOf(TimeoutIOException.class));
|
||||
} else {
|
||||
assertThat(IPCUtil.wrapException(addr, exception), instanceOf(exception.getClass()));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue