Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public class AbfsConfiguration{
private final AbfsServiceType fsConfiguredServiceType;
private final boolean isSecure;
private static final Logger LOG = LoggerFactory.getLogger(AbfsConfiguration.class);
private Trilean isNamespaceEnabled = null;

@StringConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_ACCOUNT_IS_HNS_ENABLED,
DefaultValue = DEFAULT_FS_AZURE_ACCOUNT_IS_HNS_ENABLED)
Expand Down Expand Up @@ -525,8 +526,11 @@ public AbfsConfiguration(final Configuration rawConfig, String accountName)
* @return TRUE/FALSE value if configured, UNKNOWN if not configured.
*/
public Trilean getIsNamespaceEnabledAccount() {
return Trilean.getTrilean(
getString(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, isNamespaceEnabledAccount));
if (isNamespaceEnabled == null) {
isNamespaceEnabled = Trilean.getTrilean(
getString(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, isNamespaceEnabledAccount));
}
return isNamespaceEnabled;
}

/**
Expand Down Expand Up @@ -1524,9 +1528,23 @@ void setMaxBackoffIntervalMilliseconds(int maxBackoffInterval) {
this.maxBackoffInterval = maxBackoffInterval;
}

/**
* Sets the namespace enabled account flag.
*
* @param isNamespaceEnabledAccount boolean value indicating if the account is namespace enabled.
*/
void setIsNamespaceEnabledAccount(boolean isNamespaceEnabledAccount) {
this.isNamespaceEnabled = Trilean.getTrilean(isNamespaceEnabledAccount);
}

/**
* Sets the namespace enabled account flag for testing purposes.
*
* @param isNamespaceEnabledAccount Trilean value indicating if the account is namespace enabled.
*/
@VisibleForTesting
void setIsNamespaceEnabledAccount(String isNamespaceEnabledAccount) {
this.isNamespaceEnabledAccount = isNamespaceEnabledAccount;
void setIsNamespaceEnabledAccountForTesting(Trilean isNamespaceEnabledAccount) {
this.isNamespaceEnabled = isNamespaceEnabledAccount;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ public void initialize(URI uri, Configuration configuration)
* HNS Account Cannot have Blob Endpoint URI.
*/
try {
// This will update namespaceEnable based on getAcl in case config is not set.
// This Information will be stored in abfsConfiguration class.
abfsConfiguration.validateConfiguredServiceType(
tryGetIsNamespaceEnabled(initFSTracingContext));
} catch (InvalidConfigurationValueException ex) {
Expand Down Expand Up @@ -296,7 +298,6 @@ public void initialize(URI uri, Configuration configuration)
}
}
}
getAbfsStore().updateClientWithNamespaceInfo(new TracingContext(initFSTracingContext));

