From 82e772bdbb153bbfaaf459dfa1bc4dd7ab347d9e Mon Sep 17 00:00:00 2001 From: Arpit Agarwal Date: Mon, 8 Jun 2015 15:37:53 -0700 Subject: [PATCH] HADOOP-12054. RPC client should not retry for InvalidToken exceptions. (Contributed by Varun Saxena) --- .../hadoop-common/CHANGES.txt | 3 + .../apache/hadoop/io/retry/RetryPolicies.java | 4 + .../java/org/apache/hadoop/ipc/TestIPC.java | 78 ++++++++++++++++--- 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 6b93e54e420..7bfc5faf360 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -353,6 +353,9 @@ Release 2.8.0 - UNRELEASED HADOOP-12052 IPC client downgrades all exception types to IOE, breaks callers trying to use them. (Brahma Reddy Battula via stevel) + HADOOP-12054. RPC client should not retry for InvalidToken exceptions. + (Varun Saxena via Arpit Agarwal) + Release 2.7.1 - UNRELEASED INCOMPATIBLE CHANGES 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 a86f443aca8..06dc4cb67e6 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 @@ -37,6 +37,7 @@ import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.ipc.RetriableException; import org.apache.hadoop.ipc.StandbyException; import org.apache.hadoop.net.ConnectTimeoutException; +import org.apache.hadoop.security.token.SecretManager.InvalidToken; /** *

@@ -575,6 +576,9 @@ public class RetryPolicies { // RetriableException or RetriableException wrapped return new RetryAction(RetryAction.RetryDecision.RETRY, getFailoverOrRetrySleepTime(retries)); + } else if (e instanceof InvalidToken) { + return new RetryAction(RetryAction.RetryDecision.FAIL, 0, + "Invalid or Cancelled Token"); } else if (e instanceof SocketException || (e instanceof IOException && !(e instanceof RemoteException))) { if (isIdempotentOrAtMostOnce) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java index b44301127dd..08508aee7a9 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java @@ -62,6 +62,9 @@ import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.io.IntWritable; import org.apache.hadoop.io.LongWritable; import org.apache.hadoop.io.Writable; +import org.apache.hadoop.io.retry.DefaultFailoverProxyProvider; +import org.apache.hadoop.io.retry.FailoverProxyProvider; +import org.apache.hadoop.io.retry.Idempotent; import org.apache.hadoop.io.retry.RetryPolicies; import org.apache.hadoop.io.retry.RetryProxy; import org.apache.hadoop.ipc.Client.ConnectionId; @@ -71,6 +74,7 @@ import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos.RpcResponseHeaderProto; import org.apache.hadoop.net.ConnectTimeoutException; import org.apache.hadoop.net.NetUtils; import org.apache.hadoop.util.StringUtils; +import org.apache.hadoop.security.token.SecretManager.InvalidToken; import org.apache.log4j.Level; import org.junit.Assert; import org.junit.Assume; @@ -204,18 +208,21 @@ public class TestIPC { this.server = server; this.total = total; } - + + protected Object returnValue(Object value) throws Exception { + if (retry++ < total) { + throw new IOException("Fake IOException"); + } + return value; + } + @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { LongWritable param = new LongWritable(RANDOM.nextLong()); LongWritable value = (LongWritable) client.call(param, NetUtils.getConnectAddress(server), null, null, 0, conf); - if (retry++ < total) { - throw new IOException("Fake IOException"); - } else { - return value; - } + return returnValue(value); } @Override @@ -226,7 +233,26 @@ public class TestIPC { return null; } } - + + private static class TestInvalidTokenHandler extends TestInvocationHandler { + private int invocations = 0; + TestInvalidTokenHandler(Client client, Server server) { + super(client, server, 1); + } + + @Override + protected Object returnValue(Object value) throws Exception { + throw new InvalidToken("Invalid Token"); + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) + throws Throwable { + invocations++; + return super.invoke(proxy, method, args); + } + } + @Test(timeout=60000) public void testSerial() throws IOException, InterruptedException { internalTestSerial(3, false, 2, 5, 100); @@ -1026,7 +1052,8 @@ public class TestIPC { /** A dummy protocol */ private interface DummyProtocol { - public void dummyRun(); + @Idempotent + public void dummyRun() throws IOException; } /** @@ -1065,7 +1092,40 @@ public class TestIPC { server.stop(); } } - + + /** + * Test that there is no retry when invalid token exception is thrown. + * Verfies fix for HADOOP-12054 + */ + @Test(expected = InvalidToken.class) + public void testNoRetryOnInvalidToken() throws IOException { + final Client client = new Client(LongWritable.class, conf); + final TestServer server = new TestServer(1, false); + TestInvalidTokenHandler handler = + new TestInvalidTokenHandler(client, server); + DummyProtocol proxy = (DummyProtocol) Proxy.newProxyInstance( + DummyProtocol.class.getClassLoader(), + new Class[] { DummyProtocol.class }, handler); + FailoverProxyProvider provider = + new DefaultFailoverProxyProvider( + DummyProtocol.class, proxy); + DummyProtocol retryProxy = + (DummyProtocol) RetryProxy.create(DummyProtocol.class, provider, + RetryPolicies.failoverOnNetworkException( + RetryPolicies.TRY_ONCE_THEN_FAIL, 100, 100, 10000, 0)); + + try { + server.start(); + retryProxy.dummyRun(); + } finally { + // Check if dummyRun called only once + Assert.assertEquals(handler.invocations, 1); + Client.setCallIdAndRetryCount(0, 0); + client.stop(); + server.stop(); + } + } + /** * Test if the rpc server gets the default retry count (0) from client. */