[
https://issues.apache.org/jira/browse/HADOOP-18647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17699620#comment-17699620
]
ASF GitHub Bot commented on HADOOP-18647:
-----------------------------------------
steveloughran commented on code in PR #5437:
URL: https://github.com/apache/hadoop/pull/5437#discussion_r1133900074
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestTracingContext.java:
##########
@@ -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);
+ }
+
+ @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);
Review Comment:
again, newline
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestTracingContext.java:
##########
@@ -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);
Review Comment:
can put that .isEqual to on a new line, just so it stands out more
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/TracingContext.java:
##########
@@ -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;
Review Comment:
add a javadoc
> x-ms-client-request-id to have some way that identifies retry of an API.
> ------------------------------------------------------------------------
>
> Key: HADOOP-18647
> URL: https://issues.apache.org/jira/browse/HADOOP-18647
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure
> Reporter: Pranav Saxena
> Assignee: Pranav Saxena
> Priority: Minor
> Labels: pull-request-available
> Fix For: 3.4.0
>
>
> In case primaryRequestId in x-ms-client-request-id is empty-string, the
> retry's primaryRequestId has to contain last part of clientRequestId UUID.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
