diff --git a/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt b/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt index 56e11457c5..56e1d8f823 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt @@ -15,3 +15,6 @@ HADOOP-7922. Improve some logging for client IPC failovers and StandbyExceptions (todd) HADOOP-7921. StandbyException should extend IOException (todd) + +HADOOP-7928. HA: Client failover policy is incorrectly trying to fail over all + IOExceptions (atm) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java index 5afda59475..a96dc9ee0b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java @@ -341,7 +341,7 @@ public RetryAction shouldRetry(Exception e, int retries, failovers == 0 ? 0 : calculateExponentialTime(delayMillis, failovers, maxDelayBase)); } else if (e instanceof SocketException || - e instanceof IOException) { + (e instanceof IOException && !(e instanceof RemoteException))) { if (isMethodIdempotent) { return RetryAction.FAILOVER_AND_RETRY; } else { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java index b52814cfc1..0a2963f7be 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java @@ -181,7 +181,7 @@ public void testFailoverOnNetworkExceptionIdempotentOperation() assertEquals("impl1", unreliable.succeedsOnceThenFailsReturningString()); try { - assertEquals("impl2", unreliable.succeedsOnceThenFailsReturningString()); + unreliable.succeedsOnceThenFailsReturningString(); fail("should not have succeeded twice"); } catch (IOException e) { // Make sure we *don't* fail over since the first implementation threw an @@ -304,4 +304,26 @@ public void run() { String result = unreliable.failsIfIdentifierDoesntMatch("renamed-impl1"); assertEquals("renamed-impl1", result); } + + /** + * Ensure that normal IO exceptions don't result in a failover. + */ + @Test + public void testExpectedIOException() { + UnreliableInterface unreliable = (UnreliableInterface)RetryProxy + .create(UnreliableInterface.class, + new FlipFlopProxyProvider(UnreliableInterface.class, + new UnreliableImplementation("impl1", TypeOfExceptionToFailWith.REMOTE_EXCEPTION), + new UnreliableImplementation("impl2", TypeOfExceptionToFailWith.UNRELIABLE_EXCEPTION)), + RetryPolicies.failoverOnNetworkException( + RetryPolicies.TRY_ONCE_THEN_FAIL, 10, 1000, 10000)); + + try { + unreliable.failsIfIdentifierDoesntMatch("no-such-identifier"); + fail("Should have thrown *some* exception"); + } catch (Exception e) { + assertTrue("Expected IOE but got " + e.getClass(), + e instanceof IOException); + } + } } \ No newline at end of file diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java index 74a63894d8..185ed2a442 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java @@ -19,6 +19,7 @@ import java.io.IOException; +import org.apache.hadoop.io.retry.UnreliableInterface.UnreliableException; import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.ipc.StandbyException; @@ -37,7 +38,8 @@ public class UnreliableImplementation implements UnreliableInterface { public static enum TypeOfExceptionToFailWith { UNRELIABLE_EXCEPTION, STANDBY_EXCEPTION, - IO_EXCEPTION + IO_EXCEPTION, + REMOTE_EXCEPTION } public UnreliableImplementation() { @@ -95,14 +97,7 @@ public String succeedsOnceThenFailsReturningString() if (succeedsOnceThenFailsCount++ < 1) { return identifier; } else { - switch (exceptionToFailWith) { - case STANDBY_EXCEPTION: - throw new StandbyException(identifier); - case UNRELIABLE_EXCEPTION: - throw new UnreliableException(identifier); - case IO_EXCEPTION: - throw new IOException(identifier); - } + throwAppropriateException(exceptionToFailWith, identifier); return null; } } @@ -113,16 +108,8 @@ public String succeedsTenTimesThenFailsReturningString() if (succeedsTenTimesThenFailsCount++ < 10) { return identifier; } else { - switch (exceptionToFailWith) { - case STANDBY_EXCEPTION: - throw new StandbyException(identifier); - case UNRELIABLE_EXCEPTION: - throw new UnreliableException(identifier); - case IO_EXCEPTION: - throw new IOException(identifier); - default: - throw new RuntimeException(identifier); - } + throwAppropriateException(exceptionToFailWith, identifier); + return null; } } @@ -132,16 +119,8 @@ public String succeedsOnceThenFailsReturningStringIdempotent() if (succeedsOnceThenFailsIdempotentCount++ < 1) { return identifier; } else { - switch (exceptionToFailWith) { - case STANDBY_EXCEPTION: - throw new StandbyException(identifier); - case UNRELIABLE_EXCEPTION: - throw new UnreliableException(identifier); - case IO_EXCEPTION: - throw new IOException(identifier); - default: - throw new RuntimeException(identifier); - } + throwAppropriateException(exceptionToFailWith, identifier); + return null; } } @@ -153,17 +132,24 @@ public String failsIfIdentifierDoesntMatch(String identifier) } else { String message = "expected '" + this.identifier + "' but received '" + identifier + "'"; - switch (exceptionToFailWith) { - case STANDBY_EXCEPTION: - throw new StandbyException(message); - case UNRELIABLE_EXCEPTION: - throw new UnreliableException(message); - case IO_EXCEPTION: - throw new IOException(message); - default: - throw new RuntimeException(message); - } + throwAppropriateException(exceptionToFailWith, message); + return null; } } + private static void throwAppropriateException(TypeOfExceptionToFailWith eType, + String message) throws UnreliableException, StandbyException, IOException { + switch (eType) { + case STANDBY_EXCEPTION: + throw new StandbyException(message); + case UNRELIABLE_EXCEPTION: + throw new UnreliableException(message); + case IO_EXCEPTION: + throw new IOException(message); + case REMOTE_EXCEPTION: + throw new RemoteException(IOException.class.getName(), message); + default: + throw new RuntimeException(message); + } + } }