You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by to...@apache.org on 2012/02/15 19:20:12 UTC
svn commit: r1244628 - in
/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common: ./
src/main/java/org/apache/hadoop/io/retry/
src/test/java/org/apache/hadoop/io/retry/
Author: todd
Date: Wed Feb 15 18:20:11 2012
New Revision: 1244628
URL: http://svn.apache.org/viewvc?rev=1244628&view=rev
Log:
HADOOP-8068. void methods can swallow exceptions when going through failover path. Contributed by Todd Lipcon.
Modified:
hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt
hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryInvocationHandler.java
hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java
hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicy.java
hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java
hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestRetryProxy.java
hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java
hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableInterface.java
Modified: hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt?rev=1244628&r1=1244627&r2=1244628&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt (original)
+++ hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt Wed Feb 15 18:20:11 2012
@@ -47,3 +47,5 @@ HADOOP-8038. Add 'ipc.client.connect.max
core-default.xml file. (Uma Maheswara Rao G via atm)
HADOOP-8041. Log a warning when a failover is first attempted (todd)
+
+HADOOP-8068. void methods can swallow exceptions when going through failover path (todd)
Modified: hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryInvocationHandler.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryInvocationHandler.java?rev=1244628&r1=1244627&r2=1244628&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryInvocationHandler.java (original)
+++ hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryInvocationHandler.java Wed Feb 15 18:20:11 2012
@@ -90,12 +90,12 @@ class RetryInvocationHandler implements
RetryAction action = policy.shouldRetry(e, retries++, invocationFailoverCount,
isMethodIdempotent);
if (action.action == RetryAction.RetryDecision.FAIL) {
- LOG.warn("Exception while invoking " + method.getName()
- + " of " + currentProxy.getClass() + ". Not retrying.", e);
- if (!method.getReturnType().equals(Void.TYPE)) {
- throw e; // non-void methods can't fail without an exception
+ if (action.reason != null) {
+ LOG.warn("Exception while invoking " +
+ currentProxy.getClass() + "." + method.getName() +
+ ". Not retrying because " + action.reason, e);
}
- return null;
+ throw e;
} else { // retry or failover
// avoid logging the failover if this is the first call on this
// proxy object, and we successfully achieve the failover without
Modified: hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java?rev=1244628&r1=1244627&r2=1244628&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java (original)
+++ hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java Wed Feb 15 18:20:11 2012
@@ -56,14 +56,6 @@ public class RetryPolicies {
/**
* <p>
- * Try once, and fail silently for <code>void</code> methods, or by
- * re-throwing the exception for non-<code>void</code> methods.
- * </p>
- */
- public static final RetryPolicy TRY_ONCE_DONT_FAIL = new TryOnceDontFail();
-
- /**
- * <p>
* Keep trying forever.
* </p>
*/
@@ -154,12 +146,6 @@ public class RetryPolicies {
static class TryOnceThenFail implements RetryPolicy {
public RetryAction shouldRetry(Exception e, int retries, int failovers,
boolean isMethodIdempotent) throws Exception {
- throw e;
- }
- }
- static class TryOnceDontFail implements RetryPolicy {
- public RetryAction shouldRetry(Exception e, int retries, int failovers,
- boolean isMethodIdempotent) throws Exception {
return RetryAction.FAIL;
}
}
@@ -185,7 +171,7 @@ public class RetryPolicies {
public RetryAction shouldRetry(Exception e, int retries, int failovers,
boolean isMethodIdempotent) throws Exception {
if (retries >= maxRetries) {
- throw e;
+ return RetryAction.FAIL;
}
return new RetryAction(RetryAction.RetryDecision.RETRY,
timeUnit.toMillis(calculateSleepTime(retries)));
@@ -325,9 +311,9 @@ public class RetryPolicies {
public RetryAction shouldRetry(Exception e, int retries,
int failovers, boolean isMethodIdempotent) throws Exception {
if (failovers >= maxFailovers) {
- LOG.info("Failovers (" + failovers + ") exceeded maximum allowed ("
+ return new RetryAction(RetryAction.RetryDecision.FAIL, 0,
+ "failovers (" + failovers + ") exceeded maximum allowed ("
+ maxFailovers + ")");
- return RetryAction.FAIL;
}
if (e instanceof ConnectException ||
@@ -345,7 +331,9 @@ public class RetryPolicies {
if (isMethodIdempotent) {
return RetryAction.FAILOVER_AND_RETRY;
} else {
- return RetryAction.FAIL;
+ return new RetryAction(RetryAction.RetryDecision.FAIL, 0,
+ "the invoked method is not idempotent, and unable to determine " +
+ "whether it was invoked");
}
} else {
return fallbackPolicy.shouldRetry(e, retries, failovers,
Modified: hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicy.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicy.java?rev=1244628&r1=1244627&r2=1244628&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicy.java (original)
+++ hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicy.java Wed Feb 15 18:20:11 2012
@@ -44,14 +44,20 @@ public interface RetryPolicy {
public final RetryDecision action;
public final long delayMillis;
+ public final String reason;
public RetryAction(RetryDecision action) {
- this(action, 0);
+ this(action, 0, null);
}
public RetryAction(RetryDecision action, long delayTime) {
+ this(action, delayTime, null);
+ }
+
+ public RetryAction(RetryDecision action, long delayTime, String reason) {
this.action = action;
this.delayMillis = delayTime;
+ this.reason = reason;
}
public enum RetryDecision {
Modified: hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java?rev=1244628&r1=1244627&r2=1244628&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java (original)
+++ hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestFailoverProxy.java Wed Feb 15 18:20:11 2012
@@ -128,7 +128,7 @@ public class TestFailoverProxy {
new FlipFlopProxyProvider(UnreliableInterface.class,
new UnreliableImplementation("impl1"),
new UnreliableImplementation("impl2")),
- RetryPolicies.TRY_ONCE_DONT_FAIL);
+ RetryPolicies.TRY_ONCE_THEN_FAIL);
unreliable.succeedsOnceThenFailsReturningString();
try {
@@ -196,6 +196,27 @@ public class TestFailoverProxy {
assertEquals("impl2", unreliable.succeedsOnceThenFailsReturningStringIdempotent());
}
+ /**
+ * Test that if a non-idempotent void function is called, and there is an exception,
+ * the exception is properly propagated
+ */
+ @Test
+ public void testExceptionPropagatedForNonIdempotentVoid() throws Exception {
+ UnreliableInterface unreliable = (UnreliableInterface)RetryProxy
+ .create(UnreliableInterface.class,
+ new FlipFlopProxyProvider(UnreliableInterface.class,
+ new UnreliableImplementation("impl1", TypeOfExceptionToFailWith.IO_EXCEPTION),
+ new UnreliableImplementation("impl2", TypeOfExceptionToFailWith.UNRELIABLE_EXCEPTION)),
+ RetryPolicies.failoverOnNetworkException(1));
+
+ try {
+ unreliable.nonIdempotentVoidFailsIfIdentifierDoesntMatch("impl2");
+ fail("did not throw an exception");
+ } catch (Exception e) {
+ }
+
+ }
+
private static class SynchronizedUnreliableImplementation extends UnreliableImplementation {
private CountDownLatch methodLatch;
Modified: hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestRetryProxy.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestRetryProxy.java?rev=1244628&r1=1244627&r2=1244628&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestRetryProxy.java (original)
+++ hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestRetryProxy.java Wed Feb 15 18:20:11 2012
@@ -19,7 +19,6 @@
package org.apache.hadoop.io.retry;
import static org.apache.hadoop.io.retry.RetryPolicies.RETRY_FOREVER;
-import static org.apache.hadoop.io.retry.RetryPolicies.TRY_ONCE_DONT_FAIL;
import static org.apache.hadoop.io.retry.RetryPolicies.TRY_ONCE_THEN_FAIL;
import static org.apache.hadoop.io.retry.RetryPolicies.retryByException;
import static org.apache.hadoop.io.retry.RetryPolicies.retryByRemoteException;
@@ -59,19 +58,6 @@ public class TestRetryProxy extends Test
}
}
- public void testTryOnceDontFail() throws UnreliableException {
- UnreliableInterface unreliable = (UnreliableInterface)
- RetryProxy.create(UnreliableInterface.class, unreliableImpl, TRY_ONCE_DONT_FAIL);
- unreliable.alwaysSucceeds();
- unreliable.failsOnceThenSucceeds();
- try {
- unreliable.failsOnceThenSucceedsWithReturnValue();
- fail("Should fail");
- } catch (UnreliableException e) {
- // expected
- }
- }
-
public void testRetryForever() throws UnreliableException {
UnreliableInterface unreliable = (UnreliableInterface)
RetryProxy.create(UnreliableInterface.class, unreliableImpl, RETRY_FOREVER);
Modified: hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java?rev=1244628&r1=1244627&r2=1244628&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java (original)
+++ hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java Wed Feb 15 18:20:11 2012
@@ -136,6 +136,18 @@ public class UnreliableImplementation im
return null;
}
}
+
+ @Override
+ public void nonIdempotentVoidFailsIfIdentifierDoesntMatch(String identifier)
+ throws UnreliableException, StandbyException, IOException {
+ if (this.identifier.equals(identifier)) {
+ return;
+ } else {
+ String message = "expected '" + this.identifier + "' but received '" +
+ identifier + "'";
+ throwAppropriateException(exceptionToFailWith, message);
+ }
+ }
private static void throwAppropriateException(TypeOfExceptionToFailWith eType,
String message) throws UnreliableException, StandbyException, IOException {
Modified: hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableInterface.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableInterface.java?rev=1244628&r1=1244627&r2=1244628&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableInterface.java (original)
+++ hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableInterface.java Wed Feb 15 18:20:11 2012
@@ -67,4 +67,7 @@ public interface UnreliableInterface {
@Idempotent
public String failsIfIdentifierDoesntMatch(String identifier)
throws UnreliableException, StandbyException, IOException;
+
+ void nonIdempotentVoidFailsIfIdentifierDoesntMatch(String identifier)
+ throws UnreliableException, StandbyException, IOException;
}