Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HADOOP-18647. x-ms-client-request-id to have some way that identifies retry of an API. #5437

Merged
merged 42 commits into from Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
e70429c
retryReason foundations.
saxenapranav Jan 12, 2023
2ddd8c9
send failureReasons for adding in x-ms-client-request-id
saxenapranav Jan 12, 2023
cc2deb2
added heuristics in RetryReason
saxenapranav Jan 13, 2023
eece65f
fixed test for connect timed out
saxenapranav Jan 13, 2023
40adc3e
testabfsrestOperation: to add 5xx tests
saxenapranav Jan 13, 2023
77c9d21
test added
saxenapranav Jan 13, 2023
b5d176b
test: testClientRequestIdForConnectAndReadTimeoutRetry
saxenapranav Jan 13, 2023
c980762
small refactors
saxenapranav Jan 13, 2023
34fe029
removed non-required capture interface
saxenapranav Jan 14, 2023
7acee90
ASF license; javadocs on the interface.
saxenapranav Jan 14, 2023
b7d7121
minimize TestAbfsRestOperation class with added methods for common te…
saxenapranav Jan 14, 2023
1c3f2ee
added javadocs in RetryReason
saxenapranav Jan 14, 2023
9201ab7
removal of non-required new-lines
saxenapranav Jan 14, 2023
77eb790
small refactors
saxenapranav Jan 14, 2023
32e69cb
checkstyle: magic number
saxenapranav Jan 17, 2023
7f77ead
review refactors
saxenapranav Jan 18, 2023
90fce12
review WIP
saxenapranav Feb 22, 2023
06af705
removing enums and having classes with implementation of RetryReasonA…
saxenapranav Feb 22, 2023
62179bc
method change in RetryReasonAbbreviationCreator
saxenapranav Feb 22, 2023
270545d
asf license
saxenapranav Feb 22, 2023
de424e3
javadocs + small refactors
saxenapranav Feb 22, 2023
94810b8
fixed access modifier, only public method in abstract class is captur…
saxenapranav Feb 23, 2023
dad7b61
javadoc for RetryReason; package name change
saxenapranav Feb 23, 2023
ac5593e
RetryReasonConstants
saxenapranav Feb 23, 2023
77aaa01
asf license in RetryReasonConstants
saxenapranav Feb 23, 2023
c49ab44
constants in TestAbfsRestOperationMockFailures
saxenapranav Feb 23, 2023
e1d38a0
TestRetryReason
saxenapranav Feb 23, 2023
e37d2e2
javadocs wip
saxenapranav Feb 23, 2023
8c4ac80
javadocs on retry-categories
saxenapranav Feb 23, 2023
9799dba
javadoc fix in the RetryReasonCategory
saxenapranav Feb 27, 2023
75594b9
Merge pull request #4 from saxenapranav/retry_reason_review2
saxenapranav Feb 27, 2023
8aa5e1e
basic code for primaryRequestId over retry; test added. More tests WIP
saxenapranav Feb 27, 2023
86cfc3e
Merge branch 'trunk' into retryReason-threadLevelInfo1
saxenapranav Feb 27, 2023
003bbfb
testRetryPrimaryRequestIdWhenInitiallySuppliedNonEmpty; testRetryPrim…
saxenapranav Feb 27, 2023
e3ba294
checkstyle issue
saxenapranav Feb 27, 2023
b33072f
Merge branch 'retry_reason' into retryReason-threadLevelInfo1
saxenapranav Feb 28, 2023
59a47bf
checkstyle
saxenapranav Feb 28, 2023
29edc5e
documentation
saxenapranav Feb 28, 2023
4a739aa
assertion description change
saxenapranav Feb 28, 2023
e15ce58
backmerge apache/hadoop:trunk
saxenapranav Mar 9, 2023
f6283a9
comment, condition small refactor
saxenapranav Mar 9, 2023
ff93fee
javadoc + small refactor as per review
saxenapranav Mar 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -62,6 +62,7 @@ public class TracingContext {
private Listener listener = null; // null except when testing
//final concatenated ID list set into x-ms-client-request-id header
private String header = EMPTY_STRING;
private String primaryRequestIdForRetry;
steveloughran marked this conversation as resolved.
Show resolved Hide resolved

private static final Logger LOG = LoggerFactory.getLogger(AbfsClient.class);
public static final int MAX_CLIENT_CORRELATION_ID_LENGTH = 72;
Expand Down Expand Up @@ -152,17 +153,17 @@ public void setListener(Listener listener) {
* X_MS_CLIENT_REQUEST_ID header of the http operation
* @param httpOperation AbfsHttpOperation instance to set header into
* connection
* @param previousFailure List of failures seen before this API trigger on
* same operation from AbfsClient.
* @param previousFailure Failure seen before this API trigger on same operation
* from AbfsClient.
*/
public void constructHeader(AbfsHttpOperation httpOperation, String previousFailure) {
clientRequestId = UUID.randomUUID().toString();
switch (format) {
case ALL_ID_FORMAT: // Optional IDs (e.g. streamId) may be empty
header =
clientCorrelationID + ":" + clientRequestId + ":" + fileSystemID + ":"
+ primaryRequestId + ":" + streamID + ":" + opType + ":"
+ retryCount;
+ getPrimaryRequestIdForHeader(retryCount > 0) + ":" + streamID
+ ":" + opType + ":" + retryCount;
header = addFailureReasons(header, previousFailure);
break;
case TWO_ID_FORMAT:
Expand All @@ -175,6 +176,31 @@ public void constructHeader(AbfsHttpOperation httpOperation, String previousFail
listener.callTracingHeaderValidator(header, format);
}
httpOperation.setRequestProperty(HttpHeaderConfigurations.X_MS_CLIENT_REQUEST_ID, header);
/*
* In case the primaryRequestId is an empty-string and if it is the first try to
* API call (previousFailure shall be null), maintain the last part of clientRequestId's
* UUID in primaryRequestIdForRetry. This field shall be used as primaryRequestId part
* of the x-ms-client-request-id header in case of retry of the same API-request.
*/
if (primaryRequestId.isEmpty() && previousFailure == null) {
String[] clientRequestIdParts = clientRequestId.split("-");
primaryRequestIdForRetry = clientRequestIdParts[
clientRequestIdParts.length - 1];
}
}

/**
* Provide value to be used as primaryRequestId part of x-ms-client-request-id header.
* @param isRetry define if it's for a retry case.
* @return {@link #primaryRequestIdForRetry}:If the {@link #primaryRequestId}
* is an empty-string, and it's a retry iteration.
* {@link #primaryRequestId} for other cases.
*/
private String getPrimaryRequestIdForHeader(final Boolean isRetry) {
if (!primaryRequestId.isEmpty() || !isRetry) {
return primaryRequestId;
}
return primaryRequestIdForRetry;
}

private String addFailureReasons(final String header,
Expand Down
Expand Up @@ -31,12 +31,14 @@
import org.junit.AssumptionViolatedException;
import org.junit.Ignore;
import org.junit.Test;
import org.mockito.Mockito;

import org.apache.hadoop.fs.CommonPathCapabilities;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
import org.apache.hadoop.fs.azurebfs.constants.FSOperationType;
import org.apache.hadoop.fs.azurebfs.enums.Trilean;
import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation;
import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
import org.apache.hadoop.fs.azurebfs.services.AuthType;
import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
Expand Down Expand Up @@ -198,4 +200,72 @@ public void testExternalOps() throws Exception {
fs.getAbfsStore().setNamespaceEnabled(Trilean.TRUE);
fs.access(new Path("/"), FsAction.READ);
}

@Test
public void testRetryPrimaryRequestIdWhenInitiallySuppliedEmpty() throws Exception {
final AzureBlobFileSystem fs = getFileSystem();
final String fileSystemId = fs.getFileSystemId();
final String clientCorrelationId = fs.getClientCorrelationId();
final TracingHeaderFormat tracingHeaderFormat = TracingHeaderFormat.ALL_ID_FORMAT;
TracingContext tracingContext = new TracingContext(clientCorrelationId,
fileSystemId, FSOperationType.CREATE_FILESYSTEM, tracingHeaderFormat, new TracingHeaderValidator(
fs.getAbfsStore().getAbfsConfiguration().getClientCorrelationId(),
fs.getFileSystemId(), FSOperationType.CREATE_FILESYSTEM, false,
0));
AbfsHttpOperation abfsHttpOperation = Mockito.mock(AbfsHttpOperation.class);
Mockito.doNothing().when(abfsHttpOperation).setRequestProperty(Mockito.anyString(), Mockito.anyString());
tracingContext.constructHeader(abfsHttpOperation, null);
String header = tracingContext.getHeader();
String clientRequestIdUsed = header.split(":")[1];
String[] clientRequestIdUsedParts = clientRequestIdUsed.split("-");
String assertionPrimaryId = clientRequestIdUsedParts[clientRequestIdUsedParts.length - 1];

tracingContext.setRetryCount(1);
tracingContext.setListener(new TracingHeaderValidator(
fs.getAbfsStore().getAbfsConfiguration().getClientCorrelationId(),
fs.getFileSystemId(), FSOperationType.CREATE_FILESYSTEM, false,
1));

tracingContext.constructHeader(abfsHttpOperation, "RT");
header = tracingContext.getHeader();
String primaryRequestId = header.split(":")[3];

Assertions.assertThat(primaryRequestId)
.describedAs("PrimaryRequestId in a retried request's "
+ "tracingContext should be equal to last part of original "
+ "request's clientRequestId UUID").isEqualTo(assertionPrimaryId);
steveloughran marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void testRetryPrimaryRequestIdWhenInitiallySuppliedNonEmpty() throws Exception {
final AzureBlobFileSystem fs = getFileSystem();
final String fileSystemId = fs.getFileSystemId();
final String clientCorrelationId = fs.getClientCorrelationId();
final TracingHeaderFormat tracingHeaderFormat = TracingHeaderFormat.ALL_ID_FORMAT;
TracingContext tracingContext = new TracingContext(clientCorrelationId,
fileSystemId, FSOperationType.CREATE_FILESYSTEM, tracingHeaderFormat, new TracingHeaderValidator(
fs.getAbfsStore().getAbfsConfiguration().getClientCorrelationId(),
fs.getFileSystemId(), FSOperationType.CREATE_FILESYSTEM, false,
0));
tracingContext.setPrimaryRequestID();
AbfsHttpOperation abfsHttpOperation = Mockito.mock(AbfsHttpOperation.class);
Mockito.doNothing().when(abfsHttpOperation).setRequestProperty(Mockito.anyString(), Mockito.anyString());
tracingContext.constructHeader(abfsHttpOperation, null);
String header = tracingContext.getHeader();
String assertionPrimaryId = header.split(":")[3];

tracingContext.setRetryCount(1);
tracingContext.setListener(new TracingHeaderValidator(
fs.getAbfsStore().getAbfsConfiguration().getClientCorrelationId(),
fs.getFileSystemId(), FSOperationType.CREATE_FILESYSTEM, false,
1));

tracingContext.constructHeader(abfsHttpOperation, "RT");
header = tracingContext.getHeader();
String primaryRequestId = header.split(":")[3];

Assertions.assertThat(primaryRequestId)
.describedAs("PrimaryRequestId in a retried request's "
+ "tracingContext should be equal to PrimaryRequestId in the original request.").isEqualTo(assertionPrimaryId);
steveloughran marked this conversation as resolved.
Show resolved Hide resolved
}
}
Expand Up @@ -130,6 +130,9 @@ private void validateBasicFormat(String[] idList) {
}
Assertions.assertThat(idList[5]).describedAs("Operation name incorrect")
.isEqualTo(operation.toString());
if (idList[6].contains("_")) {
idList[6] = idList[6].split("_")[0];
}
int retryCount = Integer.parseInt(idList[6]);
Assertions.assertThat(retryCount)
.describedAs("Retry was required due to issue on server side")
Expand Down