[
https://issues.apache.org/jira/browse/HADOOP-19137?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17850820#comment-17850820
]
ASF GitHub Bot commented on HADOOP-19137:
-----------------------------------------
steveloughran commented on code in PR #6752:
URL: https://github.com/apache/hadoop/pull/6752#discussion_r1621197499
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java:
##########
@@ -367,16 +366,35 @@ private String[] authorityParts(URI uri) throws
InvalidUriAuthorityException, In
public boolean getIsNamespaceEnabled(TracingContext tracingContext)
throws AzureBlobFileSystemException {
try {
- return this.isNamespaceEnabled.toBoolean();
+ return getNamespaceConfig();
} catch (TrileanConversionException e) {
LOG.debug("isNamespaceEnabled is UNKNOWN; fall back and determine
through"
+ " getAcl server call", e);
}
- isNamespaceEnabled =
Trilean.getTrilean(NamespaceUtil.isNamespaceEnabled(client, tracingContext));
+ try {
+ LOG.debug("Get root ACL status");
+ getClient().getAclStatus(AbfsHttpConstants.ROOT_PATH, tracingContext);
+ 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.
+ if (HttpURLConnection.HTTP_BAD_REQUEST != ex.getStatusCode()) {
+ throw ex;
+ }
+ isNamespaceEnabled = Trilean.getTrilean(false);
+ } catch (AzureBlobFileSystemException ex) {
+ throw ex;
+ }
return isNamespaceEnabled.toBoolean();
}
+ @VisibleForTesting
+ boolean getNamespaceConfig() throws TrileanConversionException {
Review Comment:
1. The name here should just be `isNamespaceEnabled`; getNamespaceConfig
implies there's more information coming back
2. the javadocs *which are needed* should explain why the exception is raised
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java:
##########
@@ -165,5 +169,20 @@ public static ApiVersion getCurrentVersion() {
*/
public static final Integer HTTP_STATUS_CATEGORY_QUOTIENT = 100;
+ private static final String CPK_CONFIG_LIST =
Review Comment:
again, javadoc with + {@value}. for the IDEs, more than the docs themselves
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java:
##########
@@ -367,16 +366,35 @@ private String[] authorityParts(URI uri) throws
InvalidUriAuthorityException, In
public boolean getIsNamespaceEnabled(TracingContext tracingContext)
throws AzureBlobFileSystemException {
try {
- return this.isNamespaceEnabled.toBoolean();
+ return getNamespaceConfig();
} catch (TrileanConversionException e) {
LOG.debug("isNamespaceEnabled is UNKNOWN; fall back and determine
through"
+ " getAcl server call", e);
}
- isNamespaceEnabled =
Trilean.getTrilean(NamespaceUtil.isNamespaceEnabled(client, tracingContext));
+ try {
+ LOG.debug("Get root ACL status");
Review Comment:
I think this should be synchronized, with the first things in the sync block
checking for the isNamespaceEnabled flag to see if it is now valid.
Lots of fs operations do this check, and so you could have multiple threads
all triggering duplicate HTTP requests.
also, mark the field as volatile.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java:
##########
@@ -367,16 +366,35 @@ private String[] authorityParts(URI uri) throws
InvalidUriAuthorityException, In
public boolean getIsNamespaceEnabled(TracingContext tracingContext)
Review Comment:
add javadoc, make clear this will issue HTTP requests to resolve the status,
if it has not already taken place -and that threads will block if another
thread is working on this
> [ABFS]:Extra getAcl call while calling the very first API of FileSystem
> -----------------------------------------------------------------------
>
> Key: HADOOP-19137
> URL: https://issues.apache.org/jira/browse/HADOOP-19137
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure
> Affects Versions: 3.4.0
> Reporter: Pranav Saxena
> Assignee: Pranav Saxena
> Priority: Major
> Labels: pull-request-available
>
> Store doesn't flow in the namespace information to the client.
> In https://github.com/apache/hadoop/pull/6221, getIsNamespaceEnabled is added
> in client methods which checks if namespace information is there or not, and
> if not there, it will make getAcl call and set the field. Once the field is
> set, it would be used in future getIsNamespaceEnabled method calls for a
> given AbfsClient.
> Since, CPK both global and encryptionContext are only for hns account, the
> fix that is proposed is that we would fail fs init if its non-hns account and
> cpk config is given.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
