100continuesupport #54

anmolanmol1234 · 2023-05-16T09:46:37Z

Added support for expect header and account level throttling.
Added a sepcial handling in case of PutBlockList to not retry without append header if request fails with 409, as that is handled for fallback.

saxenapranav · 2023-05-17T05:01:23Z

Very huge changes!
Hope these are brought in by cherry-picking the merged commits in trunk. Can you please add reference to those commits. Let me know if cherry-pick is not the option used.

Thanks.

saxenapranav · 2023-05-17T05:02:05Z

hadoop-tools/hadoop-azure/src/config/checkstyle-suppressions.xml

              files="org[\\/]apache[\\/]hadoop[\\/]fs[\\/]azurebfs[\\/]utils[\\/]Base64.java"/>
    <suppress checks="ParameterNumber|VisibilityModifier"
              files="org[\\/]apache[\\/]hadoop[\\/]fs[\\/]azurebfs[\\/]ITestSmallWriteOptimization.java"/>
+<<<<<<< HEAD


git conflict

anmolanmol1234 · 2023-05-17T06:10:22Z

Pull requests merged in trunk added as part of this PR :- apache#5516
apache#5034

sreeb-msft · 2023-05-29T09:41:22Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java

+  }
+
+  public boolean accountThrottlingEnabled() {
+    return accountThrottlingEnabled;


[nit] In case we can either do return this. or just return for all the methods, it would be helpful in terms of readability.

sreeb-msft · 2023-05-29T10:47:15Z

...ls/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java

-          wasbUri,
-          rawConfig,
-          new AzureFileSystemInstrumentation(rawConfig));
+              wasbUri,


[nit] some spacing changes have creeped in.

sreeb-msft · 2023-05-29T10:47:46Z

...hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCreate.java

 import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
 import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation;
-import org.apache.hadoop.fs.azurebfs.services.TestAbfsClient;
+


[nit] extra line

sreeb-msft · 2023-05-29T10:55:06Z

...tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java

 */

 package org.apache.hadoop.fs.azurebfs.services;



Would it affect any backward dependencies of other parts of code depending on TestAbfsClient? (Esp if any within the current set of PRs being raised)

No this branch has been backmerged to dev, should not cause any issues.

saxenapranav · 2023-05-30T09:55:37Z

throttling part looks good.

saxenapranav · 2023-05-30T09:56:14Z

since it has been made up by cherry-picking the prs of throttling and 100continue, can you please attach the test-results in the pr please @anmolanmol1234

saxenapranav · 2023-05-30T10:28:55Z

...ls/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java

saxenapranav · 2023-05-30T10:36:26Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

+  private boolean checkUserErrorBlob(int responseStatusCode) {
+    return (responseStatusCode >= HttpURLConnection.HTTP_BAD_REQUEST
+            && responseStatusCode < HttpURLConnection.HTTP_INTERNAL_ERROR
+            && responseStatusCode != HttpURLConnection.HTTP_CONFLICT);
+  }
+


Please give reasoning for excluding 409.

saxenapranav · 2023-05-30T10:41:32Z

