HADOOP-16580. Disable retry of FailoverOnNetworkExceptionRetry in case of AccessControlException. Contributed by Adam Antal

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/retry/RetryPolicies.java

@@ -38,6 +38,7 @@
 import org.apache.hadoop.ipc.RetriableException;
 import org.apache.hadoop.ipc.StandbyException;
 import org.apache.hadoop.net.ConnectTimeoutException;
+import org.apache.hadoop.security.AccessControlException;
 import org.apache.hadoop.security.token.SecretManager.InvalidToken;
 import org.ietf.jgss.GSSException;
 
@@ -688,6 +689,9 @@ public RetryAction shouldRetry(Exception e, int retries,
       } else if (e instanceof InvalidToken) {
         return new RetryAction(RetryAction.RetryDecision.FAIL, 0,
             "Invalid or Cancelled Token");
+      } else if (e instanceof AccessControlException) {
+        return new RetryAction(RetryAction.RetryDecision.FAIL, 0,
+            "Access denied");
       } else if (e instanceof SocketException
           || (e instanceof IOException && !(e instanceof RemoteException))) {
         if (isIdempotentOrAtMostOnce) {

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/TestRetryProxy.java

@@ -25,6 +25,7 @@
 import org.apache.hadoop.io.retry.UnreliableInterface.UnreliableException;
 import org.apache.hadoop.ipc.ProtocolTranslator;
 import org.apache.hadoop.ipc.RemoteException;
+import org.apache.hadoop.security.AccessControlException;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
@@ -48,6 +49,14 @@
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.*;
 
+/**
+ * TestRetryProxy tests the behaviour of the {@link RetryPolicy} class using
+ * a certain method of {@link UnreliableInterface} implemented by
+ * {@link UnreliableImplementation}.
+ *
+ * Some methods may be sensitive to the {@link Idempotent} annotation
+ * (annotated in {@link UnreliableInterface}).
+ */
 public class TestRetryProxy {
   
   private UnreliableImplementation unreliableImpl;
@@ -348,4 +357,24 @@ public void testNoRetryOnSaslError() throws Exception {
       assertEquals(RetryDecision.FAIL, caughtRetryAction.action);
     }
   }
+
+  @Test
+  public void testNoRetryOnAccessControlException() throws Exception {
+    RetryPolicy policy = mock(RetryPolicy.class);
+    RetryPolicy realPolicy = RetryPolicies.failoverOnNetworkException(5);
+    setupMockPolicy(policy, realPolicy);
+
+    UnreliableInterface unreliable = (UnreliableInterface) RetryProxy.create(
+        UnreliableInterface.class, unreliableImpl, policy);
+
+    try {
+      unreliable.failsWithAccessControlExceptionEightTimes();
+      fail("Should fail");
+    } catch (AccessControlException e) {
+      // expected
+      verify(policy, times(1)).shouldRetry(any(Exception.class), anyInt(),
+          anyInt(), anyBoolean());
+      assertEquals(RetryDecision.FAIL, caughtRetryAction.action);
+    }
+  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableImplementation.java

@@ -23,7 +23,14 @@
 
 import org.apache.hadoop.ipc.RemoteException;
 import org.apache.hadoop.ipc.StandbyException;
+import org.apache.hadoop.security.AccessControlException;
 
+/**
+ * For the usage and purpose of this class see {@link UnreliableInterface}
+ * which this class implements.
+ *
+ * @see UnreliableInterface
+ */
 class UnreliableImplementation implements UnreliableInterface {
 
   private int failsOnceInvocationCount,
@@ -32,6 +39,7 @@ class UnreliableImplementation implements UnreliableInterface {
     failsOnceRemoteExceptionInvocationCount,
     failsTenTimesInvocationCount,
     failsWithSASLExceptionTenTimesInvocationCount,
+    failsWithAccessControlExceptionInvocationCount,
     succeedsOnceThenFailsCount,
     succeedsOnceThenFailsIdempotentCount,
     succeedsTenTimesThenFailsCount;
@@ -123,6 +131,14 @@ public void failsWithSASLExceptionTenTimes() throws SaslException {
     }
   }
 
+  @Override
+  public void failsWithAccessControlExceptionEightTimes()
+      throws AccessControlException {
+    if (failsWithAccessControlExceptionInvocationCount++ < 8) {
+      throw new AccessControlException();
+    }
+  }
+
   @Override
   public String succeedsOnceThenFailsReturningString()
       throws UnreliableException, IOException, StandbyException {

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/retry/UnreliableInterface.java

@@ -24,7 +24,20 @@
 
 import org.apache.hadoop.ipc.RemoteException;
 import org.apache.hadoop.ipc.StandbyException;
+import org.apache.hadoop.security.AccessControlException;
 
+/**
+ * The methods of UnreliableInterface could throw exceptions in a
+ * predefined way. It is currently used for testing {@link RetryPolicy}
+ * and {@link FailoverProxyProvider} classes, but can be potentially used
+ * to test any class's behaviour where an underlying interface or class
+ * may throw exceptions.
+ *
+ * Some methods may be annotated with the {@link Idempotent} annotation.
+ * In order to test those some methods of UnreliableInterface are annotated,
+ * but they are not actually Idempotent functions.
+ *
+ */
 public interface UnreliableInterface {
   
   public static class UnreliableException extends Exception {
@@ -66,6 +79,10 @@ public static class FatalException extends UnreliableException {
 
   void failsWithSASLExceptionTenTimes() throws SaslException;
 
+  @Idempotent
+  void failsWithAccessControlExceptionEightTimes()
+      throws AccessControlException;
+
   public String succeedsOnceThenFailsReturningString()
       throws UnreliableException, StandbyException, IOException;
   @Idempotent