LOG.trace("Initiate check for delegation token manager");
if (UserGroupInformation.isSecurityEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ public class AzureBlobFileSystemStore implements Closeable, ListingSupport {

private final AbfsConfiguration abfsConfiguration;
private Set<String> azureInfiniteLeaseDirSet;
private volatile Trilean isNamespaceEnabled;
private final AuthType authType;
private final UserGroupInformation userGroupInformation;
private final IdentityTransformerInterface identityTransformer;
Expand Down Expand Up @@ -234,8 +233,6 @@ public AzureBlobFileSystemStore(

LOG.trace("AbfsConfiguration init complete");

this.isNamespaceEnabled = abfsConfiguration.getIsNamespaceEnabledAccount();

this.userGroupInformation = UserGroupInformation.getCurrentUser();
this.userName = userGroupInformation.getShortUserName();
LOG.trace("UGI init complete");
Expand Down Expand Up @@ -287,18 +284,6 @@ public AzureBlobFileSystemStore(
"abfs-bounded");
}

/**
* Updates the client with the namespace information.
*
* @param tracingContext the tracing context to be used for the operation
* @throws AzureBlobFileSystemException if an error occurs while updating the client
*/
public void updateClientWithNamespaceInfo(TracingContext tracingContext)
throws AzureBlobFileSystemException {
boolean isNamespaceEnabled = getIsNamespaceEnabled(tracingContext);
AbfsClient.setIsNamespaceEnabled(isNamespaceEnabled);
}

/**
* Checks if the given key in Azure Storage should be stored as a page
* blob instead of block blob.
Expand Down Expand Up @@ -409,32 +394,32 @@ public boolean getIsNamespaceEnabled(TracingContext tracingContext)

private synchronized boolean getNamespaceEnabledInformationFromServer(
final TracingContext tracingContext) throws AzureBlobFileSystemException {
if (isNamespaceEnabled != Trilean.UNKNOWN) {
return isNamespaceEnabled.toBoolean();
if (getAbfsConfiguration().getIsNamespaceEnabledAccount() != Trilean.UNKNOWN) {
return isNamespaceEnabled();
}
try {
LOG.debug("Get root ACL status");
getClient(AbfsServiceType.DFS).getAclStatus(AbfsHttpConstants.ROOT_PATH, tracingContext);
// If getAcl succeeds, namespace is enabled.
isNamespaceEnabled = Trilean.getTrilean(true);
setNamespaceEnabled(true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use true false variable from AbfsHttpConstants

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the boolean value, don't think we need to get it from AbfsHttpConstants.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is optional as you mentioned.

} 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 account type.
if (HttpURLConnection.HTTP_BAD_REQUEST != ex.getStatusCode()) {
// If getAcl fails with anything other than 400, namespace is enabled.
isNamespaceEnabled = Trilean.getTrilean(true);
setNamespaceEnabled(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);
setNamespaceEnabled(false);
} catch (AzureBlobFileSystemException ex) {
throw ex;
}
return isNamespaceEnabled.toBoolean();
return isNamespaceEnabled();
}

/**
Expand All @@ -443,7 +428,7 @@ private synchronized boolean getNamespaceEnabledInformationFromServer(
*/
@VisibleForTesting
boolean isNamespaceEnabled() throws TrileanConversionException {
return this.isNamespaceEnabled.toBoolean();
return getAbfsConfiguration().getIsNamespaceEnabledAccount().toBoolean();
}

@VisibleForTesting
Expand Down Expand Up @@ -2026,9 +2011,8 @@ DataBlocks.BlockFactory getBlockFactory() {
return blockFactory;
}

@VisibleForTesting
void setNamespaceEnabled(Trilean isNamespaceEnabled){
this.isNamespaceEnabled = isNamespaceEnabled;
void setNamespaceEnabled(boolean isNamespaceEnabled){
getAbfsConfiguration().setIsNamespaceEnabledAccount(isNamespaceEnabled);
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidFileSystemPropertyException;
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidUriException;
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.SASTokenProviderException;
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.TrileanConversionException;
import org.apache.hadoop.fs.azurebfs.contracts.services.AppendRequestParameters;
import org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode;
import org.apache.hadoop.fs.azurebfs.contracts.services.ListResultEntrySchema;
Expand Down Expand Up @@ -196,7 +197,6 @@ public abstract class AbfsClient implements Closeable {
private KeepAliveCache keepAliveCache;

private AbfsApacheHttpClient abfsApacheHttpClient;
private static boolean isNamespaceEnabled = false;

/**
* logging the rename failure if metadata is in an incomplete state.
Expand Down Expand Up @@ -442,7 +442,7 @@ protected List<AbfsHttpHeader> createCommonHeaders(ApiVersion xMsVersion) {
requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, xMsVersion.toString()));
requestHeaders.add(new AbfsHttpHeader(ACCEPT_CHARSET, UTF_8));
requestHeaders.add(new AbfsHttpHeader(CONTENT_TYPE, EMPTY_STRING));
requestHeaders.add(new AbfsHttpHeader(USER_AGENT, userAgent));
requestHeaders.add(new AbfsHttpHeader(USER_AGENT, getUserAgent()));
return requestHeaders;
}

Expand Down Expand Up @@ -1322,8 +1322,9 @@ String initializeUserAgent(final AbfsConfiguration abfsConfiguration,
sb.append(abfsConfiguration.getClusterType());

// Add a unique identifier in FNS-Blob user agent string
if (!getIsNamespaceEnabled()
&& abfsConfiguration.getFsConfiguredServiceType() == BLOB) {
// Current filesystem init restricts HNS-Blob combination
// so namespace check not required.
if (BLOB == abfsConfiguration.getFsConfiguredServiceType()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the FNS check removed ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we only support FNS over Blob endpoint. HNS-Blob will fail during init only.
After having offline discussion with the team, I made this change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

sb.append(SEMICOLON)
.append(SINGLE_WHITE_SPACE)
.append(FNS_BLOB_USER_AGENT_IDENTIFIER);
Expand Down Expand Up @@ -1726,18 +1727,15 @@ protected String getUserAgent() {
* Checks if the namespace is enabled.
*
* @return True if the namespace is enabled, false otherwise.
* @throws AzureBlobFileSystemException if the conversion fails.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be AbfsDriverException?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AzureBlobFileSystemException is parent of AbfsDriverException, so fine to throw it like this,

*/
public static boolean getIsNamespaceEnabled() {
return isNamespaceEnabled;
}

/**
* Sets the namespace enabled status.
*
* @param namespaceEnabled True to enable the namespace, false to disable it.
*/
public static void setIsNamespaceEnabled(final boolean namespaceEnabled) {
isNamespaceEnabled = namespaceEnabled;
public boolean getIsNamespaceEnabled() throws AzureBlobFileSystemException {
try {
return getAbfsConfiguration().getIsNamespaceEnabledAccount().toBoolean();
} catch (TrileanConversionException ex) {
LOG.error("Failed to convert namespace enabled account property to boolean", ex);
throw new AbfsDriverException("Failed to determine account type", ex);
}
}

protected boolean isRenameResilience() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1563,9 +1563,11 @@ private Hashtable<String, String> parseCommaSeparatedXmsProperties(String xMsPro
* @param requestHeaders list of headers to be sent with the request
*
* @return client transaction id
* @throws AzureBlobFileSystemException if an error occurs while generating the client transaction id
*/
@VisibleForTesting
public String addClientTransactionIdToHeader(List<AbfsHttpHeader> requestHeaders) {
public String addClientTransactionIdToHeader(List<AbfsHttpHeader> requestHeaders)
throws AzureBlobFileSystemException {
String clientTransactionId = null;
// Set client transaction ID if the namespace and client transaction ID config are enabled.
if (getIsNamespaceEnabled() && getAbfsConfiguration().getIsClientTransactionIdEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ private void setTestUserFs() throws Exception {
conf.setBoolean(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, isHNSEnabled);
this.testUserFs = FileSystem.newInstance(conf);
// Resetting the namespace enabled flag to unknown after file system init.
((AzureBlobFileSystem) testUserFs).getAbfsStore().setNamespaceEnabled(
Trilean.UNKNOWN);
((AzureBlobFileSystem) testUserFs).getAbfsStore()
.getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);
}

private void setTestFsConf(final String fsConfKey,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException;
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;
Expand Down Expand Up @@ -83,11 +82,7 @@ public void testGetAclCallOnHnsConfigAbsence() throws Exception {
AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore());
AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient(AbfsServiceType.DFS));
Mockito.doReturn(client).when(store).getClient(AbfsServiceType.DFS);

Mockito.doThrow(TrileanConversionException.class)
.when(store)
.isNamespaceEnabled();
store.setNamespaceEnabled(Trilean.UNKNOWN);
store.getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);

TracingContext tracingContext = getSampleTracingContext(fs, true);
Mockito.doReturn(Mockito.mock(AbfsRestOperation.class))
Expand Down
Loading