From 01cc6d0bc8236d87eefa646a9ad2161841984fd3 Mon Sep 17 00:00:00 2001 From: Anmol Asrani Date: Thu, 31 Aug 2023 19:40:04 +0530 Subject: [PATCH] HADOOP-18865. ABFS: Add "100-continue" in userAgent if enabled (#5987) Contributed by Anmol Asrani --- .../services/AppendRequestParameters.java | 10 ++++ .../fs/azurebfs/services/AbfsClient.java | 17 ++++++ .../fs/azurebfs/services/ITestAbfsClient.java | 58 +++++++++++++++---- 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AppendRequestParameters.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AppendRequestParameters.java index 57e559a60e..9da6427d65 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AppendRequestParameters.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AppendRequestParameters.java @@ -35,6 +35,7 @@ public enum Mode { private final boolean isAppendBlob; private final String leaseId; private boolean isExpectHeaderEnabled; + private boolean isRetryDueToExpect; public AppendRequestParameters(final long position, final int offset, @@ -50,6 +51,7 @@ public AppendRequestParameters(final long position, this.isAppendBlob = isAppendBlob; this.leaseId = leaseId; this.isExpectHeaderEnabled = isExpectHeaderEnabled; + this.isRetryDueToExpect = false; } public long getPosition() { @@ -80,6 +82,14 @@ public boolean isExpectHeaderEnabled() { return isExpectHeaderEnabled; } + public boolean isRetryDueToExpect() { + return isRetryDueToExpect; + } + + public void setRetryDueToExpect(boolean retryDueToExpect) { + isRetryDueToExpect = retryDueToExpect; + } + public void setExpectHeaderEnabled(boolean expectHeaderEnabled) { isExpectHeaderEnabled = expectHeaderEnabled; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 77b8dcb2b9..45cb538d0b 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -87,6 +87,7 @@ */ public class AbfsClient implements Closeable { public static final Logger LOG = LoggerFactory.getLogger(AbfsClient.class); + public static final String HUNDRED_CONTINUE_USER_AGENT = SINGLE_WHITE_SPACE + HUNDRED_CONTINUE + SEMICOLON; private final URL baseUrl; private final SharedKeyCredentials sharedKeyCredentials; @@ -751,6 +752,15 @@ public AbfsRestOperation append(final String path, final byte[] buffer, } } + // Check if the retry is with "Expect: 100-continue" header being present in the previous request. + if (reqParams.isRetryDueToExpect()) { + String userAgentRetry = userAgent; + // Remove the specific marker related to "Expect: 100-continue" from the User-Agent string. + userAgentRetry = userAgentRetry.replace(HUNDRED_CONTINUE_USER_AGENT, EMPTY_STRING); + requestHeaders.removeIf(header -> header.getName().equalsIgnoreCase(USER_AGENT)); + requestHeaders.add(new AbfsHttpHeader(USER_AGENT, userAgentRetry)); + } + // AbfsInputStream/AbfsOutputStream reuse SAS tokens for better performance String sasTokenForReuse = appendSASTokenToQuery(path, SASTokenProvider.WRITE_OPERATION, abfsUriQueryBuilder, cachedSasToken); @@ -780,6 +790,7 @@ public AbfsRestOperation append(final String path, final byte[] buffer, if (checkUserError(responseStatusCode) && reqParams.isExpectHeaderEnabled()) { LOG.debug("User error, retrying without 100 continue enabled for the given path {}", path); reqParams.setExpectHeaderEnabled(false); + reqParams.setRetryDueToExpect(true); return this.append(path, buffer, reqParams, cachedSasToken, tracingContext); } @@ -1371,6 +1382,12 @@ String initializeUserAgent(final AbfsConfiguration abfsConfiguration, appendIfNotEmpty(sb, ExtensionHelper.getUserAgentSuffix(tokenProvider, EMPTY_STRING), true); + if (abfsConfiguration.isExpectHeaderEnabled()) { + sb.append(SINGLE_WHITE_SPACE); + sb.append(HUNDRED_CONTINUE); + sb.append(SEMICOLON); + } + sb.append(SINGLE_WHITE_SPACE); sb.append(abfsConfiguration.getClusterName()); sb.append(FORWARD_SLASH); diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java index c031e5daa6..18d1e3917f 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java @@ -86,6 +86,7 @@ public final class ITestAbfsClient extends AbstractAbfsIntegrationTest { private static final String ACCOUNT_NAME = "bogusAccountName.dfs.core.windows.net"; private static final String FS_AZURE_USER_AGENT_PREFIX = "Partner Service"; + private static final String HUNDRED_CONTINUE_USER_AGENT = SINGLE_WHITE_SPACE + HUNDRED_CONTINUE + SEMICOLON; private static final String TEST_PATH = "/testfile"; public static final int REDUCED_RETRY_COUNT = 2; public static final int REDUCED_BACKOFF_INTERVAL = 100; @@ -143,15 +144,15 @@ private String getUserAgentString(AbfsConfiguration config, } @Test - public void verifybBasicInfo() throws Exception { + public void verifyBasicInfo() throws Exception { final Configuration configuration = new Configuration(); configuration.addResource(TEST_CONFIGURATION_FILE_NAME); AbfsConfiguration abfsConfiguration = new AbfsConfiguration(configuration, ACCOUNT_NAME); - verifybBasicInfo(getUserAgentString(abfsConfiguration, false)); + verifyBasicInfo(getUserAgentString(abfsConfiguration, false)); } - private void verifybBasicInfo(String userAgentStr) { + private void verifyBasicInfo(String userAgentStr) { Assertions.assertThat(userAgentStr) .describedAs("User-Agent string [" + userAgentStr + "] should be of the pattern: " + this.userAgentStringPattern.pattern()) @@ -180,7 +181,7 @@ public void verifyUserAgentPrefix() ACCOUNT_NAME); String userAgentStr = getUserAgentString(abfsConfiguration, false); - verifybBasicInfo(userAgentStr); + verifyBasicInfo(userAgentStr); Assertions.assertThat(userAgentStr) .describedAs("User-Agent string should contain " + FS_AZURE_USER_AGENT_PREFIX) .contains(FS_AZURE_USER_AGENT_PREFIX); @@ -190,12 +191,47 @@ public void verifyUserAgentPrefix() ACCOUNT_NAME); userAgentStr = getUserAgentString(abfsConfiguration, false); - verifybBasicInfo(userAgentStr); + verifyBasicInfo(userAgentStr); Assertions.assertThat(userAgentStr) .describedAs("User-Agent string should not contain " + FS_AZURE_USER_AGENT_PREFIX) .doesNotContain(FS_AZURE_USER_AGENT_PREFIX); } + /** + * This method represents a unit test for verifying the behavior of the User-Agent header + * with respect to the "Expect: 100-continue" header setting in the Azure Blob File System (ABFS) configuration. + * + * The test ensures that the User-Agent string includes or excludes specific information based on whether the + * "Expect: 100-continue" header is enabled or disabled in the configuration. + * + */ + @Test + public void verifyUserAgentExpectHeader() + throws IOException, IllegalAccessException { + final Configuration configuration = new Configuration(); + configuration.addResource(TEST_CONFIGURATION_FILE_NAME); + configuration.set(ConfigurationKeys.FS_AZURE_USER_AGENT_PREFIX_KEY, FS_AZURE_USER_AGENT_PREFIX); + configuration.setBoolean(ConfigurationKeys.FS_AZURE_ACCOUNT_IS_EXPECT_HEADER_ENABLED, true); + AbfsConfiguration abfsConfiguration = new AbfsConfiguration(configuration, + ACCOUNT_NAME); + String userAgentStr = getUserAgentString(abfsConfiguration, false); + + verifyBasicInfo(userAgentStr); + Assertions.assertThat(userAgentStr) + .describedAs("User-Agent string should contain " + HUNDRED_CONTINUE_USER_AGENT) + .contains(HUNDRED_CONTINUE_USER_AGENT); + + configuration.setBoolean(ConfigurationKeys.FS_AZURE_ACCOUNT_IS_EXPECT_HEADER_ENABLED, false); + abfsConfiguration = new AbfsConfiguration(configuration, + ACCOUNT_NAME); + userAgentStr = getUserAgentString(abfsConfiguration, false); + + verifyBasicInfo(userAgentStr); + Assertions.assertThat(userAgentStr) + .describedAs("User-Agent string should not contain " + HUNDRED_CONTINUE_USER_AGENT) + .doesNotContain(HUNDRED_CONTINUE_USER_AGENT); + } + @Test public void verifyUserAgentWithoutSSLProvider() throws Exception { final Configuration configuration = new Configuration(); @@ -206,14 +242,14 @@ public void verifyUserAgentWithoutSSLProvider() throws Exception { ACCOUNT_NAME); String userAgentStr = getUserAgentString(abfsConfiguration, true); - verifybBasicInfo(userAgentStr); + verifyBasicInfo(userAgentStr); Assertions.assertThat(userAgentStr) .describedAs("User-Agent string should contain sslProvider") .contains(DelegatingSSLSocketFactory.getDefaultFactory().getProviderName()); userAgentStr = getUserAgentString(abfsConfiguration, false); - verifybBasicInfo(userAgentStr); + verifyBasicInfo(userAgentStr); Assertions.assertThat(userAgentStr) .describedAs("User-Agent string should not contain sslProvider") .doesNotContain(DelegatingSSLSocketFactory.getDefaultFactory().getProviderName()); @@ -229,7 +265,7 @@ public void verifyUserAgentClusterName() throws Exception { ACCOUNT_NAME); String userAgentStr = getUserAgentString(abfsConfiguration, false); - verifybBasicInfo(userAgentStr); + verifyBasicInfo(userAgentStr); Assertions.assertThat(userAgentStr) .describedAs("User-Agent string should contain cluster name") .contains(clusterName); @@ -239,7 +275,7 @@ public void verifyUserAgentClusterName() throws Exception { ACCOUNT_NAME); userAgentStr = getUserAgentString(abfsConfiguration, false); - verifybBasicInfo(userAgentStr); + verifyBasicInfo(userAgentStr); Assertions.assertThat(userAgentStr) .describedAs("User-Agent string should not contain cluster name") .doesNotContain(clusterName) @@ -257,7 +293,7 @@ public void verifyUserAgentClusterType() throws Exception { ACCOUNT_NAME); String userAgentStr = getUserAgentString(abfsConfiguration, false); - verifybBasicInfo(userAgentStr); + verifyBasicInfo(userAgentStr); Assertions.assertThat(userAgentStr) .describedAs("User-Agent string should contain cluster type") .contains(clusterType); @@ -267,7 +303,7 @@ public void verifyUserAgentClusterType() throws Exception { ACCOUNT_NAME); userAgentStr = getUserAgentString(abfsConfiguration, false); - verifybBasicInfo(userAgentStr); + verifyBasicInfo(userAgentStr); Assertions.assertThat(userAgentStr) .describedAs("User-Agent string should not contain cluster type") .doesNotContain(clusterType)