Use linear back-off, document back-off parameters, add details to timeout exception message

Author: nicktindall

docs/reference/snapshot-restore/repository-s3.asciidoc

@@ -329,6 +329,20 @@ include::repository-shared-settings.asciidoc[]
  `1000` which is the maximum number supported by the https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListMultipartUploads.html[AWS
  ListMultipartUploads API]. If set to `0`, {es} will not attempt to clean up dangling multipart uploads.
 
+`throttled_delete_retry.delay_increment`::
+
+ (integer) This value is used as the delay before the first retry and the amount the delay is incremented by on each subsequent retry. Default is 50ms, minimum is 0ms.
+
+`throttled_delete_retry.maximum_delay`::
+
+ (integer) This is the upper bound on how long the delays between retries will grow to. Default is 500ms, minimum is 0ms.
+
+`throttled_delete_retry.maximum_number_of_retries`::
+
+ (integer) Sets the number times to retry a throttled snapshot deletion. Defaults to `10`, minimum value is `0` which
+ will disable retries altogether. Note that if retries are enabled in the Azure client, each of these retries will
+ comprise of that many attempts.
+
 NOTE: The option of defining client settings in the repository settings as
 documented below is considered deprecated, and will be removed in a future
 version.

modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryMetricsTests.java

@@ -267,7 +267,7 @@ public void testRetrySnapshotDeleteMetricsOnEventualSuccess() throws IOException
  Settings.builder()
  .put(repositorySettings(repositoryName))
  .put(S3ClientSettings.MAX_RETRIES_SETTING.getConcreteSettingForNamespace("placeholder").getKey(), 0)
- .put(S3Repository.RETRY_THROTTLED_DELETE_INITIAL_DELAY.getKey(), TimeValue.timeValueMillis(10))
+ .put(S3Repository.RETRY_THROTTLED_DELETE_DELAY_INCREMENT.getKey(), TimeValue.timeValueMillis(10))
  .put(S3Repository.RETRY_THROTTLED_DELETE_MAX_NUMBER_OF_RETRIES.getKey(), maxRetries)
  .build(),
  false
@@ -299,7 +299,7 @@ public void testRetrySnapshotDeleteMetricsWhenRetriesExhausted() {
  Settings.builder()
  .put(repositorySettings(repositoryName))
  .put(S3ClientSettings.MAX_RETRIES_SETTING.getConcreteSettingForNamespace("placeholder").getKey(), 0)
- .put(S3Repository.RETRY_THROTTLED_DELETE_INITIAL_DELAY.getKey(), TimeValue.timeValueMillis(10))
+ .put(S3Repository.RETRY_THROTTLED_DELETE_DELAY_INCREMENT.getKey(), TimeValue.timeValueMillis(10))
  .put(S3Repository.RETRY_THROTTLED_DELETE_MAX_NUMBER_OF_RETRIES.getKey(), maxRetries)
  .build(),
  false

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java

@@ -424,7 +424,11 @@ private boolean maybeDelayAndRetryDelete(Iterator<TimeValue> retries) {
  logger.warn("Aborting tenacious snapshot delete retries due to interrupt");
  }
  } else {
- logger.warn("Exceeded maximum tenacious snapshot delete retries, aborting");
+ logger.warn(
+ "Exceeded maximum tenacious snapshot delete retries, aborting. Using back-off policy "
+ + retryThrottledDeleteBackoffPolicy
+ + ", see the throttled_delete_retry.* S3 repository properties to configure the back-off parameters"
+ );
  }
  return false;
  }

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java

@@ -204,17 +204,22 @@ class S3Repository extends MeteredBlobStoreRepository {
  );
 
  /**
- * We will retry deletes that fail due to throttling. We use an {@link BackoffPolicy#exponentialBackoff(TimeValue, int)}
- * with the following <code>initialDelay</code> and <code>maxNumberOfRetries</code>
+ * We will retry deletes that fail due to throttling. We use an {@link BackoffPolicy#linearBackoff(TimeValue, int, TimeValue)}
+ * with the following parameters
  */
