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 "sreeb-msft (via GitHub)" <gi...@apache.org> on 2023/03/02 07:20:42 UTC
[GitHub] [hadoop] sreeb-msft opened a new pull request, #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
sreeb-msft opened a new pull request, #5446:
URL: https://github.com/apache/hadoop/pull/5446
This PR introduces two changes that allows client-side throttling and backoff only for new requests, and increases the level of control through new configs in AbfsClientThrottlingAnalyzer.
For the first change, it checks for whether the current rest operation corresponds to a new or retried request. In case of a new request, it calls the necessary methods to apply throttling and backoff at the client side if necessary. In case of a retried request, these methods are skipped and no backoff is applied, or even checked if necessary. This, however, does not affect any other flow such as updating metrics, which happens for each request, irrespective of whether retried or new.
In code, the check for whether it is a retried request or not, and the subsequent call for CST is moved to a separate new method, which directly takes input of the current retry count, and makes the decision based on that.
#### Tests Added:
Two separate tests have been added as part of this change, one for the read and the other for write/append requests - which are the only cases where client-side throttling is applied. Each test does the following -
1. Validates for a new request:
- The method for CST (sendingRequest) is called.
- The counters for throttling (read/append) are incremented by 1.
2. Validates for a retried request:
- The CST method (sendingRequest) call is skipped.
- The counters for throttling (read/append) are not incremented and are the same before and after the apply throttling backoff call happens.
For the second change, new configs are introduced for the following -
1. `fs.azure.min.acceptable.error.percentage`
2. `fs.azure.max.equilibrium.error.percentage`
3. `fs.azure.rapid.sleep.decrease.factor`
4. `fs.azure.rapid.sleep.decrease.transition.ms`
5. `fs.azure.sleep.decrease.factor`
6. `fs.azure.sleep.increase.factor`
All of these are meant to replace the static values that were in place, by allowing the user more control to configure these directly. The static values for these variables, earlier present in code itself, now would be used as default values for the configs.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1130357920
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -268,10 +269,62 @@
DefaultValue = DEFAULT_ACCOUNT_OPERATION_IDLE_TIMEOUT_MS)
private int accountOperationIdleTimeout;
+ /**
+ * Analysis Period for client-side throttling
+ */
Review Comment:
nit: spacing.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] sreeb-msft commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "sreeb-msft (via GitHub)" <gi...@apache.org>.
sreeb-msft commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1126325114
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
+
+ @Test
+ public void testClientBackoffOnlyNewReadRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_GET;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.ReadFile;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(new Long(readThrottleStatBefore+1), readThrottleStatAfter);
+
+
+ readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(readThrottleStatBefore, readThrottleStatAfter);
+ }
+
+ @Test
+ public void testReadThrottleNewRequest() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient());
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ AbfsThrottlingIntercept intercept = Mockito.mock(AbfsThrottlingIntercept.class);
+ Mockito.doNothing().when(intercept).sendingRequest(Mockito.any(AbfsRestOperationType.class), Mockito.any(AbfsCounters.class));
+ Mockito.doReturn(intercept).when(client).getIntercept();
+
+ // setting up the spy AbfsRestOperation class for read
+ final List<AbfsHttpHeader> requestHeaders = client.createDefaultHeaders();
+
+ final AbfsUriQueryBuilder abfsUriQueryBuilder = client.createDefaultUriQueryBuilder();
+
+ final URL url = client.createRequestUrl("/dummyReadFile", abfsUriQueryBuilder.toString());
+ final AbfsRestOperation mockRestOp = Mockito.spy(new AbfsRestOperation(
+ AbfsRestOperationType.ReadFile,
+ client,
+ HTTP_METHOD_GET,
+ url,
+ requestHeaders));
+
+ // setting up mock behavior for the AbfsHttpOperation class
+ AbfsHttpOperation mockHttpOp = Mockito.spy(mockRestOp.createHttpOperationInstance());
Review Comment:
Just Mocking is leading to NPE under executeHttpOperation method. Keeping it as a spy.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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 #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1453847787
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 37s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 42m 13s | | trunk passed |
| +1 :green_heart: | compile | 0m 41s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 0m 38s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 0m 35s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 45s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 42s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 33s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 24s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 32s | | the patch passed |
| +1 :green_heart: | compile | 0m 33s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| -1 :x: | javac | 0m 33s | [/results-compile-javac-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/2/artifact/out/results-compile-javac-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt) | hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 generated 2 new + 55 unchanged - 0 fixed = 57 total (was 55) |
| +1 :green_heart: | compile | 0m 29s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 0m 29s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 19s | [/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/2/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt) | hadoop-tools/hadoop-azure: The patch generated 9 new + 4 unchanged - 0 fixed = 13 total (was 4) |
| +1 :green_heart: | mvnsite | 0m 32s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 26s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 25s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 3s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 10s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 3m 13s | [/patch-unit-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/2/artifact/out/patch-unit-hadoop-tools_hadoop-azure.txt) | hadoop-azure in the patch passed. |
| -1 :x: | asflicense | 0m 38s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/2/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 97m 52s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.fs.azurebfs.services.TestAbfsClientThrottlingAnalyzer |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5446 |
| JIRA Issue | HADOOP-18640 |
| Optional Tests | dupname asflicense codespell detsecrets compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle |
| uname | Linux dc39661540a8 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 91c932722d3e92aa48d19a5d275e4721a29e48f6 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/2/testReport/ |
| Max. process+thread count | 555 (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-5446/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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 #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1461580972
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 38s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 38m 44s | | trunk passed |
| +1 :green_heart: | compile | 0m 41s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 0m 39s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 0m 36s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 43s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 32s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 19s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 30s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 20m 50s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 30s | | the patch passed |
| +1 :green_heart: | compile | 0m 34s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 0m 34s | | the patch passed |
| +1 :green_heart: | compile | 0m 29s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 0m 29s | | the patch passed |
| -1 :x: | blanks | 0m 0s | [/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/7/artifact/out/blanks-eol.txt) | The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| -0 :warning: | checkstyle | 0m 19s | [/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/7/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt) | hadoop-tools/hadoop-azure: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7) |
| +1 :green_heart: | mvnsite | 0m 33s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 4s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 35s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 3m 12s | [/patch-unit-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/7/artifact/out/patch-unit-hadoop-tools_hadoop-azure.txt) | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 37s | | The patch does not generate ASF License warnings. |
| | | 94m 54s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.fs.azurebfs.services.TestAbfsClientThrottlingAnalyzer |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5446 |
| JIRA Issue | HADOOP-18640 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux bc0f9d906cc6 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 9bea2eff81d5f897a138d59cb5bb0575e4f6ade6 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/7/testReport/ |
| Max. process+thread count | 705 (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-5446/7/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1130357512
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -222,6 +224,10 @@ AbfsThrottlingIntercept getIntercept() {
return intercept;
}
+ boolean shouldThrottleRetries() {
+ return throttleRetries;
+ }
+
Review Comment:
Either of two is fine.
if we keep in abfsClient, it will be stored account level, and we dont need to check anything till abfsClient object is alived. In abfsRestOp, new field will be created as new object is created for each api call.
if we keep it in abfsRestOperation, it is something which is actually requried in abfsRestOperation. Though it doesn't matter. You may please resolve this comment.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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
Re: [PR] HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests [hadoop]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-3445309605
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you feel like this was a mistake, or you would like to continue working on it, please feel free to re-open it and ask for a committer to remove the stale tag and review again.
Thanks all for your contribution.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1462348876
error was due to space on empty line: try merging https://github.com/sreeb-msft/hadoop/pull/2.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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
Re: [PR] HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests [hadoop]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
URL: https://github.com/apache/hadoop/pull/5446
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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 #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1455548802
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 36s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 40m 43s | | trunk passed |
| +1 :green_heart: | compile | 0m 43s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 0m 41s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 0m 33s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 38s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 33s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 46s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 31s | | the patch passed |
| +1 :green_heart: | compile | 0m 33s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| -1 :x: | javac | 0m 33s | [/results-compile-javac-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/3/artifact/out/results-compile-javac-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt) | hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 generated 2 new + 55 unchanged - 0 fixed = 57 total (was 55) |
| +1 :green_heart: | compile | 0m 29s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 0m 29s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 19s | [/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/3/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt) | hadoop-tools/hadoop-azure: The patch generated 9 new + 4 unchanged - 0 fixed = 13 total (was 4) |
| +1 :green_heart: | mvnsite | 0m 33s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 25s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 4s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 10s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 3m 12s | [/patch-unit-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/3/artifact/out/patch-unit-hadoop-tools_hadoop-azure.txt) | hadoop-azure in the patch passed. |
| -1 :x: | asflicense | 0m 37s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/3/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 96m 43s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.fs.azurebfs.services.TestAbfsClientThrottlingAnalyzer |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5446 |
| JIRA Issue | HADOOP-18640 |
| Optional Tests | dupname asflicense codespell detsecrets compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle |
| uname | Linux 0f2865589c72 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / b036f9285b4b2124273dda736bf44f002f7e27d1 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/3/testReport/ |
| Max. process+thread count | 558 (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-5446/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1127368834
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
+
+ @Test
+ public void testClientBackoffOnlyNewReadRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_GET;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.ReadFile;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(new Long(readThrottleStatBefore+1), readThrottleStatAfter);
+
+
+ readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(readThrottleStatBefore, readThrottleStatAfter);
+ }
+
+ @Test
+ public void testReadThrottleNewRequest() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient());
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ AbfsThrottlingIntercept intercept = Mockito.mock(AbfsThrottlingIntercept.class);
+ Mockito.doNothing().when(intercept).sendingRequest(Mockito.any(AbfsRestOperationType.class), Mockito.any(AbfsCounters.class));
+ Mockito.doReturn(intercept).when(client).getIntercept();
+
+ // setting up the spy AbfsRestOperation class for read
+ final List<AbfsHttpHeader> requestHeaders = client.createDefaultHeaders();
+
+ final AbfsUriQueryBuilder abfsUriQueryBuilder = client.createDefaultUriQueryBuilder();
+
+ final URL url = client.createRequestUrl("/dummyReadFile", abfsUriQueryBuilder.toString());
+ final AbfsRestOperation mockRestOp = Mockito.spy(new AbfsRestOperation(
+ AbfsRestOperationType.ReadFile,
+ client,
+ HTTP_METHOD_GET,
+ url,
+ requestHeaders));
+
+ // setting up mock behavior for the AbfsHttpOperation class
+ AbfsHttpOperation mockHttpOp = Mockito.spy(mockRestOp.createHttpOperationInstance());
Review Comment:
everything is possible :). Bit of debug is required. Please merge https://github.com/sreeb-msft/hadoop/pull/1 in your pr.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1122694142
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -272,6 +273,30 @@
DefaultValue = DEFAULT_ANALYSIS_PERIOD_MS)
private int analysisPeriod;
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_MIN_ACCEPTABLE_ERROR_PERCENTAGE,
+ DefaultValue = DEFAULT_MIN_ACCEPTABLE_ERROR_PERCENTAGE)
+ private double minAcceptableErrorPercentage;
+
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_MAX_EQUILIBRIUM_ERROR_PERCENTAGE,
+ DefaultValue = DEFAULT_MAX_EQUILIBRIUM_ERROR_PERCENTAGE)
+ private double maxEquilibriumErrorPercentage;
+
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_RAPID_SLEEP_DECREASE_FACTOR,
+ DefaultValue = DEFAULT_RAPID_SLEEP_DECREASE_FACTOR)
+ private double rapidSleepDecreaseFactor;
+
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_RAPID_SLEEP_DECREASE_TRANSITION_MS,
+ DefaultValue = DEFAULT_RAPID_SLEEP_DECREASE_TRANSITION_PERIOD_MS)
+ private double rapidSleepDecreaseTransitionPeriodMs;
+
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_SLEEP_DECREASE_FACTOR,
+ DefaultValue = DEFAULT_SLEEP_DECREASE_FACTOR)
+ private double sleepDecreaseFactor;
+
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_SLEEP_INCREASE_FACTOR,
+ DefaultValue = DEFAULT_SLEEP_INCREASE_FACTOR)
+ private double sleepIncreaseFactor;
Review Comment:
lets add documentation of what each field means.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +293,67 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+@Test
+public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
Review Comment:
why sleep is there?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java:
##########
@@ -334,6 +337,21 @@ private boolean executeHttpOperation(final int retryCount,
return true;
}
+ /**
+ * Makes a call for client side throttling based on
+ * the request count.
+ * @param operationType operation type of current request
+ * @param abfsCounters AbfsCounters instance
+ */
+ @VisibleForTesting
+ boolean applyThrottlingBackoff(int retryCount, AbfsRestOperationType operationType, AbfsCounters abfsCounters) {
+ if (retryCount == 0) {
+ intercept.sendingRequest(operationType, abfsCounters);
+ return true;
+ }
+ return false;
+ }
+
Review Comment:
once test is refactored. lets keep
```
if (retryCount == 0) {
intercept.sendingRequest(operationType, abfsCounters);
return true;
}
```
inline to line 286.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +293,67 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+@Test
+public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
Review Comment:
lets not just test this method.
lets test the whole executeRequest method. Have httpOperation mocked. And change the behaviour of httpOperation.processResponse.. Fail it for 2-3 times. and assert.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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 #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1462216813
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 38s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 38m 40s | | trunk passed |
| +1 :green_heart: | compile | 0m 43s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 0m 39s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 0m 36s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 35s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 18s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 53s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 21m 12s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 35s | | the patch passed |
| +1 :green_heart: | compile | 0m 38s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 0m 38s | | the patch passed |
| +1 :green_heart: | compile | 0m 32s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 0m 32s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 19s | [/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/8/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt) | hadoop-tools/hadoop-azure: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7) |
| +1 :green_heart: | mvnsite | 0m 36s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 28s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 15s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 15s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 3m 12s | [/patch-unit-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/8/artifact/out/patch-unit-hadoop-tools_hadoop-azure.txt) | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 36s | | The patch does not generate ASF License warnings. |
| | | 97m 10s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.fs.azurebfs.services.TestAbfsClientThrottlingAnalyzer |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/8/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5446 |
| JIRA Issue | HADOOP-18640 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 137e902f552a 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 9c5e993d71f861abc6145cd9d2ca1a78343c9474 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/8/testReport/ |
| Max. process+thread count | 554 (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-5446/8/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1127328623
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -268,10 +269,62 @@
DefaultValue = DEFAULT_ACCOUNT_OPERATION_IDLE_TIMEOUT_MS)
private int accountOperationIdleTimeout;
+ /**
+ * Analysis Period for client-side throttling
+ */
@IntegerConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_ANALYSIS_PERIOD,
DefaultValue = DEFAULT_ANALYSIS_PERIOD_MS)
private int analysisPeriod;
+ /**
+ * Lower limit of acceptable error percentage
+ */
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_MIN_ACCEPTABLE_ERROR_PERCENTAGE,
+ DefaultValue = DEFAULT_MIN_ACCEPTABLE_ERROR_PERCENTAGE)
+ private double minAcceptableErrorPercentage;
+
+ /**
+ * Maximum equilibrium error percentage
+ */
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_MAX_EQUILIBRIUM_ERROR_PERCENTAGE,
+ DefaultValue = DEFAULT_MAX_EQUILIBRIUM_ERROR_PERCENTAGE)
+ private double maxEquilibriumErrorPercentage;
+
+ /**
+ * Rapid sleep decrease factor to increase throughput
+ */
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_RAPID_SLEEP_DECREASE_FACTOR,
+ DefaultValue = DEFAULT_RAPID_SLEEP_DECREASE_FACTOR)
+ private double rapidSleepDecreaseFactor;
+
+ /**
+ * Rapid sleep decrease transition period in milliseconds
+ */
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_RAPID_SLEEP_DECREASE_TRANSITION_MS,
+ DefaultValue = DEFAULT_RAPID_SLEEP_DECREASE_TRANSITION_PERIOD_MS)
+ private double rapidSleepDecreaseTransitionPeriodMs;
+
+ /**
+ * Sleep decrease factor to increase throughput
+ */
Review Comment:
fix spacing in all javadocs wherever required.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -20,28 +20,42 @@
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
+import static java.net.HttpURLConnection.HTTP_OK;
+import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_GET;
+import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_PUT;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_BACKOFF_INTERVAL;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MAX_BACKOFF_INTERVAL;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MAX_IO_RETRIES;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MIN_BACKOFF_INTERVAL;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_LEVEL_THROTTLING_ENABLED;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ENABLE_AUTOTHROTTLING;
import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.MIN_BUFFER_SIZE;
+import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.IF_MATCH;
+import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.RANGE;
import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_ABFS_ACCOUNT1_NAME;
import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_ACCOUNT_NAME;
import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.TEST_CONFIGURATION_FILE_NAME;
import static org.junit.Assume.assumeTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
Review Comment:
fix wildcard. You can switch off wildcard imports on IDE so in future it doesnt come.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
+
+ @Test
+ public void testClientBackoffOnlyNewReadRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_GET;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.ReadFile;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(new Long(readThrottleStatBefore+1), readThrottleStatAfter);
+
+
+ readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(readThrottleStatBefore, readThrottleStatAfter);
+ }
+
+ @Test
+ public void testReadThrottleNewRequest() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient());
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ AbfsThrottlingIntercept intercept = Mockito.mock(AbfsThrottlingIntercept.class);
+ Mockito.doNothing().when(intercept).sendingRequest(Mockito.any(AbfsRestOperationType.class), Mockito.any(AbfsCounters.class));
+ Mockito.doReturn(intercept).when(client).getIntercept();
+
+ // setting up the spy AbfsRestOperation class for read
+ final List<AbfsHttpHeader> requestHeaders = client.createDefaultHeaders();
+
+ final AbfsUriQueryBuilder abfsUriQueryBuilder = client.createDefaultUriQueryBuilder();
+
+ final URL url = client.createRequestUrl("/dummyReadFile", abfsUriQueryBuilder.toString());
+ final AbfsRestOperation mockRestOp = Mockito.spy(new AbfsRestOperation(
+ AbfsRestOperationType.ReadFile,
+ client,
+ HTTP_METHOD_GET,
+ url,
+ requestHeaders));
+
+ // setting up mock behavior for the AbfsHttpOperation class
+ AbfsHttpOperation mockHttpOp = Mockito.spy(mockRestOp.createHttpOperationInstance());
Review Comment:
lets resolve to not do this. Some issue in our mocking. The way described should work.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -222,6 +224,10 @@ AbfsThrottlingIntercept getIntercept() {
return intercept;
}
+ boolean shouldThrottleRetries() {
+ return throttleRetries;
+ }
+
Review Comment:
why is it in abfsClient. What risk is there to keep it in abfsRestOperation.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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 #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1451533058
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 12m 23s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 38m 2s | | trunk passed |
| +1 :green_heart: | compile | 0m 42s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 0m 39s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 0m 34s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 33s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 18s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 18s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 34s | | the patch passed |
| +1 :green_heart: | compile | 0m 33s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| -1 :x: | javac | 0m 33s | [/results-compile-javac-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/1/artifact/out/results-compile-javac-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt) | hadoop-tools_hadoop-azure-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 generated 2 new + 55 unchanged - 0 fixed = 57 total (was 55) |
| +1 :green_heart: | compile | 0m 30s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 0m 30s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 19s | [/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/1/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt) | hadoop-tools/hadoop-azure: The patch generated 7 new + 4 unchanged - 0 fixed = 11 total (was 4) |
| +1 :green_heart: | mvnsite | 0m 34s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 25s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 9s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 56s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 3m 11s | [/patch-unit-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/1/artifact/out/patch-unit-hadoop-tools_hadoop-azure.txt) | hadoop-azure in the patch passed. |
| -1 :x: | asflicense | 0m 35s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/1/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 107m 9s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.fs.azurebfs.services.TestAbfsClientThrottlingAnalyzer |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5446 |
| JIRA Issue | HADOOP-18640 |
| Optional Tests | dupname asflicense codespell detsecrets compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle |
| uname | Linux 794084458b10 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 0011b04d75d2908ea36540b8958d43d988d3d864 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/1/testReport/ |
| Max. process+thread count | 751 (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-5446/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1125941455
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java:
##########
@@ -50,6 +50,14 @@ public final class FileSystemConfigurations {
public static final int DEFAULT_AZURE_OAUTH_TOKEN_FETCH_RETRY_MAX_BACKOFF_INTERVAL = SIXTY_SECONDS;
public static final int DEFAULT_AZURE_OAUTH_TOKEN_FETCH_RETRY_DELTA_BACKOFF = 2;
+ // Throttling Analysis defaults.
+ public static final double DEFAULT_MIN_ACCEPTABLE_ERROR_PERCENTAGE = .1;
Review Comment:
nit:`0.1`
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
Review Comment:
what is happening in throttlingIntercept is out of scope for the pr. its not testing what the change we want to have. What its touching is that if the if-else block is working correct or not. even if we want to to test it, lets have intercept mocked and just check if `intercept.sendingRequest` is being called or not.
What i want to say is that, lets not get into whats happening inside throttlingIntercept.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java:
##########
@@ -50,6 +50,14 @@ public final class FileSystemConfigurations {
public static final int DEFAULT_AZURE_OAUTH_TOKEN_FETCH_RETRY_MAX_BACKOFF_INTERVAL = SIXTY_SECONDS;
public static final int DEFAULT_AZURE_OAUTH_TOKEN_FETCH_RETRY_DELTA_BACKOFF = 2;
+ // Throttling Analysis defaults.
+ public static final double DEFAULT_MIN_ACCEPTABLE_ERROR_PERCENTAGE = .1;
Review Comment:
please check other double values in default configs added.
nit comment.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java:
##########
@@ -334,6 +337,25 @@ private boolean executeHttpOperation(final int retryCount,
return true;
}
+ /**
+ * Makes a call for client side throttling based on
+ * the request count.
+ * @param operationType operation type of current request
+ * @param abfsCounters AbfsCounters instance
+ */
+ @VisibleForTesting
+ boolean applyThrottlingBackoff(int retryCount, AbfsRestOperationType operationType, AbfsCounters abfsCounters) {
+ if (retryCount == 0) {
+ intercept.sendingRequest(operationType, abfsCounters);
+ return true;
+ }
+ return false;
+ }
+
+ public AbfsHttpOperation createHttpOperationInstance() throws IOException {
Review Comment:
dont have it public. have it like:
```
AbfsHttpOperation createHttpOperationInstance() throws IOException {
```
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -268,10 +269,55 @@
DefaultValue = DEFAULT_ACCOUNT_OPERATION_IDLE_TIMEOUT_MS)
private int accountOperationIdleTimeout;
+ /*
+ Analysis Period for client-side throttling
+ */
@IntegerConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_ANALYSIS_PERIOD,
DefaultValue = DEFAULT_ANALYSIS_PERIOD_MS)
private int analysisPeriod;
+ /*
Review Comment:
lets have it in javadocs?
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
Review Comment:
nit: lets remove this line.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
+
+ @Test
+ public void testClientBackoffOnlyNewReadRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_GET;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.ReadFile;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(new Long(readThrottleStatBefore+1), readThrottleStatAfter);
+
+
+ readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
Review Comment:
same comment as above.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
+
+ @Test
+ public void testClientBackoffOnlyNewReadRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_GET;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.ReadFile;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(new Long(readThrottleStatBefore+1), readThrottleStatAfter);
+
+
+ readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(readThrottleStatBefore, readThrottleStatAfter);
+ }
+
+ @Test
+ public void testReadThrottleNewRequest() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient());
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ AbfsThrottlingIntercept intercept = Mockito.mock(AbfsThrottlingIntercept.class);
+ Mockito.doNothing().when(intercept).sendingRequest(Mockito.any(AbfsRestOperationType.class), Mockito.any(AbfsCounters.class));
+ Mockito.doReturn(intercept).when(client).getIntercept();
+
+ // setting up the spy AbfsRestOperation class for read
+ final List<AbfsHttpHeader> requestHeaders = client.createDefaultHeaders();
+
+ final AbfsUriQueryBuilder abfsUriQueryBuilder = client.createDefaultUriQueryBuilder();
+
+ final URL url = client.createRequestUrl("/dummyReadFile", abfsUriQueryBuilder.toString());
+ final AbfsRestOperation mockRestOp = Mockito.spy(new AbfsRestOperation(
+ AbfsRestOperationType.ReadFile,
+ client,
+ HTTP_METHOD_GET,
+ url,
+ requestHeaders));
+
+ // setting up mock behavior for the AbfsHttpOperation class
+ AbfsHttpOperation mockHttpOp = Mockito.spy(mockRestOp.createHttpOperationInstance());
+ Mockito.doReturn(-1)
+ .doReturn(-1)
+ .doReturn(-1)
+ .doReturn(HTTP_OK)
+ .when(mockHttpOp).getStatusCode();
+ Mockito.doNothing().when(mockHttpOp).setRequestProperty(nullable(String.class), nullable(String.class));
+ Mockito.doNothing().when(mockHttpOp).sendRequest(nullable(byte[].class), nullable(int.class), nullable(int.class));
+ Mockito.doNothing().when(mockHttpOp).processResponse(nullable(byte[].class), nullable(int.class), nullable(int.class));
+
+ Mockito.doReturn(mockHttpOp).when(mockRestOp).createHttpOperationInstance();
+ Mockito.doReturn(mockHttpOp).when(mockRestOp).getResult();
+
+ mockRestOp.execute(getTestTracingContext(fs, false));
+ Mockito.verify(intercept, times(1)).sendingRequest(Mockito.any(AbfsRestOperationType.class), Mockito.any(AbfsCounters.class));
Review Comment:
since you plan to keep `applyThrottlingBackoff` in your code, let check if `applyThrottlingBackoff` is being called 4 times. it would be great addition. What you feel?
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
+
+ @Test
+ public void testClientBackoffOnlyNewReadRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_GET;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.ReadFile;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(new Long(readThrottleStatBefore+1), readThrottleStatAfter);
+
+
+ readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(readThrottleStatBefore, readThrottleStatAfter);
+ }
+
+ @Test
+ public void testReadThrottleNewRequest() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient());
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ AbfsThrottlingIntercept intercept = Mockito.mock(AbfsThrottlingIntercept.class);
+ Mockito.doNothing().when(intercept).sendingRequest(Mockito.any(AbfsRestOperationType.class), Mockito.any(AbfsCounters.class));
+ Mockito.doReturn(intercept).when(client).getIntercept();
+
+ // setting up the spy AbfsRestOperation class for read
+ final List<AbfsHttpHeader> requestHeaders = client.createDefaultHeaders();
+
+ final AbfsUriQueryBuilder abfsUriQueryBuilder = client.createDefaultUriQueryBuilder();
+
+ final URL url = client.createRequestUrl("/dummyReadFile", abfsUriQueryBuilder.toString());
+ final AbfsRestOperation mockRestOp = Mockito.spy(new AbfsRestOperation(
+ AbfsRestOperationType.ReadFile,
+ client,
+ HTTP_METHOD_GET,
+ url,
+ requestHeaders));
+
+ // setting up mock behavior for the AbfsHttpOperation class
+ AbfsHttpOperation mockHttpOp = Mockito.spy(mockRestOp.createHttpOperationInstance());
+ Mockito.doReturn(-1)
+ .doReturn(-1)
+ .doReturn(-1)
+ .doReturn(HTTP_OK)
+ .when(mockHttpOp).getStatusCode();
+ Mockito.doNothing().when(mockHttpOp).setRequestProperty(nullable(String.class), nullable(String.class));
+ Mockito.doNothing().when(mockHttpOp).sendRequest(nullable(byte[].class), nullable(int.class), nullable(int.class));
+ Mockito.doNothing().when(mockHttpOp).processResponse(nullable(byte[].class), nullable(int.class), nullable(int.class));
+
+ Mockito.doReturn(mockHttpOp).when(mockRestOp).createHttpOperationInstance();
+ Mockito.doReturn(mockHttpOp).when(mockRestOp).getResult();
+
+ mockRestOp.execute(getTestTracingContext(fs, false));
+ Mockito.verify(intercept, times(1)).sendingRequest(Mockito.any(AbfsRestOperationType.class), Mockito.any(AbfsCounters.class));
+ }
+
+ @Test
+ public void testWriteThrottleNewRequest() throws IOException {
Review Comment:
this is not different than `testReadThrottleNewRequest`, just that api is changing. we can remove it, since its not adding any significance. What you feel?
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] sreeb-msft commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "sreeb-msft (via GitHub)" <gi...@apache.org>.
sreeb-msft commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1130546200
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -222,6 +224,10 @@ AbfsThrottlingIntercept getIntercept() {
return intercept;
}
+ boolean shouldThrottleRetries() {
+ return throttleRetries;
+ }
+
Review Comment:
Right. That makes sense. We can directly invoke it from within AbfsRestOperation.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1126166721
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.", expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter = counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
+
+ @Test
+ public void testClientBackoffOnlyNewReadRequest() throws IOException, InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_GET;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.ReadFile;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType, client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0, testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(new Long(readThrottleStatBefore+1), readThrottleStatAfter);
+
+
+ readThrottleStatBefore = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType, counters);
+ assertEquals(false, appliedBackoff);
+ readThrottleStatAfter = counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(readThrottleStatBefore, readThrottleStatAfter);
+ }
+
+ @Test
+ public void testReadThrottleNewRequest() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient());
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ AbfsThrottlingIntercept intercept = Mockito.mock(AbfsThrottlingIntercept.class);
+ Mockito.doNothing().when(intercept).sendingRequest(Mockito.any(AbfsRestOperationType.class), Mockito.any(AbfsCounters.class));
+ Mockito.doReturn(intercept).when(client).getIntercept();
+
+ // setting up the spy AbfsRestOperation class for read
+ final List<AbfsHttpHeader> requestHeaders = client.createDefaultHeaders();
+
+ final AbfsUriQueryBuilder abfsUriQueryBuilder = client.createDefaultUriQueryBuilder();
+
+ final URL url = client.createRequestUrl("/dummyReadFile", abfsUriQueryBuilder.toString());
+ final AbfsRestOperation mockRestOp = Mockito.spy(new AbfsRestOperation(
+ AbfsRestOperationType.ReadFile,
+ client,
+ HTTP_METHOD_GET,
+ url,
+ requestHeaders));
+
+ // setting up mock behavior for the AbfsHttpOperation class
+ AbfsHttpOperation mockHttpOp = Mockito.spy(mockRestOp.createHttpOperationInstance());
Review Comment:
No need to do spy.
```
AbfsHttpOperation mockHttpOp = Mockito.mock(AbfsHttpOperation.class);
Mockito.doReturn(mockHttpOp).when(mockRestOp).createHttpOperationInstance();
```
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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 #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1456190858
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 40s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 38m 29s | | trunk passed |
| +1 :green_heart: | compile | 0m 41s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 0m 38s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 0m 35s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 42s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 34s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 46s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 32s | | the patch passed |
| +1 :green_heart: | compile | 0m 32s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 0m 32s | | the patch passed |
| +1 :green_heart: | compile | 0m 29s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 0m 29s | | the patch passed |
| -1 :x: | blanks | 0m 0s | [/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/4/artifact/out/blanks-eol.txt) | The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| -0 :warning: | checkstyle | 0m 19s | [/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/4/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt) | hadoop-tools/hadoop-azure: The patch generated 12 new + 7 unchanged - 0 fixed = 19 total (was 7) |
| +1 :green_heart: | mvnsite | 0m 31s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 3s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 4s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 3m 12s | [/patch-unit-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/4/artifact/out/patch-unit-hadoop-tools_hadoop-azure.txt) | hadoop-azure in the patch passed. |
| -1 :x: | asflicense | 0m 38s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/4/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 94m 44s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.fs.azurebfs.services.TestAbfsClientThrottlingAnalyzer |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5446 |
| JIRA Issue | HADOOP-18640 |
| Optional Tests | dupname asflicense codespell detsecrets compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle |
| uname | Linux 6cabcb9c56bb 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 94bab6c212bb2af7792ae25595a1582f68132269 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/4/testReport/ |
| Max. process+thread count | 603 (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-5446/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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 #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1458113928
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 36s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 38m 23s | | trunk passed |
| +1 :green_heart: | compile | 0m 42s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 0m 37s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 0m 34s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 43s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 34s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 22s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 31s | | the patch passed |
| +1 :green_heart: | compile | 0m 33s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 0m 33s | | the patch passed |
| +1 :green_heart: | compile | 0m 30s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 0m 30s | | the patch passed |
| -1 :x: | blanks | 0m 0s | [/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/5/artifact/out/blanks-eol.txt) | The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| -0 :warning: | checkstyle | 0m 19s | [/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/5/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt) | hadoop-tools/hadoop-azure: The patch generated 11 new + 7 unchanged - 0 fixed = 18 total (was 7) |
| +1 :green_heart: | mvnsite | 0m 32s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 0m 24s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 1m 3s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 7s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 3m 13s | [/patch-unit-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/5/artifact/out/patch-unit-hadoop-tools_hadoop-azure.txt) | hadoop-azure in the patch passed. |
| -1 :x: | asflicense | 0m 38s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/5/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 94m 2s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.fs.azurebfs.services.TestAbfsClientThrottlingAnalyzer |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5446 |
| JIRA Issue | HADOOP-18640 |
| Optional Tests | dupname asflicense codespell detsecrets compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle |
| uname | Linux 133ddfe34150 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 4c6f2ae89f22a3d525ec7b7c50ddc9b13687d638 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5446/5/testReport/ |
| Max. process+thread count | 688 (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-5446/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] saxenapranav commented on pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#issuecomment-1462328124
it seems yetus is giving wrong line number. there should be a space on 839.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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] sreeb-msft commented on a diff in pull request #5446: HADOOP-18640: [ABFS] Enabling Client-side Backoff only for new requests
Posted by "sreeb-msft (via GitHub)" <gi...@apache.org>.
sreeb-msft commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1129256701
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -222,6 +224,10 @@ AbfsThrottlingIntercept getIntercept() {
return intercept;
}
+ boolean shouldThrottleRetries() {
+ return throttleRetries;
+ }
+
Review Comment:
Within AbfsRestOperation, would have to do client.getAbfsConfiguration.getShouldThrottleRetries. Would that be more preferable?
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
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