HADOOP-14982. Clients using FailoverOnNetworkExceptionRetry can go into a loop if they're used without authenticating with kerberos in HA env (pbacsko via rkanter)

This commit is contained in:
Robert Kanter 2017-11-16 11:11:19 -08:00
parent 6bf2c30192
commit f2efaf013f
4 changed files with 58 additions and 2 deletions

View File

@ -32,11 +32,14 @@ import java.util.Map.Entry;
import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import javax.security.sasl.SaslException;
import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.ipc.RemoteException;
import org.apache.hadoop.ipc.RetriableException; import org.apache.hadoop.ipc.RetriableException;
import org.apache.hadoop.ipc.StandbyException; import org.apache.hadoop.ipc.StandbyException;
import org.apache.hadoop.net.ConnectTimeoutException; import org.apache.hadoop.net.ConnectTimeoutException;
import org.apache.hadoop.security.token.SecretManager.InvalidToken; import org.apache.hadoop.security.token.SecretManager.InvalidToken;
import org.ietf.jgss.GSSException;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import org.slf4j.Logger; import org.slf4j.Logger;
@ -663,6 +666,11 @@ public class RetryPolicies {
+ retries + ") exceeded maximum allowed (" + maxRetries + ")"); + retries + ") exceeded maximum allowed (" + maxRetries + ")");
} }
if (isSaslFailure(e)) {
return new RetryAction(RetryAction.RetryDecision.FAIL, 0,
"SASL failure");
}
if (e instanceof ConnectException || if (e instanceof ConnectException ||
e instanceof EOFException || e instanceof EOFException ||
e instanceof NoRouteToHostException || e instanceof NoRouteToHostException ||
@ -726,6 +734,18 @@ public class RetryPolicies {
return unwrapped instanceof StandbyException; return unwrapped instanceof StandbyException;
} }
private static boolean isSaslFailure(Exception e) {
Throwable current = e;
do {
if (current instanceof SaslException) {
return true;
}
current = current.getCause();
} while (current != null);
return false;
}
static RetriableException getWrappedRetriableException(Exception e) { static RetriableException getWrappedRetriableException(Exception e) {
if (!(e instanceof RemoteException)) { if (!(e instanceof RemoteException)) {
return null; return null;

View File

@ -39,6 +39,8 @@ import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
import javax.security.sasl.SaslException;
import static org.apache.hadoop.io.retry.RetryPolicies.*; import static org.apache.hadoop.io.retry.RetryPolicies.*;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
@ -326,4 +328,24 @@ public class TestRetryProxy {
assertEquals(InterruptedException.class, e.getCause().getClass()); assertEquals(InterruptedException.class, e.getCause().getClass());
assertEquals("sleep interrupted", e.getCause().getMessage()); assertEquals("sleep interrupted", e.getCause().getMessage());
} }
@Test
public void testNoRetryOnSaslError() 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.failsWithSASLExceptionTenTimes();
fail("Should fail");
} catch (SaslException e) {
// expected
verify(policy, times(1)).shouldRetry(any(Exception.class), anyInt(),
anyInt(), anyBoolean());
assertEquals(RetryDecision.FAIL, caughtRetryAction.action);
}
}
} }

View File

@ -19,6 +19,8 @@ package org.apache.hadoop.io.retry;
import java.io.IOException; import java.io.IOException;
import javax.security.sasl.SaslException;
import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.ipc.RemoteException;
import org.apache.hadoop.ipc.StandbyException; import org.apache.hadoop.ipc.StandbyException;
@ -29,6 +31,7 @@ class UnreliableImplementation implements UnreliableInterface {
failsOnceIOExceptionInvocationCount, failsOnceIOExceptionInvocationCount,
failsOnceRemoteExceptionInvocationCount, failsOnceRemoteExceptionInvocationCount,
failsTenTimesInvocationCount, failsTenTimesInvocationCount,
failsWithSASLExceptionTenTimesInvocationCount,
succeedsOnceThenFailsCount, succeedsOnceThenFailsCount,
succeedsOnceThenFailsIdempotentCount, succeedsOnceThenFailsIdempotentCount,
succeedsTenTimesThenFailsCount; succeedsTenTimesThenFailsCount;
@ -113,6 +116,13 @@ class UnreliableImplementation implements UnreliableInterface {
} }
} }
@Override
public void failsWithSASLExceptionTenTimes() throws SaslException {
if (failsWithSASLExceptionTenTimesInvocationCount ++ < 10) {
throw new SaslException();
}
}
@Override @Override
public String succeedsOnceThenFailsReturningString() public String succeedsOnceThenFailsReturningString()
throws UnreliableException, IOException, StandbyException { throws UnreliableException, IOException, StandbyException {

View File

@ -20,6 +20,8 @@ package org.apache.hadoop.io.retry;
import java.io.IOException; import java.io.IOException;
import javax.security.sasl.SaslException;
import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.ipc.RemoteException;
import org.apache.hadoop.ipc.StandbyException; import org.apache.hadoop.ipc.StandbyException;
@ -62,6 +64,8 @@ public interface UnreliableInterface {
void failsTenTimesThenSucceeds() throws UnreliableException; void failsTenTimesThenSucceeds() throws UnreliableException;
void failsWithSASLExceptionTenTimes() throws SaslException;
public String succeedsOnceThenFailsReturningString() public String succeedsOnceThenFailsReturningString()
throws UnreliableException, StandbyException, IOException; throws UnreliableException, StandbyException, IOException;
@Idempotent @Idempotent