HADOOP-17811: ABFS ExponentialRetryPolicy doesn't pick up configuration values

hadoop-tools/hadoop-azure/dev-support/findbugs-exclude.xml

@@ -83,4 +83,12 @@
     <Bug pattern="IS2_INCONSISTENT_SYNC" />
   </Match>
 
+  <!-- See comment in code on the read(long,byte[],int,int) method. Usage is safe based
+  on a conditional property. Either a properly synchronized parent class is called, or
+  no resources needing synchronization are used. -->
+  <Match>
+    <Class name="org.apache.hadoop.fs.azure.NativeAzureFileSystem$NativeAzureFsInputStream" />
+    <Field name="in" />
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>
  </FindBugsFilter>

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

@@ -1524,7 +1524,7 @@ private void initializeClient(URI uri, String fileSystemName,
   private AbfsClientContext populateAbfsClientContext() {
     return new AbfsClientContextBuilder()
         .withExponentialRetryPolicy(
-            new ExponentialRetryPolicy(abfsConfiguration.getMaxIoRetries()))
+            new ExponentialRetryPolicy(abfsConfiguration))
         .withAbfsCounters(abfsCounters)
         .withAbfsPerfTracker(abfsPerfTracker)
         .build();

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

@@ -21,6 +21,7 @@
 import java.util.Random;
 import java.net.HttpURLConnection;
 
+import org.apache.hadoop.fs.azurebfs.AbfsConfiguration;
 import org.apache.hadoop.thirdparty.com.google.common.annotations.VisibleForTesting;
 
 /**
@@ -89,6 +90,16 @@ public ExponentialRetryPolicy(final int maxIoRetries) {
         DEFAULT_CLIENT_BACKOFF);
   }
 
+  /**
+   * Initializes a new instance of the {@link ExponentialRetryPolicy} class.
+   *
+   * @param conf The {@link AbfsConfiguration} from which to retrieve retry configuration.
+   */
+  public ExponentialRetryPolicy(AbfsConfiguration conf) {
+    this(conf.getMaxIoRetries(), conf.getMinBackoffIntervalMilliseconds(), conf.getMaxBackoffIntervalMilliseconds(),
+        conf.getBackoffIntervalMilliseconds());
+  }
+
   /**
    * Initializes a new instance of the {@link ExponentialRetryPolicy} class.
    *

hadoop-tools/hadoop-azure/src/site/markdown/abfs.md

@@ -338,7 +338,7 @@ with the following configurations.
  retries. Default value is 5.
 * `fs.azure.oauth.token.fetch.retry.min.backoff.interval`: Minimum back-off
   interval. Added to the retry interval computed from delta backoff. By
-   default this si set as 0. Set the interval in milli seconds.
+   default this is set as 0. Set the interval in milli seconds.
 * `fs.azure.oauth.token.fetch.retry.max.backoff.interval`: Maximum back-off
 interval. Default value is 60000 (sixty seconds). Set the interval in milli
 seconds.
@@ -803,6 +803,28 @@ Currently this is used only for the server call retry logic. Used within
 AbfsClient class as part of the ExponentialRetryPolicy. The value should be
 greater than or equal to 0.
 
+`fs.azure.io.retry.min.backoff.interval`: Sets the minimum backoff interval for
+retries of IO operations. Currently this is used only for the server call retry
+logic. Used within AbfsClient class as part of the EponentialRetryPolicy. This
+value indicates the smallest interval (in milliseconds) to wait before retrying
+an IO operation. The default value is 3000 (3 seconds).
+
+`fs.azure.io.retry.max.backoff.interval`: Sets the maximum backoff interval for
+retries of IO operations. Currently this is used only for the server call retry
+logic. Used within AbfsClient class as part of the EponentialRetryPolicy. This
+value indicates the largest interval (in milliseconds) to wait before retrying
+an IO operation. The default value is 30000 (30 seconds).
+
+`fs.azure.io.retry.backoff.interval`: Sets the default backoff interval for
+retries of IO operations. Currently this is used only for the server call retry
+logic. Used within AbfsClient class as part of the EponentialRetryPolicy. This
+value is used to compute a random delta between 80% and 120% of the specified
+value. This random delta is then multiplied by an exponent of the current IO
+retry number (i.e., the default is multiplied by `2^(retryNum - 1)`) and then
+contstrained within the range of [`fs.azure.io.retry.min.backoff.interval`,
+`fs.azure.io.retry.max.backoff.interval`] to determine the amount of time to
+wait before the next IO retry attempt. The default value is 3000 (3 seconds).
+
 `fs.azure.write.request.size`: To set the write buffer size. Specify the value
 in bytes. The value should be between 16384 to 104857600 both inclusive (16 KB
 to 100 MB). The default value will be 8388608 (8 MB).

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java

@@ -18,6 +18,11 @@
 
 package org.apache.hadoop.fs.azurebfs.services;
 
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_BACKOFF_INTERVAL;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MAX_BACKOFF_INTERVAL;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MAX_IO_RETRIES;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MIN_BACKOFF_INTERVAL;
+
 import java.util.Random;
 
 import org.junit.Assert;
@@ -32,7 +37,6 @@
  * Unit test TestExponentialRetryPolicy.
  */
 public class TestExponentialRetryPolicy extends AbstractAbfsIntegrationTest {
-
   private final int maxRetryCount = 30;
   private final int noRetryCount = 0;
   private final int retryCount = new Random().nextInt(maxRetryCount);
@@ -57,12 +61,38 @@ public void testDifferentMaxIORetryCount() throws Exception {
   @Test
   public void testDefaultMaxIORetryCount() throws Exception {
     AbfsConfiguration abfsConfig = getAbfsConfig();
-    Assert.assertTrue(
+    Assert.assertEquals(
         String.format("default maxIORetry count is %s.", maxRetryCount),
-        abfsConfig.getMaxIoRetries() == maxRetryCount);
+        maxRetryCount, abfsConfig.getMaxIoRetries());
     testMaxIOConfig(abfsConfig);
   }
 
+  @Test
+  public void testAbfsConfigConstructor() throws Exception {
+    // Ensure we choose expected values that are not defaults
+    ExponentialRetryPolicy template = new ExponentialRetryPolicy(
+        getAbfsConfig().getMaxIoRetries());
+    int testModifier = 1;
+    int expectedMaxRetries = template.getRetryCount() + testModifier;
+    int expectedMinBackoff = template.getMinBackoff() + testModifier;
+    int expectedMaxBackoff = template.getMaxBackoff() + testModifier;
+    int expectedDeltaBackoff = template.getDeltaBackoff() + testModifier;
+
+    Configuration config = new Configuration(this.getRawConfiguration());
+    config.setInt(AZURE_MAX_IO_RETRIES, expectedMaxRetries);
+    config.setInt(AZURE_MIN_BACKOFF_INTERVAL, expectedMinBackoff);
+    config.setInt(AZURE_MAX_BACKOFF_INTERVAL, expectedMaxBackoff);
+    config.setInt(AZURE_BACKOFF_INTERVAL, expectedDeltaBackoff);
+
+    ExponentialRetryPolicy policy = new ExponentialRetryPolicy(
+        new AbfsConfiguration(config, "dummyAccountName"));
+
+    Assert.assertEquals(expectedMaxRetries, policy.getRetryCount());
+    Assert.assertEquals(expectedMinBackoff, policy.getMinBackoff());
+    Assert.assertEquals(expectedMaxBackoff, policy.getMaxBackoff());
+    Assert.assertEquals(expectedDeltaBackoff, policy.getDeltaBackoff());
+  }
+
   private AbfsConfiguration getAbfsConfig() throws Exception {
     Configuration
         config = new Configuration(this.getRawConfiguration());
@@ -81,8 +111,8 @@ private void testMaxIOConfig(AbfsConfiguration abfsConfig) {
       localRetryCount++;
     }
 
-    Assert.assertTrue(
+    Assert.assertEquals(
         "When all retries are exhausted, the retryCount will be same as max configured",
-        localRetryCount == abfsConfig.getMaxIoRetries());
+        abfsConfig.getMaxIoRetries(), localRetryCount);
   }
 }