anujmodi2021 commented on code in PR #5711:
URL: https://github.com/apache/hadoop/pull/5711#discussion_r1219286147
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java:
##########
@@ -322,34 +326,46 @@ private boolean executeHttpOperation(final int retryCount,
}
failureReason = RetryReason.getAbbreviation(ex, -1, "");
-
- if (!client.getRetryPolicy().shouldRetry(retryCount, -1)) {
+ retryPolicy = client.getRetryPolicy(failureReason);
+ 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.
- */
+ /**
+ * 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.
+ * */
boolean updateMetricsResponseCode = (status <
HttpURLConnection.HTTP_MULT_CHOICE
|| status >= HttpURLConnection.HTTP_INTERNAL_ERROR);
- if (updateMetricsResponseCode) {
+
+ /**
+ * Connection Timeout failures should not contribute to throttling
+ * In case the current request fails with Connection Timeout we will have
+ * status code as -1 and failure reason as CT
+ * In case the current request failed with 5xx, failure reason will be
+ * updated after finally block but status code will not be -1;
+ */
+ boolean isCTFailure =
CONNECTION_TIMEOUT_ABBREVIATION.equals(failureReason) && status == -1;
Review Comment:
The problem here is if we try to get failure reason inside finally block, it
will override the failure reason already computed in one of the catch blocks
above. For asserting that the failure reason is CT, we need exception thrown so
that cannot be moved outside catch block.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
