You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2020/08/26 04:57:24 UTC

[GitHub] [hadoop] snvijaya opened a new pull request #2246: Hadoop 17215: Disable default create overwrite

snvijaya opened a new pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246


   ## NOTICE
   
   Please create an issue in ASF JIRA before opening a pull request,
   and you need to set the title of the pull request which starts with
   the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
   For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] DadanielZ commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r480281889



##########
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:
       it is more likely the race condition can still happen in the retry process for this call.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] DadanielZ commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
DadanielZ commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-683922533


   If the main purpose is to resolve the potential race condition, this fix seems not enough as the race condition can still happen in the retry process for the second "create with override" call in catch logic. And we introduce an extra call for create operation.
   @snvijaya Is this PR tested and proven to be helpful?


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya edited a comment on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya edited a comment on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-680836914


   Test results from accounts in East US 2.
   
   From HNS Enabled account with OAuth credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 74
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   
   From HNS Enabled account with SharedKey credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 41
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   From HNS Non-enabled account with OAuth credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 250
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   
   From HNS Non-enabled account with SharedKey credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 246
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   The fix for the failing test is tracked under: https://issues.apache.org/jira/browse/HADOOP-17229


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ThomasMarquardt commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
ThomasMarquardt commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r489708814



##########
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:
       Seems we should be doing something like https://issues.apache.org/jira/browse/HADOOP-16948 instead.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop 17215: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-680682210


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 35s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  32m 15s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 38s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   0m 24s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  17m 35s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 27s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m  8s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m  5s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 33s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |   0m 29s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-azure: The patch generated 2 new + 5 unchanged - 0 fixed = 7 total (was 5)  |
   | +1 :green_heart: |  mvnsite  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  16m 48s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 27s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   1m 10s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 35s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  80m 26s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2246 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 6b7ce80be716 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 6a49bf9bffb |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | checkstyle | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/1/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/1/testReport/ |
   | Max. process+thread count | 424 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/1/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-680862405


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  31m 59s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 38s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |   0m 30s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   0m 23s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 43s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 31s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m  2s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 59s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-azure: The patch generated 2 new + 5 unchanged - 0 fixed = 7 total (was 5)  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  14m 35s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   1m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 30s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 38s |  The patch does not generate ASF License warnings.  |
   |  |   |  75m 42s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2246 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 20928ef8a0c1 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 75db5526b5d |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | checkstyle | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/4/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/4/testReport/ |
   | Max. process+thread count | 421 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/4/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ThomasMarquardt commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
ThomasMarquardt commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-695146807


   commit e31a636e922a8fdbe0aa7cca53f6de7175e97254
   Author: Sneha Vijayarajan <sn...@gmail.com>
   Date:   Wed Aug 26 00:31:35 2020 +0530
   
       HADOOP-17215: Support for conditional overwrite.
   
       Contributed by Sneha Vijayarajan
   
       DETAILS:
   
           This change adds config key "fs.azure.enable.conditional.create.overwrite" with
           a default of true.  When enabled, if create(path, overwrite: true) is invoked
           and the file exists, the ABFS driver will first obtain its etag and then attempt
           to overwrite the file on the condition that the etag matches. The purpose of this
           is to mitigate the non-idempotency of this method.  Specifically, in the event of
           a network error or similar, the client will retry and this can result in the file
           being created more than once which may result in data loss.  In essense this is
           like a poor man's file handle, and will be addressed more thoroughly in the future
           when support for lease is added to ABFS.
   
       TEST RESULTS:
   
           namespace.enabled=true
           auth.type=SharedKey
           -------------------
           $mvn -T 1C -Dparallel-tests=abfs -Dscale -DtestsThreadCount=8 clean verify
           Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
           Tests run: 457, Failures: 0, Errors: 0, Skipped: 42
           Tests run: 207, Failures: 0, Errors: 0, Skipped: 24
   
           namespace.enabled=true
           auth.type=OAuth
           -------------------
           $mvn -T 1C -Dparallel-tests=abfs -Dscale -DtestsThreadCount=8 clean verify
           Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
           Tests run: 457, Failures: 0, Errors: 0, Skipped: 74
           Tests run: 207, Failures: 0, Errors: 0, Skipped: 140
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop 17215: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r477040596



##########
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:
       Reset isFirstAttemptToCreateWithoutOverwrite 




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] shanemkm commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
shanemkm commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r480718118



