Skip to content

Commit 75594b9

Browse files
authored
Merge pull request #4 from saxenapranav/retry_reason_review2
Review-comments for pullRequest 5299
2 parents 7f77ead + 9799dba commit 75594b9

17 files changed

Lines changed: 740 additions & 199 deletions

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,14 @@ public final class AbfsHttpConstants {
112112
public static final char CHAR_STAR = '*';
113113
public static final char CHAR_PLUS = '+';
114114
/**
115-
* Value that differentiates categories of the http_status.<br>
116-
* 100 - 199 : Informational responses<br>
117-
* 200 - 299 : Successful responses<br>
118-
* 300 - 399 : Redirection messages<br>
119-
* 400 - 499 : Client error responses<br>
120-
* 500 - 599 : Server error responses<br>
115+
* Value that differentiates categories of the http_status.
116+
* <pre>
117+
* 100 - 199 : Informational responses
118+
* 200 - 299 : Successful responses
119+
* 300 - 399 : Redirection messages
120+
* 400 - 499 : Client error responses
121+
* 500 - 599 : Server error responses
122+
* </pre>
121123
* */
122124
public static final Integer HTTP_STATUS_CATEGORY_QUOTIENT = 100;
123125

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ public String getErrorCode() {
6666
return this.errorCode;
6767
}
6868

69+
public String getErrorMessage() {
70+
return this.errorMessage;
71+
}
72+
6973
public static List<AzureServiceErrorCode> getAzureServiceCode(int httpStatusCode) {
7074
List<AzureServiceErrorCode> errorCodes = new ArrayList<>();
7175
if (httpStatusCode == UNKNOWN.httpStatusCode) {

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ public class AbfsRestOperation {
7474
private AbfsHttpOperation result;
7575
private AbfsCounters abfsCounters;
7676

77-
private String failureReason = null;
77+
/**
78+
* This variable contains the reason of last API call within the same
79+
* AbfsRestOperation object.
80+
* */
81+
private String failureReason;
7882

7983
/**
8084
* Checks if there is non-null HTTP response.
@@ -242,7 +246,7 @@ private void completeExecute(TracingContext tracingContext)
242246

243247
@VisibleForTesting
244248
String getClientLatency() {
245-
return this.client.getAbfsPerfTracker().getClientLatency();
249+
return client.getAbfsPerfTracker().getClientLatency();
246250
}
247251

248252
/**
@@ -256,7 +260,7 @@ private boolean executeHttpOperation(final int retryCount,
256260

257261
try {
258262
// initialize the HTTP request and open the connection
259-
httpOperation = getHttpOperation();
263+
httpOperation = createHttpOperation();
260264
incrementCounter(AbfsStatistic.CONNECTIONS_MADE, 1);
261265
tracingContext.constructHeader(httpOperation, failureReason);
262266

@@ -347,8 +351,12 @@ private boolean executeHttpOperation(final int retryCount,
347351
return true;
348352
}
349353

354+
/**
355+
* Creates new object of {@link AbfsHttpOperation} with the url, method, and
356+
* requestHeaders fields of the AbfsRestOperation object.
357+
* */
350358
@VisibleForTesting
351-
AbfsHttpOperation getHttpOperation() throws IOException {
359+
AbfsHttpOperation createHttpOperation() throws IOException {
352360
return new AbfsHttpOperation(url, method, requestHeaders);
353361
}
354362

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

Lines changed: 53 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -18,159 +18,83 @@
1818

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

21-
import java.io.IOException;
22-
import java.net.SocketException;
23-
import java.net.UnknownHostException;
24-
import java.util.ArrayList;
21+
import java.util.LinkedList;
2522
import java.util.List;
2623

27-
import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
28-
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_STATUS_CATEGORY_QUOTIENT;
24+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.ClientErrorRetryReason;
25+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.ConnectionResetRetryReason;
26+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.ConnectionTimeoutRetryReason;
27+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.ReadTimeoutRetryReason;
28+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.RetryReasonCategory;
29+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.ServerErrorRetryReason;
30+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.UnknownHostRetryReason;
31+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.UnknownIOExceptionRetryReason;
32+
import org.apache.hadoop.fs.azurebfs.services.retryReasonCategories.UnknownSocketExceptionRetryReason;
33+
2934

3035
/**
31-
* In case of retry, this enum would give the information on the reason for
32-
* previous API call.
36+
* This utility class exposes methods to convert a server response-error to a
37+
* category of error.
3338
* */
34-
public enum RetryReason {
35-
CONNECTION_TIMEOUT(2,
36-
((exceptionCaptured, statusCode, serverErrorMessage) -> {
37-
if (exceptionCaptured != null && "connect timed out".equalsIgnoreCase(
38-
exceptionCaptured.getMessage())) {
39-
return "CT";
40-
}
41-
return null;
42-
})),
43-
READ_TIMEOUT(2, ((exceptionCaptured, statusCode, serverErrorMessage) -> {
44-
if (exceptionCaptured != null && "Read timed out".equalsIgnoreCase(
45-
exceptionCaptured.getMessage())) {
46-
return "RT";
47-
}
48-
return null;
49-
})),
50-
UNKNOWN_HOST(2, ((exceptionCaptured, statusCode, serverErrorMessage) -> {
51-
if (exceptionCaptured instanceof UnknownHostException) {
52-
return "UH";
53-
}
54-
return null;
55-
})),
56-
CONNECTION_RESET(2, ((exceptionCaptured, statusCode, serverErrorMessage) -> {
57-
if (exceptionCaptured != null && exceptionCaptured.getMessage() != null
58-
&& exceptionCaptured.getMessage().contains("Connection reset")) {
59-
return "CR";
60-
}
61-
return null;
62-
})),
63-
STATUS_5XX(0, ((exceptionCaptured, statusCode, serverErrorMessage) -> {
64-
if (statusCode == null || statusCode / HTTP_STATUS_CATEGORY_QUOTIENT != 5) {
65-
return null;
66-
}
67-
if (statusCode == HTTP_UNAVAILABLE) {
68-
serverErrorMessage = serverErrorMessage.split(System.lineSeparator(),
69-
2)[0];
70-
if ("Ingress is over the account limit.".equalsIgnoreCase(
71-
serverErrorMessage)) {
72-
return "ING";
73-
}
74-
if ("Egress is over the account limit.".equalsIgnoreCase(
75-
serverErrorMessage)) {
76-
return "EGR";
77-
}
78-
if ("Operations per second is over the account limit.".equalsIgnoreCase(
79-
serverErrorMessage)) {
80-
return "OPR";
81-
}
82-
return HTTP_UNAVAILABLE + "";
83-
}
84-
return statusCode + "";
85-
})),
86-
STATUS_4XX(0, ((exceptionCaptured, statusCode, serverErrorMessage) -> {
87-
if (statusCode == null || statusCode / HTTP_STATUS_CATEGORY_QUOTIENT != 4) {
88-
return null;
89-
}
90-
return statusCode + "";
91-
})),
92-
UNKNOWN_SOCKET_EXCEPTION(1,
93-
((exceptionCaptured, statusCode, serverErrorMessage) -> {
94-
if (exceptionCaptured instanceof SocketException) {
95-
return "SE";
96-
}
97-
return null;
98-
})),
99-
UNKNOWN_IO_EXCEPTION(0,
100-
((exceptionCaptured, statusCode, serverErrorMessage) -> {
101-
if (exceptionCaptured instanceof IOException) {
102-
return "IOE";
103-
}
104-
return null;
105-
}));
106-
107-
private RetryReasonAbbreviationCreator retryReasonAbbreviationCreator = null;
108-
109-
private int rank = 0;
39+
class RetryReason {
11040

11141
/**
112-
* Constructor to have rank and the implementation of {@link RetryReasonAbbreviationCreator}.
113-
* @param rank rank of a given enum. For example SocketTimeoutException is
114-
* subclass of IOException. Rank of SocketTimeoutException enum has to be
115-
* more than that of IOException enum.
116-
* @param abbreviationCreator The implementation of {@link RetryReasonAbbreviationCreator}
117-
* which would give the information if a given enum can be mapped to an error or not.
42+
* Linked-list of the implementations of RetryReasonCategory. The objects in the
43+
* list are arranged by the rank of their significance.
44+
* <ul>
45+
* <li>ServerError (statusCode==5XX), ClientError (statusCode==4XX) are
46+
* independent of other retryReason categories.</li>
47+
* <li>Since {@link java.net.SocketException} is subclass of
48+
* {@link java.io.IOException},
49+
* hence, {@link UnknownIOExceptionRetryReason} is placed before
50+
* {@link UnknownSocketExceptionRetryReason}</li>
51+
* <li>Since, connectionTimeout, readTimeout, and connectionReset are
52+
* {@link java.net.SocketTimeoutException} exceptions with different messages,
53+
* hence, {@link ConnectionTimeoutRetryReason}, {@link ReadTimeoutRetryReason},
54+
* {@link ConnectionResetRetryReason} are above {@link UnknownIOExceptionRetryReason}.
55+
* There is no order between the three reasons as they are differentiated
56+
* by exception-message.</li>
57+
* <li>Since, {@link java.net.UnknownHostException} is subclass of
58+
* {@link java.io.IOException}, {@link UnknownHostRetryReason} is placed
59+
* over {@link UnknownIOExceptionRetryReason}</li>
60+
* </ul>
11861
* */
119-
RetryReason(int rank,
120-
RetryReasonAbbreviationCreator abbreviationCreator) {
121-
this.rank = rank;
122-
this.retryReasonAbbreviationCreator = abbreviationCreator;
123-
}
62+
private static List<RetryReasonCategory> rankedReasonCategories
63+
= new LinkedList<RetryReasonCategory>() {{
64+
add(new ServerErrorRetryReason());
65+
add(new ClientErrorRetryReason());
66+
add(new UnknownIOExceptionRetryReason());
67+
add(new UnknownSocketExceptionRetryReason());
68+
add(new ConnectionTimeoutRetryReason());
69+
add(new ReadTimeoutRetryReason());
70+
add(new UnknownHostRetryReason());
71+
add(new ConnectionResetRetryReason());
72+
}};
12473

125-
private static List<RetryReason> retryReasonSortedList;
74+
private RetryReason() {
12675

127-
/**
128-
* Synchronized method to assign sorted list in {@link RetryReason#retryReasonSortedList}.
129-
* Method would check if list is assigned or not. If yes, method would return. This is required
130-
* because multiple threads could be waiting to get into this method, and once a thread is done
131-
* with this method, other thread would get into this method. Since the list would be assigned by
132-
* first thread, the second thread need not run the whole mechanism of sorting.
133-
* The enums are sorted on the ascending order of their rank.
134-
* */
135-
private static synchronized void sortRetryReason() {
136-
if (retryReasonSortedList != null) {
137-
return;
138-
}
139-
List<RetryReason> list = new ArrayList<>();
140-
for (RetryReason reason : values()) {
141-
list.add(reason);
142-
}
143-
list.sort((c1, c2) -> {
144-
return c1.rank - c2.rank;
145-
});
146-
retryReasonSortedList = list;
14776
}
14877

14978
/**
15079
* Method to get correct abbreviation for a given set of exception, statusCode,
15180
* storageStatusCode.
152-
* Method would iterate through the {@link RetryReason#retryReasonSortedList},
153-
* and would return the abbreviation returned by highest enum to be applicable on the group.
154-
* For example, if SocketTimeoutException(rank 2) and IOException(rank 0) can be
155-
* applied on the group, the abbreviation of SocketTimeoutException has to be returned.
15681
*
15782
* @param ex exception caught during server communication.
15883
* @param statusCode statusCode in the server response.
15984
* @param storageErrorMessage storageErrorMessage in the server response.
85+
*
86+
* @return abbreviation for the the given set of exception, statusCode, storageStatusCode.
16087
* */
16188
static String getAbbreviation(Exception ex,
16289
Integer statusCode,
16390
String storageErrorMessage) {
16491
String result = null;
165-
if (retryReasonSortedList == null) {
166-
sortRetryReason();
167-
}
168-
for (RetryReason retryReason : retryReasonSortedList) {
169-
String enumCapturedAndAbbreviate
170-
= retryReason.retryReasonAbbreviationCreator.capturableAndGetAbbreviation(
171-
ex, statusCode, storageErrorMessage);
172-
if (enumCapturedAndAbbreviate != null) {
173-
result = enumCapturedAndAbbreviate;
92+
for (RetryReasonCategory retryReasonCategory : rankedReasonCategories) {
93+
final String abbreviation
94+
= retryReasonCategory.captureAndGetAbbreviation(ex,
95+
statusCode, storageErrorMessage);
96+
if (abbreviation != null) {
97+
result = abbreviation;
17498
}
17599
}
176100
return result;

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

Lines changed: 0 additions & 40 deletions
This file was deleted.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
* <p>
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
* <p>
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package org.apache.hadoop.fs.azurebfs.services;
20+
21+
public class RetryReasonConstants {
22+
public static final String CONNECTION_TIMEOUT_JDK_MESSAGE = "connect timed out";
23+
public static final String READ_TIMEOUT_JDK_MESSAGE = "Read timed out";
24+
public static final String CONNECTION_RESET_MESSAGE = "Connection reset";
25+
public static final String OPERATION_BREACH_MESSAGE = "Operations per second is over the account limit.";
26+
public static final String CONNECTION_RESET_ABBREVIATION = "CR";
27+
public static final String CONNECTION_TIMEOUT_ABBREVIATION = "CT";
28+
public static final String READ_TIMEOUT_ABBREVIATION = "RT";
29+
public static final String INGRESS_LIMIT_BREACH_ABBREVIATION = "ING";
30+
public static final String EGRESS_LIMIT_BREACH_ABBREVIATION = "EGR";
31+
public static final String OPERATION_LIMIT_BREACH_ABBREVIATION = "OPR";
32+
public static final String UNKNOWN_HOST_EXCEPTION_ABBREVIATION = "UH";
33+
public static final String IO_EXCEPTION_ABBREVIATION = "IOE";
34+
public static final String SOCKET_EXCEPTION_ABBREVIATION = "SE";
35+
}

0 commit comments

Comments
 (0)