- static final Setting<TimeValue> RETRY_THROTTLED_DELETE_INITIAL_DELAY = Setting.timeSetting(
- "throttled_delete_retry.initial_delay",
+ static final Setting<TimeValue> RETRY_THROTTLED_DELETE_DELAY_INCREMENT = Setting.timeSetting(
+ "throttled_delete_retry.delay_increment",
  new TimeValue(50, TimeUnit.MILLISECONDS),
- new TimeValue(10, TimeUnit.MILLISECONDS)
+ new TimeValue(0, TimeUnit.MILLISECONDS)
+ );
+ static final Setting<TimeValue> RETRY_THROTTLED_DELETE_MAXIMUM_DELAY = Setting.timeSetting(
+ "throttled_delete_retry.maximum_delay",
+ new TimeValue(500, TimeUnit.MILLISECONDS),
+ new TimeValue(0, TimeUnit.MILLISECONDS)
  );
  static final Setting<Integer> RETRY_THROTTLED_DELETE_MAX_NUMBER_OF_RETRIES = Setting.intSetting(
- "throttled_delete_retry.max_number_of_retries",
- 8,
+ "throttled_delete_retry.maximum_number_of_retries",
+ 10,
  0
  );
 
@@ -441,9 +446,10 @@ protected S3BlobStore createBlobStore() {
  bigArrays,
  threadPool,
  s3RepositoriesMetrics,
- BackoffPolicy.exponentialBackoff(
- RETRY_THROTTLED_DELETE_INITIAL_DELAY.get(metadata.settings()),
- RETRY_THROTTLED_DELETE_MAX_NUMBER_OF_RETRIES.get(metadata.settings())
+ BackoffPolicy.linearBackoff(
+ RETRY_THROTTLED_DELETE_DELAY_INCREMENT.get(metadata.settings()),
+ RETRY_THROTTLED_DELETE_MAX_NUMBER_OF_RETRIES.get(metadata.settings()),
+ RETRY_THROTTLED_DELETE_MAXIMUM_DELAY.get(metadata.settings())
  )
  );
  }

server/src/main/java/org/elasticsearch/common/BackoffPolicy.java

@@ -8,6 +8,7 @@
  */
 package org.elasticsearch.common;
 
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.TimeValue;
 
 import java.util.Collections;
@@ -81,6 +82,18 @@ public static BackoffPolicy exponentialBackoff(TimeValue initialDelay, int maxNu
  return new ExponentialBackoff((int) checkDelay(initialDelay).millis(), maxNumberOfRetries);
  }
 
+ /**
+ * Creates a new linear backoff policy with the provided configuration
+ *
+ * @param delayIncrement The amount by which to increment the delay on each retry
+ * @param maxNumberOfRetries The maximum number of retries
+ * @param maximumDelay The maximum delay
+ * @return A backoff policy with linear increase in wait time for retries.
+ */
+ public static BackoffPolicy linearBackoff(TimeValue delayIncrement, int maxNumberOfRetries, TimeValue maximumDelay) {
+ return new LinearBackoff(delayIncrement, maxNumberOfRetries, maximumDelay);
+ }
+
  /**
  * Wraps the backoff policy in one that calls a method every time a new backoff is taken from the policy.
  */
@@ -100,6 +113,11 @@ private static class NoBackoff extends BackoffPolicy {
  public Iterator<TimeValue> iterator() {
  return Collections.emptyIterator();
  }
+
+ @Override
+ public String toString() {
+ return "NoBackoff";
+ }
  }
 
  private static class ExponentialBackoff extends BackoffPolicy {
@@ -118,6 +136,11 @@ private ExponentialBackoff(int start, int numberOfElements) {
  public Iterator<TimeValue> iterator() {
  return new ExponentialBackoffIterator(start, numberOfElements);
  }
+
+ @Override
+ public String toString() {
+ return "ExponentialBackoff{start=" + start + ", numberOfElements=" + numberOfElements + '}';
+ }
  }
 
  private static class ExponentialBackoffIterator implements Iterator<TimeValue> {
@@ -163,6 +186,11 @@ private static final class ConstantBackoff extends BackoffPolicy {
  public Iterator<TimeValue> iterator() {
  return new ConstantBackoffIterator(delay, numberOfElements);
  }
+
+ @Override
+ public String toString() {
+ return "ConstantBackoff{delay=" + delay + ", numberOfElements=" + numberOfElements + '}';
+ }
  }
 
  private static final class ConstantBackoffIterator implements Iterator<TimeValue> {
@@ -203,6 +231,11 @@ private static final class WrappedBackoffPolicy extends BackoffPolicy {
  public Iterator<TimeValue> iterator() {
  return new WrappedBackoffIterator(delegate.iterator(), onBackoff);
  }
+
+ @Override
+ public String toString() {
+ return "WrappedBackoffPolicy{delegate=" + delegate + ", onBackoff=" + onBackoff + '}';
+ }
  }
 
  private static final class WrappedBackoffIterator implements Iterator<TimeValue> {
@@ -228,4 +261,60 @@ public TimeValue next() {
  return delegate.next();
  }
  }
+
+ private static final class LinearBackoff extends BackoffPolicy {
+
+ private final TimeValue delayIncrement;
+ private final int maxNumberOfRetries;
+ private final TimeValue maximumDelay;
+
+ private LinearBackoff(TimeValue delayIncrement, int maxNumberOfRetries, @Nullable TimeValue maximumDelay) {
+ this.delayIncrement = delayIncrement;
+ this.maxNumberOfRetries = maxNumberOfRetries;
+ this.maximumDelay = maximumDelay;
+ }
+
+ @Override
+ public Iterator<TimeValue> iterator() {
+ return new LinearBackoffIterator(delayIncrement, maxNumberOfRetries, maximumDelay);
+ }
+
+ @Override
+ public String toString() {
+ return "LinearBackoff{"
+ + "delayIncrement="
+ + delayIncrement
+ + ", maxNumberOfRetries="
+ + maxNumberOfRetries
+ + ", maximumDelay="
+ + maximumDelay
+ + '}';
+ }
+ }
+
+ private static final class LinearBackoffIterator implements Iterator<TimeValue> {
+
+ private final TimeValue delayIncrement;
+ private final int maxNumberOfRetries;
+ private final TimeValue maximumDelay;
+ private int curr;
+
+ private LinearBackoffIterator(TimeValue delayIncrement, int maxNumberOfRetries, @Nullable TimeValue maximumDelay) {
+ this.delayIncrement = delayIncrement;
+ this.maxNumberOfRetries = maxNumberOfRetries;
+ this.maximumDelay = maximumDelay;
+ }
+
+ @Override
+ public boolean hasNext() {
+ return curr < maxNumberOfRetries;
+ }
+
+ @Override
+ public TimeValue next() {
+ curr++;
+ TimeValue timeValue = TimeValue.timeValueMillis(curr * delayIncrement.millis());
+ return maximumDelay == null ? timeValue : timeValue.compareTo(maximumDelay) < 0 ? timeValue : maximumDelay;
+ }
+ }
 }

server/src/test/java/org/elasticsearch/common/BackoffPolicyTests.java

@@ -77,6 +77,25 @@ public void testExponentialBackOff() {
  }
  }
 
+ public void testLinearBackoffWithLimit() {
+ int incrementMillis = randomIntBetween(10, 500);
+ int limitMillis = randomIntBetween(1000, 5000);
+ int maxNumberOfRetries = randomIntBetween(0, 30);
+ BackoffPolicy timeValues = BackoffPolicy.linearBackoff(
+ timeValueMillis(incrementMillis),
+ maxNumberOfRetries,
+ timeValueMillis(limitMillis)
+ );
+ int counter = 0;
+ for (TimeValue timeValue : timeValues) {
+ counter++;
+ int unlimitedValue = counter * incrementMillis;
+ int expectedValue = Math.min(unlimitedValue, limitMillis);
+ assertEquals(timeValueMillis(expectedValue), timeValue);
+ }
+ assertEquals(counter, maxNumberOfRetries);
+ }
+
  public void testNoBackoff() {
  BackoffPolicy noBackoff = BackoffPolicy.noBackoff();
  int numberOfBackoffsToPerform = randomIntBetween(1, 3);