##########
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:
       In createPathImpl, it will go through retries with backoff inside of that, correct? Just want to make sure since our observation was that those retries almost always succeed, hence why this change would remove the race condition/stale write requests almost entirely




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop 17215: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r477040222



##########
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;

Review comment:
       Move to line 287




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] DadanielZ commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r480281889



##########
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:
       it is more like the race condition can still happen in the retry process for this call.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
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) {
+        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:
       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.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-691103774






----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ThomasMarquardt commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
ThomasMarquardt commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r489647598



##########
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:
       The original design was such that AbfsClient is a thin client over the REST API, and AzureBlobFileSystemStore is where the app logic lived, such as handling continuation tokens or making multiple calls such as getting the etag to use it in a conditional request.  I think we should stick to the original design and move this fancy logic to AzureBlobFileSystemStore and expose an option on AbfsClient to take an optional condition (the etag) when creating a file.  In this way, the update to the AbfsClient would be a single line to add the optional "If-Match: E-Tag" request header, but there would be a new method in AzureBlobFileSystemStore that implements the new conditional overwrite behavior.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop 17215: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-680684569


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 35s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  33m 11s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |   0m 34s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   0m 26s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 41s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 25s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 27s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 45s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m 10s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m  8s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |   0m 26s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 18s |  hadoop-tools/hadoop-azure: The patch generated 2 new + 5 unchanged - 0 fixed = 7 total (was 5)  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  14m 52s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   0m 56s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 29s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  77m 15s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2246 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux caee03aec651 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 6a49bf9bffb |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | checkstyle | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/2/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/2/testReport/ |
   | Max. process+thread count | 454 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/2/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop 17215: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r477039654



##########
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;

Review comment:
       Add code comments for this change




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya edited a comment on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya edited a comment on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-680836914


   Test results from accounts in East US 2.
   
   From HNS Enabled account with OAuth credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 74
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   
   From HNS Enabled account with SharedKey credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 41
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   From HNS Non-enabled account with OAuth credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 250
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   
   From HNS Non-enabled account with SharedKey credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 246
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   The fix for the failing test is tracked under: https://issues.apache.org/jira/browse/HADOOP-17229


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-680908736


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 50s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  32m  5s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 40s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |   0m 32s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   0m 25s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  17m 24s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m  4s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m  0s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  16m  1s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   1m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 28s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 29s |  The patch does not generate ASF License warnings.  |
   |  |   |  79m 18s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2246 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 72954e7413bb 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 75db5526b5d |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/5/testReport/ |
   | Max. process+thread count | 414 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/5/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-695026539


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  28m 30s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 4 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  28m 54s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |   0m 34s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   0m 28s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 38s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 25s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   0m 57s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 56s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 25s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 52s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   0m 57s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 24s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 34s |  The patch does not generate ASF License warnings.  |
   |  |   |  98m 53s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/10/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2246 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 6a1edbe5151e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / fc2435cb5cf |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/10/testReport/ |
   | Max. process+thread count | 414 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/10/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-680836914


   Test results from accounts in East US 2.
   
   From HNS Enabled account with OAuth credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 74
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   From HNS Enabled account with SharedKey credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 41
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   From HNS Non-enabled account with OAuth credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 250
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   From HNS Non-enabled account with SharedKey credential settings
   `[INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 246
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   The fix for the failing test is tracked under: https://issues.apache.org/jira/browse/HADOOP-17229


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop 17215: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r477039919



##########
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.
+        op = createPath(path, abfsUriQueryBuilder, true, permission, umask);
+      } else {
+        throw e;
+      }
+    }
+
+    return op;
+  }
+
+  private AbfsRestOperation createPath(final String path,

Review comment:
       Rename to createPathImpl




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ThomasMarquardt commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
ThomasMarquardt commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r489616399



##########
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:
       There is reformatting of the previous code here that is unnecessary, but more importantly can cause merge conflicts when back porting and can introduce regressions if not carefully reviewed or caught by existing test automation.  It is better to leave the old code as-is, but only update what must be updated.  Just my thoughts.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya edited a comment on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya edited a comment on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-680836914


   Test results from accounts in East US 2.
   
   From HNS Enabled account with OAuth credential settings
   -----------------------------------------------------------------------------
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 74
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140
   
   From HNS Enabled account with SharedKey credential settings
   -----------------------------------------------------------------------------
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 41
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24
   
   From HNS Non-enabled account with OAuth credential settings
   -----------------------------------------------------------------------------
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 250
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140
   
   From HNS Non-enabled account with SharedKey credential settings
   -----------------------------------------------------------------------------
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [ERROR] Failures: 
   [ERROR]   ITestAbfsNetworkStatistics.testAbfsHttpResponseStatistics:291->AbstractAbfsIntegrationTest.assertAbfsStatistics:445->Assert.assertEquals:645->Assert.failNotEquals:834->Assert.fail:88 Mismatch in bytes_received expected:<143> but was:<311>
   [INFO] 
   [ERROR] Tests run: 453, Failures: 1, Errors: 0, Skipped: 246
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24
   
   The fix for the failing test is tracked under: https://issues.apache.org/jira/browse/HADOOP-17229


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-692028113


   Non-HNS account with SharedKey
   `[INFO] Results:
   [INFO] 
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 246
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   Non-HNS account with OAuth
   `[INFO] Results:
   [INFO] 
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 250
   WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   
   HNS account with SharedKey
   `[INFO] Results:
   [INFO] 
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 41
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   HNS account with OAuth
   `[INFO] Results:
   [INFO] 
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 74
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-694999489


   Repeated the HNS tests:
   **HNS account with SharedKey**
   [INFO] Results:
   
   [INFO]
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 41
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24
   
   **HNS account with OAuth**
   [INFO] Results:
   
   [INFO]
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 74
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-691103774






----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-691103774


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  36m 16s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  31m  3s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 37s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |   0m 33s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   0m 28s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 16s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 32s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   0m 57s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 54s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 25s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 17s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  14m  5s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   0m 58s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 28s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   | 109m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/6/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2246 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 503e9408ad19 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 89428f142fe |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/6/testReport/ |
   | Max. process+thread count | 429 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/6/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ThomasMarquardt closed pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
ThomasMarquardt closed pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya edited a comment on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya edited a comment on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-692028113


   Non-HNS account with SharedKey
   `[INFO] Results:
   
   [INFO] 
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 246
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   Non-HNS account with OAuth
   `[INFO] Results:
   
   [INFO] 
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 250
   
   WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   
   HNS account with SharedKey
   `[INFO] Results:
   
   [INFO] 
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 41
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   HNS account with OAuth
   `[INFO] Results:
   
   [INFO] 
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 74
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ThomasMarquardt commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
ThomasMarquardt commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r489701120



##########
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:
       HDFS would simply acquire a lease and overwrite the file or fail to acquire the lease and throw an IOException, so what we're really pointing out here is the need for Azure Blob Storage to support lease atomically on file creation and for ABFS to use leases when writing to files so that it can uphold the single writer semantics.  We knew this was needed from the beginning but the work has not been 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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-695029813


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 4 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  33m  2s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 41s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 43s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  17m 10s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 32s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 31s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m  4s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  15m  3s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   1m  4s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 36s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  79m 16s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/11/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2246 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux b61e07c1a5ba 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / fc2435cb5cf |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/11/testReport/ |
   | Max. process+thread count | 422 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/11/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] shanemkm commented on a change in pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
shanemkm commented on a change in pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#discussion_r480718575



##########
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:
       Is Conflict a bit too generic, i.e. should we be checking for the storage error PathAlreadyExists?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya edited a comment on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
snvijaya edited a comment on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-692028113


   **Non-HNS account with SharedKey**
   `[INFO] Results:
   
   [INFO] 
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 246
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   **Non-HNS account with OAuth**
   `[INFO] Results:
   
   [INFO] 
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 250
   
   WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`
   
   **HNS account with SharedKey**
   `[INFO] Results:
   
   [INFO] 
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 41
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 24`
   
   **HNS account with OAuth**
   `[INFO] Results:
   
   [INFO] 
   
   [INFO] Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
   
   [WARNING] Tests run: 457, Failures: 0, Errors: 0, Skipped: 74
   
   [WARNING] Tests run: 207, Failures: 0, Errors: 0, Skipped: 140`


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2246: Hadoop-17215. ABFS: Disable default create overwrite

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2246:
URL: https://github.com/apache/hadoop/pull/2246#issuecomment-693437512


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  34m 33s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 4 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  33m 44s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 40s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |   0m 33s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   0m 25s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 38s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  18m 37s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 31s |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m  5s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m  1s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 54s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   0m 58s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 21s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 35s |  The patch does not generate ASF License warnings.  |
   |  |   | 113m 31s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/9/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2246 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 614f5a16b2a6 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / ce861836918 |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/9/testReport/ |
   | Max. process+thread count | 429 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2246/9/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org