[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite
snvijaya commented on a change in pull request #2246: URL: https://github.com/apache/hadoop/pull/2246#discussion_r491097819 ## File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/exceptions/ConcurrentWriteOperationDetectedException.java ## @@ -0,0 +1,32 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.fs.azurebfs.contracts.exceptions; + +/** + * Thrown when a concurrent write operation is detected. + */ +@org.apache.hadoop.classification.InterfaceAudience.Public +@org.apache.hadoop.classification.InterfaceStability.Evolving +public class ConcurrentWriteOperationDetectedException Review comment: Will check on what the scope for lease support is on server and how it can be extended to support this PR scenario when we plan for adopting the new lease related changes in server. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r491095402
##
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java
##
@@ -263,10 +265,102 @@ public AbfsRestOperation deleteFilesystem() throws
AzureBlobFileSystemException
return op;
}
- public AbfsRestOperation createPath(final String path, final boolean isFile,
final boolean overwrite,
- final String permission, final String
umask,
- final boolean isAppendBlob) throws
AzureBlobFileSystemException {
+ public AbfsRestOperation createPath(final String path,
Review comment:
Only update to createPath in AbfsClient with the new iteration is the
new eTag parameter now. Have tried to minimize code changes to existing code
only where needed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r491094362
##
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java
##
@@ -263,10 +265,102 @@ public AbfsRestOperation deleteFilesystem() throws
AzureBlobFileSystemException
return op;
}
- public AbfsRestOperation createPath(final String path, final boolean isFile,
final boolean overwrite,
- final String permission, final String
umask,
- final boolean isAppendBlob) throws
AzureBlobFileSystemException {
+ public AbfsRestOperation createPath(final String path,
+ final boolean isFile,
+ final boolean overwrite,
+ final String permission,
+ final String umask,
+ final boolean isAppendBlob) throws AzureBlobFileSystemException {
+String operation = isFile
+? SASTokenProvider.CREATE_FILE_OPERATION
+: SASTokenProvider.CREATE_DIRECTORY_OPERATION;
+
+// if "fs.azure.enable.conditional.create.overwrite" is enabled,
+// trigger a create with overwrite=false first so that eTag fetch can be
+// avoided for cases when no pre-existing file is present (which is the
+// case with most part of create traffic)
+boolean isFirstAttemptToCreateWithoutOverwrite = false;
+if (isFile && overwrite
+&& abfsConfiguration.isConditionalCreateOverwriteEnabled()) {
+ isFirstAttemptToCreateWithoutOverwrite = true;
+}
+
+AbfsRestOperation op = null;
+// Query builder
+final AbfsUriQueryBuilder abfsUriQueryBuilder =
createDefaultUriQueryBuilder();
+abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE,
+operation.equals(SASTokenProvider.CREATE_FILE_OPERATION)
+? FILE
+: DIRECTORY);
+
+if (isAppendBlob) {
+ abfsUriQueryBuilder.addQuery(QUERY_PARAM_BLOBTYPE, APPEND_BLOB_TYPE);
+}
+
+appendSASTokenToQuery(path, operation, abfsUriQueryBuilder);
+
+try {
+ op = createPathImpl(path, abfsUriQueryBuilder,
+ (isFirstAttemptToCreateWithoutOverwrite ? false : overwrite),
+ permission, umask, null);
+} catch (AbfsRestOperationException e) {
+ if ((e.getStatusCode() == HttpURLConnection.HTTP_CONFLICT)
+ && isFirstAttemptToCreateWithoutOverwrite) {
+// Was the first attempt made to create file without overwrite which
+// failed because there is a pre-existing file.
+// resetting the first attempt flag for readabiltiy
+isFirstAttemptToCreateWithoutOverwrite = false;
+
+// Fetch eTag
+try {
+ op = getPathStatus(path, false);
+} catch (AbfsRestOperationException ex) {
+ if (ex.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) {
+// Is a parallel access case, as file which was found to be
+// present went missing by this request.
+throw new ConcurrentWriteOperationDetectedException(
+"Parallel access to the create path detected. Failing request "
++ "to honor single writer semantics");
+ } else {
+throw ex;
+ }
+}
Review comment:
Updated PR with the recommendation.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r480968643
##
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java
##
@@ -271,10 +272,67 @@ public AbfsRestOperation deleteFilesystem() throws
AzureBlobFileSystemException
return op;
}
- public AbfsRestOperation createPath(final String path, final boolean isFile,
final boolean overwrite,
- final String permission, final String
umask,
- final boolean isAppendBlob) throws
AzureBlobFileSystemException {
+ public AbfsRestOperation createPath(final String path,
+ final boolean isFile,
+ final boolean overwrite,
+ final String permission,
+ final String umask,
+ final boolean isAppendBlob) throws AzureBlobFileSystemException {
+String operation = isFile
+? SASTokenProvider.CREATE_FILE_OPERATION
+: SASTokenProvider.CREATE_DIRECTORY_OPERATION;
+
+// HDFS FS defaults overwrite behaviour to true for create file which leads
+// to majority create API traffic with overwrite=true. In some cases, this
+// will end in race conditions at backend with parallel operations issued
to
+// same path either by means of the customer workload or ABFS driver retry.
+// Disabling the create overwrite default setting to false should
+// significantly reduce the chances for such race conditions.
+boolean isFirstAttemptToCreateWithoutOverwrite = false;
+if (isFile && overwrite
+&& abfsConfiguration.isDefaultCreateOverwriteDisabled()) {
+ isFirstAttemptToCreateWithoutOverwrite = true;
+}
+
+AbfsRestOperation op = null;
+// Query builder
+final AbfsUriQueryBuilder abfsUriQueryBuilder =
createDefaultUriQueryBuilder();
+abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE,
+operation.equals(SASTokenProvider.CREATE_FILE_OPERATION)
+? FILE
+: DIRECTORY);
+if (isAppendBlob) {
+ abfsUriQueryBuilder.addQuery(QUERY_PARAM_BLOBTYPE, APPEND_BLOB_TYPE);
+}
+
+appendSASTokenToQuery(path, operation, abfsUriQueryBuilder);
+
+try {
+ op = createPathImpl(path, abfsUriQueryBuilder,
+ (isFirstAttemptToCreateWithoutOverwrite ? false : overwrite),
+ permission, umask);
+} catch (AbfsRestOperationException e) {
+ if ((e.getStatusCode() == HttpURLConnection.HTTP_CONFLICT)
+ && isFirstAttemptToCreateWithoutOverwrite) {
+isFirstAttemptToCreateWithoutOverwrite = false;
+// was a first attempt made to create without overwrite. Now try again
+// with overwrite now.
+op = createPathImpl(path, abfsUriQueryBuilder, true, permission,
umask);
Review comment:
PR has been modified to include the step to fetch eTag to ensure that
the create overwrite=true retry case is handled too.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r480968575
##
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java
##
@@ -271,10 +272,67 @@ public AbfsRestOperation deleteFilesystem() throws
AzureBlobFileSystemException
return op;
}
- public AbfsRestOperation createPath(final String path, final boolean isFile,
final boolean overwrite,
- final String permission, final String
umask,
- final boolean isAppendBlob) throws
AzureBlobFileSystemException {
+ public AbfsRestOperation createPath(final String path,
+ final boolean isFile,
+ final boolean overwrite,
+ final String permission,
+ final String umask,
+ final boolean isAppendBlob) throws AzureBlobFileSystemException {
+String operation = isFile
+? SASTokenProvider.CREATE_FILE_OPERATION
+: SASTokenProvider.CREATE_DIRECTORY_OPERATION;
+
+// HDFS FS defaults overwrite behaviour to true for create file which leads
+// to majority create API traffic with overwrite=true. In some cases, this
+// will end in race conditions at backend with parallel operations issued
to
+// same path either by means of the customer workload or ABFS driver retry.
+// Disabling the create overwrite default setting to false should
+// significantly reduce the chances for such race conditions.
+boolean isFirstAttemptToCreateWithoutOverwrite = false;
+if (isFile && overwrite
+&& abfsConfiguration.isDefaultCreateOverwriteDisabled()) {
+ isFirstAttemptToCreateWithoutOverwrite = true;
+}
+
+AbfsRestOperation op = null;
+// Query builder
+final AbfsUriQueryBuilder abfsUriQueryBuilder =
createDefaultUriQueryBuilder();
+abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE,
+operation.equals(SASTokenProvider.CREATE_FILE_OPERATION)
+? FILE
+: DIRECTORY);
+if (isAppendBlob) {
+ abfsUriQueryBuilder.addQuery(QUERY_PARAM_BLOBTYPE, APPEND_BLOB_TYPE);
+}
+
+appendSASTokenToQuery(path, operation, abfsUriQueryBuilder);
+
+try {
+ op = createPathImpl(path, abfsUriQueryBuilder,
Review comment:
Yes, this PR change is not an absolute fix to resolve the race condition
issue. But will help in reducing the overall create overwrite=true traffic with
HDFS default for overwrite being true.
##
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java
##
@@ -271,10 +272,67 @@ public AbfsRestOperation deleteFilesystem() throws
AzureBlobFileSystemException
return op;
}
- public AbfsRestOperation createPath(final String path, final boolean isFile,
final boolean overwrite,
- final String permission, final String
umask,
- final boolean isAppendBlob) throws
AzureBlobFileSystemException {
+ public AbfsRestOperation createPath(final String path,
+ final boolean isFile,
+ final boolean overwrite,
+ final String permission,
+ final String umask,
+ final boolean isAppendBlob) throws AzureBlobFileSystemException {
+String operation = isFile
+? SASTokenProvider.CREATE_FILE_OPERATION
+: SASTokenProvider.CREATE_DIRECTORY_OPERATION;
+
+// HDFS FS defaults overwrite behaviour to true for create file which leads
+// to majority create API traffic with overwrite=true. In some cases, this
+// will end in race conditions at backend with parallel operations issued
to
+// same path either by means of the customer workload or ABFS driver retry.
+// Disabling the create overwrite default setting to false should
+// significantly reduce the chances for such race conditions.
+boolean isFirstAttemptToCreateWithoutOverwrite = false;
+if (isFile && overwrite
+&& abfsConfiguration.isDefaultCreateOverwriteDisabled()) {
+ isFirstAttemptToCreateWithoutOverwrite = true;
+}
+
+AbfsRestOperation op = null;
+// Query builder
+final AbfsUriQueryBuilder abfsUriQueryBuilder =
createDefaultUriQueryBuilder();
+abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE,
+operation.equals(SASTokenProvider.CREATE_FILE_OPERATION)
+? FILE
+: DIRECTORY);
+if (isAppendBlob) {
+ abfsUriQueryBuilder.addQuery(QUERY_PARAM_BLOBTYPE, APPEND_BLOB_TYPE);
+}
+
+appendSASTokenToQuery(path, operation, abfsUriQueryBuilder);
+
+try {
+ op = createPathImpl(path, abfsUriQueryBuilder,
+ (isFirstAttemptToCreateWithoutOverwrite ? false : overwrite),
+ permission, umask);
+} catch (AbfsRestOperationException e) {
+ if ((e.getStatusCode() == HttpURLConnection.HTTP_CONFLICT)
+ && isFirstAttemptToCreateWithoutOverwrite) {
+
[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r480967391
##
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java
##
@@ -271,10 +272,67 @@ public AbfsRestOperation deleteFilesystem() throws
AzureBlobFileSystemException
return op;
}
- public AbfsRestOperation createPath(final String path, final boolean isFile,
final boolean overwrite,
- final String permission, final String
umask,
- final boolean isAppendBlob) throws
AzureBlobFileSystemException {
+ public AbfsRestOperation createPath(final String path,
+ final boolean isFile,
+ final boolean overwrite,
+ final String permission,
+ final String umask,
+ final boolean isAppendBlob) throws AzureBlobFileSystemException {
+String operation = isFile
+? SASTokenProvider.CREATE_FILE_OPERATION
+: SASTokenProvider.CREATE_DIRECTORY_OPERATION;
+
+// HDFS FS defaults overwrite behaviour to true for create file which leads
+// to majority create API traffic with overwrite=true. In some cases, this
+// will end in race conditions at backend with parallel operations issued
to
+// same path either by means of the customer workload or ABFS driver retry.
+// Disabling the create overwrite default setting to false should
+// significantly reduce the chances for such race conditions.
+boolean isFirstAttemptToCreateWithoutOverwrite = false;
+if (isFile && overwrite
+&& abfsConfiguration.isDefaultCreateOverwriteDisabled()) {
+ isFirstAttemptToCreateWithoutOverwrite = true;
+}
+
+AbfsRestOperation op = null;
+// Query builder
+final AbfsUriQueryBuilder abfsUriQueryBuilder =
createDefaultUriQueryBuilder();
+abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE,
+operation.equals(SASTokenProvider.CREATE_FILE_OPERATION)
+? FILE
+: DIRECTORY);
+if (isAppendBlob) {
+ abfsUriQueryBuilder.addQuery(QUERY_PARAM_BLOBTYPE, APPEND_BLOB_TYPE);
+}
+
+appendSASTokenToQuery(path, operation, abfsUriQueryBuilder);
+
+try {
+ op = createPathImpl(path, abfsUriQueryBuilder,
+ (isFirstAttemptToCreateWithoutOverwrite ? false : overwrite),
+ permission, umask);
+} catch (AbfsRestOperationException e) {
+ if ((e.getStatusCode() == HttpURLConnection.HTTP_CONFLICT)
Review comment:
In the driver code, we try to maintain all checks with HTTP status code.
HTTP_CONFLICT is possible only for operation on an already existing target.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r477244840
##
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java
##
@@ -271,10 +272,62 @@ public AbfsRestOperation deleteFilesystem() throws
AzureBlobFileSystemException
return op;
}
- public AbfsRestOperation createPath(final String path, final boolean isFile,
final boolean overwrite,
- final String permission, final String
umask,
- final boolean isAppendBlob) throws
AzureBlobFileSystemException {
+ public AbfsRestOperation createPath(final String path,
+ final boolean isFile,
+ final boolean overwrite,
+ final String permission,
+ final String umask,
+ final boolean isAppendBlob) throws AzureBlobFileSystemException {
+boolean isFirstAttemptToCreateWithoutOverwrite = false;
+
+String operation = isFile
+? SASTokenProvider.CREATE_FILE_OPERATION
+: SASTokenProvider.CREATE_DIRECTORY_OPERATION;
+
+// attemptFileCreateWithoutOverwriteFirst
+if (isFile && overwrite
+&& abfsConfiguration.isDefaultCreateOverwriteDisabled()) {
+ isFirstAttemptToCreateWithoutOverwrite = true;
+}
+
+AbfsRestOperation op = null;
+// Query builder
+final AbfsUriQueryBuilder abfsUriQueryBuilder =
createDefaultUriQueryBuilder();
+abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE,
+operation.equals(SASTokenProvider.CREATE_FILE_OPERATION)
+? FILE
+: DIRECTORY);
+if (isAppendBlob) {
+ abfsUriQueryBuilder.addQuery(QUERY_PARAM_BLOBTYPE, APPEND_BLOB_TYPE);
+}
+
+appendSASTokenToQuery(path, operation, abfsUriQueryBuilder);
+
+try {
+ op = createPath(path, abfsUriQueryBuilder,
+ (isFirstAttemptToCreateWithoutOverwrite ? false : overwrite),
+ permission, umask);
+} catch (AbfsRestOperationException e) {
+ if ((e.getStatusCode() == HttpURLConnection.HTTP_CONFLICT)
+ && isFirstAttemptToCreateWithoutOverwrite) {
+// was a first attempt made to create without overwrite. Now try again
+// with overwrite now.
Review comment:
Done
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r477243460
##
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java
##
@@ -271,10 +272,62 @@ public AbfsRestOperation deleteFilesystem() throws
AzureBlobFileSystemException
return op;
}
- public AbfsRestOperation createPath(final String path, final boolean isFile,
final boolean overwrite,
- final String permission, final String
umask,
- final boolean isAppendBlob) throws
AzureBlobFileSystemException {
+ public AbfsRestOperation createPath(final String path,
+ final boolean isFile,
+ final boolean overwrite,
+ final String permission,
+ final String umask,
+ final boolean isAppendBlob) throws AzureBlobFileSystemException {
+boolean isFirstAttemptToCreateWithoutOverwrite = false;
+
+String operation = isFile
+? SASTokenProvider.CREATE_FILE_OPERATION
+: SASTokenProvider.CREATE_DIRECTORY_OPERATION;
+
+// attemptFileCreateWithoutOverwriteFirst
+if (isFile && overwrite
+&& abfsConfiguration.isDefaultCreateOverwriteDisabled()) {
+ isFirstAttemptToCreateWithoutOverwrite = true;
+}
+
+AbfsRestOperation op = null;
+// Query builder
+final AbfsUriQueryBuilder abfsUriQueryBuilder =
createDefaultUriQueryBuilder();
+abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE,
+operation.equals(SASTokenProvider.CREATE_FILE_OPERATION)
+? FILE
+: DIRECTORY);
+if (isAppendBlob) {
+ abfsUriQueryBuilder.addQuery(QUERY_PARAM_BLOBTYPE, APPEND_BLOB_TYPE);
+}
+
+appendSASTokenToQuery(path, operation, abfsUriQueryBuilder);
+
+try {
+ op = createPath(path, abfsUriQueryBuilder,
+ (isFirstAttemptToCreateWithoutOverwrite ? false : overwrite),
+ permission, umask);
+} catch (AbfsRestOperationException e) {
+ if ((e.getStatusCode() == HttpURLConnection.HTTP_CONFLICT)
+ && isFirstAttemptToCreateWithoutOverwrite) {
+// was a first attempt made to create without overwrite. Now try again
+// with overwrite now.
Review comment:
Its leading to an error that its a unused variable set. Hence have
avoided reset which was recommended for readability.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
