mukund-thakur commented on code in PR #6276:
URL: https://github.com/apache/hadoop/pull/6276#discussion_r1540174681
##########
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:
+1 on this.
--
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-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
