Browse Source

HADOOP-18865. ABFS: Add "100-continue" in userAgent if enabled (#5987)

Contributed by Anmol Asrani
Anmol Asrani 1 year ago
parent
commit
01cc6d0bc8

+ 10 - 0
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AppendRequestParameters.java

@@ -35,6 +35,7 @@ public class AppendRequestParameters {
   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 class AppendRequestParameters {
     this.isAppendBlob = isAppendBlob;
     this.leaseId = leaseId;
     this.isExpectHeaderEnabled = isExpectHeaderEnabled;
+    this.isRetryDueToExpect = false;
   }
 
   public long getPosition() {
@@ -80,6 +82,14 @@ public class AppendRequestParameters {
     return isExpectHeaderEnabled;
   }
 
+  public boolean isRetryDueToExpect() {
+    return isRetryDueToExpect;
+  }
+
+  public void setRetryDueToExpect(boolean retryDueToExpect) {
+    isRetryDueToExpect = retryDueToExpect;
+  }
+
   public void setExpectHeaderEnabled(boolean expectHeaderEnabled) {
     isExpectHeaderEnabled = expectHeaderEnabled;
   }

+ 17 - 0
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

@@ -87,6 +87,7 @@ import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceError
  */
 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 class AbfsClient implements Closeable {
       }
     }
 
+    // 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 class AbfsClient implements Closeable {
       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 @@ public class AbfsClient implements Closeable {
     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);

+ 47 - 11
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 @@ public final class ITestAbfsClient extends AbstractAbfsIntegrationTest {
   }
 
   @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 final class ITestAbfsClient extends AbstractAbfsIntegrationTest {
         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 final class ITestAbfsClient extends AbstractAbfsIntegrationTest {
         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 final class ITestAbfsClient extends AbstractAbfsIntegrationTest {
         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 final class ITestAbfsClient extends AbstractAbfsIntegrationTest {
         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 final class ITestAbfsClient extends AbstractAbfsIntegrationTest {
         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 final class ITestAbfsClient extends AbstractAbfsIntegrationTest {
         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 final class ITestAbfsClient extends AbstractAbfsIntegrationTest {
         ACCOUNT_NAME);
     userAgentStr = getUserAgentString(abfsConfiguration, false);
 
-    verifybBasicInfo(userAgentStr);
+    verifyBasicInfo(userAgentStr);
     Assertions.assertThat(userAgentStr)
       .describedAs("User-Agent string should not contain cluster type")
       .doesNotContain(clusterType)