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 ar...@apache.org on 2015/06/09 00:45:30 UTC

[1/2] hadoop git commit: HADOOP-12054. RPC client should not retry for InvalidToken exceptions. (Contributed by Varun Saxena)

Repository: hadoop
Updated Branches:
  refs/heads/branch-2 869304dc8 -> 82e772bdb
  refs/heads/trunk 2b2465dfa -> 84ba1a75b


HADOOP-12054. RPC client should not retry for InvalidToken exceptions. (Contributed by Varun Saxena)


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/84ba1a75
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/84ba1a75
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/84ba1a75

Branch: refs/heads/trunk
Commit: 84ba1a75b6bcd696dfc20aeabb6f19cb4eff6011
Parents: 2b2465d
Author: Arpit Agarwal <ar...@apache.org>
Authored: Mon Jun 8 15:37:53 2015 -0700
Committer: Arpit Agarwal <ar...@apache.org>
Committed: Mon Jun 8 15:45:23 2015 -0700

----------------------------------------------------------------------
 hadoop-common-project/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(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/84ba1a75/hadoop-common-project/hadoop-common/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index 79f3178..fa6e4b7 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -837,6 +837,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

http://git-wip-us.apache.org/repos/asf/hadoop/blob/84ba1a75/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java
----------------------------------------------------------------------
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 a86f443..06dc4cb 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;
 
 /**
  * <p>
@@ -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) {

http://git-wip-us.apache.org/repos/asf/hadoop/blob/84ba1a75/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java
----------------------------------------------------------------------
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 b443011..08508ae 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<DummyProtocol> provider =
+        new DefaultFailoverProxyProvider<DummyProtocol>(
+            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.
    */


[2/2] hadoop git commit: HADOOP-12054. RPC client should not retry for InvalidToken exceptions. (Contributed by Varun Saxena)

Posted by ar...@apache.org.
HADOOP-12054. RPC client should not retry for InvalidToken exceptions. (Contributed by Varun Saxena)


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/82e772bd
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/82e772bd
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/82e772bd

Branch: refs/heads/branch-2
Commit: 82e772bdbb153bbfaaf459dfa1bc4dd7ab347d9e
Parents: 869304d
Author: Arpit Agarwal <ar...@apache.org>
Authored: Mon Jun 8 15:37:53 2015 -0700
Committer: Arpit Agarwal <ar...@apache.org>
Committed: Mon Jun 8 15:45:25 2015 -0700

----------------------------------------------------------------------
 hadoop-common-project/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(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/82e772bd/hadoop-common-project/hadoop-common/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index 6b93e54..7bfc5fa 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

http://git-wip-us.apache.org/repos/asf/hadoop/blob/82e772bd/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java
----------------------------------------------------------------------
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 a86f443..06dc4cb 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;
 
 /**
  * <p>
@@ -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) {

http://git-wip-us.apache.org/repos/asf/hadoop/blob/82e772bd/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java
----------------------------------------------------------------------
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 b443011..08508ae 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<DummyProtocol> provider =
+        new DefaultFailoverProxyProvider<DummyProtocol>(
+            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.
    */