HADOOP-19208: [ABFS] Fixing logic to determine HNS nature of account to avoid extra getAcl() calls (#6893)
This commit is contained in:
parent
4f0ee9d67d
commit
51cb858cc8
@ -395,14 +395,21 @@ private synchronized boolean getNamespaceEnabledInformationFromServer(
|
||||
try {
|
||||
LOG.debug("Get root ACL status");
|
||||
getClient().getAclStatus(AbfsHttpConstants.ROOT_PATH, tracingContext);
|
||||
// If getAcl succeeds, namespace is enabled.
|
||||
isNamespaceEnabled = Trilean.getTrilean(true);
|
||||
} catch (AbfsRestOperationException ex) {
|
||||
// Get ACL status is a HEAD request, its response doesn't contain
|
||||
// errorCode
|
||||
// So can only rely on its status code to determine its account type.
|
||||
// Get ACL status is a HEAD request, its response doesn't contain errorCode
|
||||
// So can only rely on its status code to determine account type.
|
||||
if (HttpURLConnection.HTTP_BAD_REQUEST != ex.getStatusCode()) {
|
||||
// If getAcl fails with anything other than 400, namespace is enabled.
|
||||
isNamespaceEnabled = Trilean.getTrilean(true);
|
||||
// Continue to throw exception as earlier.
|
||||
LOG.debug("Failed to get ACL status with non 400. Inferring namespace enabled", ex);
|
||||
throw ex;
|
||||
}
|
||||
// If getAcl fails with 400, namespace is disabled.
|
||||
LOG.debug("Failed to get ACL status with 400. "
|
||||
+ "Inferring namespace disabled and ignoring error", ex);
|
||||
isNamespaceEnabled = Trilean.getTrilean(false);
|
||||
} catch (AzureBlobFileSystemException ex) {
|
||||
throw ex;
|
||||
|
@ -28,6 +28,7 @@
|
||||
|
||||
import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys;
|
||||
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.TrileanConversionException;
|
||||
import org.apache.hadoop.fs.azurebfs.enums.Trilean;
|
||||
import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
|
||||
import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
|
||||
import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
|
||||
@ -67,6 +68,7 @@ public void testGetAclCallOnHnsConfigAbsence() throws Exception {
|
||||
Mockito.doThrow(TrileanConversionException.class)
|
||||
.when(store)
|
||||
.isNamespaceEnabled();
|
||||
store.setNamespaceEnabled(Trilean.UNKNOWN);
|
||||
|
||||
TracingContext tracingContext = getSampleTracingContext(fs, true);
|
||||
Mockito.doReturn(Mockito.mock(AbfsRestOperation.class))
|
||||
|
@ -24,9 +24,11 @@
|
||||
import org.junit.Assume;
|
||||
import org.junit.Test;
|
||||
import org.assertj.core.api.Assertions;
|
||||
import org.mockito.Mockito;
|
||||
|
||||
import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
|
||||
import org.apache.hadoop.fs.FileSystem;
|
||||
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
|
||||
import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
|
||||
import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
|
||||
import org.apache.hadoop.conf.Configuration;
|
||||
@ -34,9 +36,14 @@
|
||||
import org.apache.hadoop.fs.azurebfs.enums.Trilean;
|
||||
import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
|
||||
|
||||
import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
|
||||
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
|
||||
import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
|
||||
import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
import static org.mockito.Mockito.doReturn;
|
||||
import static org.mockito.Mockito.doThrow;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.times;
|
||||
@ -217,4 +224,45 @@ private AbfsClient callAbfsGetIsNamespaceEnabledAndReturnMockAbfsClient()
|
||||
return mockClient;
|
||||
}
|
||||
|
||||
@Test
|
||||
public void ensureGetAclDetermineHnsStatusAccurately() throws Exception {
|
||||
ensureGetAclDetermineHnsStatusAccuratelyInternal(HTTP_BAD_REQUEST,
|
||||
false, false);
|
||||
ensureGetAclDetermineHnsStatusAccuratelyInternal(HTTP_NOT_FOUND,
|
||||
true, true);
|
||||
ensureGetAclDetermineHnsStatusAccuratelyInternal(HTTP_INTERNAL_ERROR,
|
||||
true, true);
|
||||
ensureGetAclDetermineHnsStatusAccuratelyInternal(HTTP_UNAVAILABLE,
|
||||
true, true);
|
||||
}
|
||||
|
||||
private void ensureGetAclDetermineHnsStatusAccuratelyInternal(int statusCode,
|
||||
boolean expectedValue, boolean isExceptionExpected) throws Exception {
|
||||
AzureBlobFileSystemStore store = Mockito.spy(getFileSystem().getAbfsStore());
|
||||
AbfsClient mockClient = mock(AbfsClient.class);
|
||||
store.setNamespaceEnabled(Trilean.UNKNOWN);
|
||||
doReturn(mockClient).when(store).getClient();
|
||||
AbfsRestOperationException ex = new AbfsRestOperationException(
|
||||
statusCode, null, Integer.toString(statusCode), null);
|
||||
doThrow(ex).when(mockClient).getAclStatus(anyString(), any(TracingContext.class));
|
||||
|
||||
if (isExceptionExpected) {
|
||||
try {
|
||||
store.getIsNamespaceEnabled(getTestTracingContext(getFileSystem(), false));
|
||||
Assertions.fail(
|
||||
"Exception Should have been thrown with status code: " + statusCode);
|
||||
} catch (AbfsRestOperationException caughtEx) {
|
||||
Assertions.assertThat(caughtEx.getStatusCode()).isEqualTo(statusCode);
|
||||
Assertions.assertThat(caughtEx.getErrorMessage()).isEqualTo(ex.getErrorMessage());
|
||||
}
|
||||
}
|
||||
// This should not trigger extra getAcl() call in case of exceptions.
|
||||
boolean isHnsEnabled = store.getIsNamespaceEnabled(
|
||||
getTestTracingContext(getFileSystem(), false));
|
||||
Assertions.assertThat(isHnsEnabled).isEqualTo(expectedValue);
|
||||
|
||||
// GetAcl() should be called only once to determine the HNS status.
|
||||
Mockito.verify(mockClient, times(1))
|
||||
.getAclStatus(anyString(), any(TracingContext.class));
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user