...zure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientThrottlingIntercept.java

        if (contentLength > 0) {
-          singleton.writeThrottler.addBytesTransferred(contentLength,
-              isFailedOperation);
+          writeThrottler.addBytesTransferred(contentLength,


lets add this part as well:

/** * Updates the metrics for the case when response code signifies throttling * but there are some expected bytes to be sent. * @param isThrottledOperation returns true if status code is HTTP_UNAVAILABLE * @param abfsHttpOperation Used for status code and data transferred. * @return true if the operation is throttled and has some bytes to transfer. */ private boolean updateBytesTransferred(boolean isThrottledOperation, AbfsHttpOperation abfsHttpOperation) { return isThrottledOperation && abfsHttpOperation.getExpectedBytesToBeSent() > 0; }

if (contentLength == 0) { /* Signifies the case where we could not update the bytesSent due to throttling but there were some expectedBytesToBeSent. */ if (updateBytesTransferred(isThrottledOperation, abfsHttpOperation)) { LOG.debug("Updating metrics due to throttling for path {}", abfsHttpOperation.getConnUrl().getPath()); contentLength = abfsHttpOperation.getExpectedBytesToBeSent(); } }

if (contentLength > 0) { writeThrottler.addBytesTransferred(contentLength, isFailedOperation);

ref: https://github.com/apache/hadoop/pull/5516/files

Sorry this code piece got missed, added it. Thanks

saxenapranav · 2023-05-30T10:47:52Z

...ols/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java

      tracingContext.constructHeader(httpOperation);

-      signRequest(httpOperation, hasRequestBody ? bufferLength : 0);
+      switch(client.getAuthType()) {


why are we pulling back code of signRequest to the executeHttpOperation

…fsHadoop into 100continuesupport

saxenapranav

LGTM! Please add test results before merging. Thanks.

anmolanmol1234 · 2023-05-31T04:16:20Z

:::: AGGREGATED TEST RESULT ::::

HNS-OAuth

[INFO] Results:
[INFO]
[WARNING] Tests run: 111, Failures: 0, Errors: 0, Skipped: 4
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestAzureBlobFileSystemCreate.testDefaultCreateOverwriteFileTest:1025->testCreateFileOverwrite:1067->AbstractAbfsIntegrationTest.assertAbfsStatistics:510->Assert.assertEquals:647->Assert.failNotEquals:835->Assert.fail:89 Mismatch in connections_made expected:<5> but was:<4>
[INFO]
[ERROR] Tests run: 744, Failures: 1, Errors: 0, Skipped: 155
[INFO] Results:
[INFO]
[WARNING] Tests run: 273, Failures: 0, Errors: 0, Skipped: 41

HNS-SharedKey

[INFO] Results:
[INFO]
[WARNING] Tests run: 111, Failures: 0, Errors: 0, Skipped: 4
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestAzureBlobFileSystemCreate.testDefaultCreateOverwriteFileTest:1025->testCreateFileOverwrite:1067->AbstractAbfsIntegrationTest.assertAbfsStatistics:510->Assert.assertEquals:647->Assert.failNotEquals:835->Assert.fail:89 Mismatch in connections_made expected:<5> but was:<4>
[INFO]
[ERROR] Tests run: 744, Failures: 1, Errors: 0, Skipped: 155
[INFO] Results:
[INFO]
[WARNING] Tests run: 273, Failures: 0, Errors: 0, Skipped: 41

NonHNS-SharedKey

[INFO] Results:
[INFO]
[WARNING] Tests run: 111, Failures: 0, Errors: 0, Skipped: 4
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR] ITestAzureBlobFileSystemExplictImplicitRename.testRenameImplicitDirectoryContainingImplicitDirectory:210->explicitImplicitDirectoryRenameTest:758->createSourcePaths:835->AbstractAbfsIntegrationTest.createAzCopyDirectory:524 » IO
[INFO]
[ERROR] Tests run: 744, Failures: 0, Errors: 1, Skipped: 275
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR] ITestAzureBlobFileSystemBasics>FileSystemContractBaseTest.testOverWriteAndRead:620->FileSystemContractBaseTest.writeAndRead:927 » TestTimedOut
[INFO]
[ERROR] Tests run: 273, Failures: 0, Errors: 1, Skipped: 45

NonHNS-OAuth

[INFO] Results:
[INFO]
[WARNING] Tests run: 111, Failures: 0, Errors: 0, Skipped: 4
[INFO] Results:
[INFO]
[WARNING] Tests run: 744, Failures: 0, Errors: 0, Skipped: 275
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR] ITestAzureBlobFileSystemBasics>FileSystemContractBaseTest.testOverWriteAndRead:620->FileSystemContractBaseTest.writeAndRead:932 » TestTimedOut
[INFO]
[ERROR] Tests run: 273, Failures: 0, Errors: 1, Skipped: 45

AppendBlob-HNS-OAuth

[INFO] Results:
[INFO]
[WARNING] Tests run: 111, Failures: 0, Errors: 0, Skipped: 4
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestAzureBlobFileSystemCreate.testDefaultCreateOverwriteFileTest:1025->testCreateFileOverwrite:1067->AbstractAbfsIntegrationTest.assertAbfsStatistics:510->Assert.assertEquals:647->Assert.failNotEquals:835->Assert.fail:89 Mismatch in connections_made expected:<5> but was:<4>
[INFO]
[ERROR] Tests run: 744, Failures: 1, Errors: 0, Skipped: 155
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR] ITestAbfsReadWriteAndSeek.testReadAndWriteWithDifferentBufferSizesAndSeek:78->testReadWriteAndSeek:120 » TestTimedOut
[INFO]
[ERROR] Tests run: 273, Failures: 0, Errors: 1, Skipped: 41

anmolanmol1234 added 4 commits May 14, 2023 23:17

100 continue support

899c790

accountThrottling

b9fa9b5

100 continue

1bc98ce

Code fixes

0159117

anmolanmol1234 requested review from anujmodi2021, saxenapranav, snvijaya and sreeb-msft May 16, 2023 09:46

anmolanmol1234 self-assigned this May 16, 2023

saxenapranav reviewed May 17, 2023

View reviewed changes

Update checkstyle-suppressions.xml

d64c280

sreeb-msft reviewed May 29, 2023

View reviewed changes

Fix spacing issues

97b6587

Merge branch 'ABFS_3.3.2_dev' into 100continuesupport

6f8ed8a

saxenapranav requested changes May 30, 2023

View reviewed changes

anmolanmol1234 added 3 commits May 30, 2023 06:11

Fix spacing issue

a078b8e

Merge branch '100continuesupport' of https://github.com/ABFSDriver/Ab…

b0b855c

…fsHadoop into 100continuesupport

PR comments

efac78c

saxenapranav approved these changes May 31, 2023

View reviewed changes

sreeb-msft approved these changes May 31, 2023

View reviewed changes

anmolanmol1234 merged commit b558a4c into ABFS_3.3.2_dev May 31, 2023

100continuesupport #54

100continuesupport #54

Uh oh!

Conversation

anmolanmol1234 commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saxenapranav commented May 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anmolanmol1234 commented May 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saxenapranav commented May 30, 2023

Uh oh!

saxenapranav commented May 30, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saxenapranav left a comment

Choose a reason for hiding this comment

Uh oh!

anmolanmol1234 commented May 31, 2023

HNS-OAuth

HNS-SharedKey

NonHNS-SharedKey

NonHNS-OAuth

AppendBlob-HNS-OAuth

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anmolanmol1234 commented May 16, 2023 •

edited

Loading