[
https://issues.apache.org/jira/browse/HADOOP-19096?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17831006#comment-17831006
]
ASF GitHub Bot commented on HADOOP-19096:
-----------------------------------------
rakeshadr commented on code in PR #6276:
URL: https://github.com/apache/hadoop/pull/6276#discussion_r1539600695
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/retryReasonCategories/ServerErrorRetryReason.java:
##########
@@ -56,10 +58,14 @@ String getAbbreviation(final Integer statusCode,
splitedServerErrorMessage)) {
return EGRESS_LIMIT_BREACH_ABBREVIATION;
}
- if (OPERATION_BREACH_MESSAGE.equalsIgnoreCase(
+ if (TPS_OVER_ACCOUNT_LIMIT.getErrorMessage().equalsIgnoreCase(
splitedServerErrorMessage)) {
return OPERATION_LIMIT_BREACH_ABBREVIATION;
Review Comment:
Could you please rename `OPERATION_LIMIT_BREACH_ABBREVIATION` to
`TPS_LIMIT_BREACH_ABBREVIATION`, this would maintain consistency with the
naming convention with other constants.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java:
##########
@@ -333,57 +355,44 @@ private boolean executeHttpOperation(final int retryCount,
}
return false;
} catch (IOException ex) {
+ wasExceptionThrown = true;
if (LOG.isDebugEnabled()) {
LOG.debug("HttpRequestFailure: {}, {}", httpOperation, ex);
}
failureReason = RetryReason.getAbbreviation(ex, -1, "");
retryPolicy = client.getRetryPolicy(failureReason);
- wasIOExceptionThrown = true;
if (!retryPolicy.shouldRetry(retryCount, -1)) {
throw new InvalidAbfsRestOperationException(ex, retryCount);
}
return false;
} finally {
- int status = httpOperation.getStatusCode();
/*
- A status less than 300 (2xx range) or greater than or equal
- to 500 (5xx range) should contribute to throttling metrics being
updated.
- Less than 200 or greater than or equal to 500 show failed operations.
2xx
- range contributes to successful operations. 3xx range is for redirects
- and 4xx range is for user errors. These should not be a part of
- throttling backoff computation.
+ Updating Client Side Throttling Metrics for relevant response status
codes.
+ 1. Status code in 2xx range: Successful Operations should contribute
+ 2. Status code in 3xx range: Redirection Operations should not
contribute
+ 3. Status code in 4xx range: User Errors should not contribute
+ 4. Status code is 503: Throttling Error should contribute as following:
+ a. 503, Ingress Over Account Limit: Should Contribute
+ b. 503, Egress Over Account Limit: Should Contribute
+ c. 503, TPS Over Account Limit: Should Contribute
+ d. 503, Other Server Throttling: Should not contribute
+ 5. Status code in 5xx range other than 503: Should not contribute
+ 6. IOException and UnknownHostExceptions: Should not contribute
*/
- boolean updateMetricsResponseCode = (status <
HttpURLConnection.HTTP_MULT_CHOICE
- || status >= HttpURLConnection.HTTP_INTERNAL_ERROR);
-
- /*
- Connection Timeout failures should not contribute to throttling
- In case the current request fails with Connection Timeout we will have
- ioExceptionThrown true and failure reason as CT
- In case the current request failed with 5xx, failure reason will be
- updated after finally block but wasIOExceptionThrown will be false;
- */
- boolean isCTFailure =
CONNECTION_TIMEOUT_ABBREVIATION.equals(failureReason) && wasIOExceptionThrown;
-
- if (updateMetricsResponseCode && !isCTFailure) {
+ int statusCode = httpOperation.getStatusCode();
+ boolean shouldUpdateCSTMetrics = (statusCode <
HttpURLConnection.HTTP_MULT_CHOICE // Case 1
+ || INGRESS_LIMIT_BREACH_ABBREVIATION.equals(failureReason) //
Case 4.a
+ || EGRESS_LIMIT_BREACH_ABBREVIATION.equals(failureReason) //
Case 4.b
+ || OPERATION_LIMIT_BREACH_ABBREVIATION.equals(failureReason)) //
Case 4.c
+ && !wasExceptionThrown; // Case 6
Review Comment:
Can you please encapsulate the chain of conditions to a method
`#shouldUpdateCSTMetrics()`
> [ABFS] Enhancing Client-Side Throttling Metrics Updation Logic
> --------------------------------------------------------------
>
> Key: HADOOP-19096
> URL: https://issues.apache.org/jira/browse/HADOOP-19096
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure
> Affects Versions: 3.4.1
> Reporter: Anuj Modi
> Assignee: Anuj Modi
> Priority: Major
> Labels: pull-request-available
> Fix For: 3.4.1
>
>
> ABFS has a client-side throttling mechanism which works on the metrics
> collected from past requests made. I requests are getting failed due to
> throttling at server, we update our metrics and client side backoff is
> calculated based on those metrics.
> This PR enhances the logic to decide which requests should be considered to
> compute client side backoff interval as follows:
> For each request made by ABFS driver, we will determine if they should
> contribute to Client-Side Throttling based on the status code and result:
> # Status code in 2xx range: Successful Operations should contribute.
> # Status code in 3xx range: Redirection Operations should not contribute.
> # Status code in 4xx range: User Errors should not contribute.
> # Status code is 503: Throttling Error should contribute only if they are
> due to client limits breach as follows:
> ## 503, Ingress Over Account Limit: Should Contribute
> ## 503, Egress Over Account Limit: Should Contribute
> ## 503, TPS Over Account Limit: Should Contribute
> ## 503, Other Server Throttling: Should not Contribute.
> # Status code in 5xx range other than 503: Should not Contribute.
> # IOException and UnknownHostExceptions: Should not Contribute.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
