Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@ private <I extends SerializableStruct, O extends SerializableStruct> O deseriali
return retry(call, request, acquireResult.token(), acquireResult.delay());
} catch (TokenAcquisitionFailedException tafe) {
// 9.b If InterceptorContext.response() is an unretryable failure, continue to step 10.
// For long-polling operations, backoff before returning.
if (tafe.delay() != null && !tafe.delay().isZero()) {
sleep(tafe.delay());
}
LOGGER.debug("Cannot acquire a retry token: {}", tafe);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,11 @@
package software.amazon.smithy.java.client.http.plugins;

import java.time.Duration;
import java.time.Instant;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;
import software.amazon.smithy.java.client.core.AutoClientPlugin;
import software.amazon.smithy.java.client.core.CallContext;
import software.amazon.smithy.java.client.core.ClientConfig;
import software.amazon.smithy.java.client.core.interceptors.ClientInterceptor;
import software.amazon.smithy.java.client.core.interceptors.OutputHook;
import software.amazon.smithy.java.client.core.settings.ClockSetting;
import software.amazon.smithy.java.client.http.HttpMessageExchange;
import software.amazon.smithy.java.context.Context;
import software.amazon.smithy.java.core.error.CallException;
Expand All @@ -33,6 +28,8 @@
*/
@SmithyInternalApi
public final class ApplyHttpRetryInfoPlugin implements AutoClientPlugin {
private static final HeaderName X_AMZ_RETRY_AFTER = HeaderName.of("x-amz-retry-after");

@Override
public void configureClient(ClientConfig.Builder config) {
// We can conditionally add the interceptor here because client transport can't change after construction.
Expand Down Expand Up @@ -60,7 +57,7 @@ public <O extends SerializableStruct> O modifyBeforeAttemptCompletion(

static void applyRetryInfo(HttpResponse response, CallException exception, Context context) {
// (1) Check with the protocol if the server explicitly wants a retry.
if (!applyRetryAfterHeader(response, exception, context)) {
if (!applyRetryAfterHeader(response, exception)) {
if (!applyThrottlingStatusCodes(response, exception)) {
// (2) If no retry was detected so far, is it safe to retry because of a 5XX error + idempotency token?
if (exception.isRetrySafe() == RetrySafety.MAYBE) {
Expand All @@ -85,29 +82,19 @@ private static boolean applyThrottlingStatusCodes(HttpResponse response, CallExc
return false;
}

// If there's a retry-after header, then the server is telling us it's retryable.
private static boolean applyRetryAfterHeader(HttpResponse response, CallException exception, Context context) {
var retryAfter = response.headers().firstValue(HeaderName.RETRY_AFTER);
if (retryAfter != null) {
exception.isThrottle(true);
exception.isRetrySafe(RetrySafety.YES);
exception.retryAfter(parseRetryAfter(retryAfter, context));
return true;
// Per SEP: use x-amz-retry-after (integer milliseconds). Ignore standard Retry-After header.
private static boolean applyRetryAfterHeader(HttpResponse response, CallException exception) {
var xAmzRetryAfter = response.headers().firstValue(X_AMZ_RETRY_AFTER);
if (xAmzRetryAfter != null) {
try {
var millis = Long.parseLong(xAmzRetryAfter);
exception.isRetrySafe(RetrySafety.YES);
exception.retryAfter(Duration.ofMillis(millis));
return true;
} catch (NumberFormatException e) {
// Invalid value — ignore, fall back to exponential backoff
}
}

return false;
}

private static Duration parseRetryAfter(String retryAfter, Context context) {
try {
return Duration.of(Integer.parseInt(retryAfter), ChronoUnit.SECONDS);
} catch (NumberFormatException e) {
// It's not a number, so it must be a http-date like "Wed, 21 Oct 2015 07:28:00 GMT".
var date = ZonedDateTime.parse(retryAfter, DateTimeFormatter.RFC_1123_DATE_TIME);
// Use the Clock associated with the context, if any, to account for things like clock skew.
var clock = context.get(ClockSetting.CLOCK);
var now = clock == null ? Instant.now() : clock.instant();
return Duration.between(now, date);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,53 +10,66 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.time.ZoneId;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Test;
import software.amazon.smithy.java.client.core.CallContext;
import software.amazon.smithy.java.client.core.settings.ClockSetting;
import software.amazon.smithy.java.context.Context;
import software.amazon.smithy.java.core.error.CallException;
import software.amazon.smithy.java.http.api.HttpHeaders;
import software.amazon.smithy.java.http.api.HttpResponse;
import software.amazon.smithy.java.retries.api.RetrySafety;

public class ApplyHttpRetryInfoPluginTest {

@Test
public void appliesRetryAfterHeader() {
public void appliesXAmzRetryAfterHeader() {
var response = HttpResponse.create()
.setStatusCode(500)
.setHeaders(HttpHeaders.of(Map.of("retry-after", List.of("10"))))
.setHeaders(HttpHeaders.of(Map.of("x-amz-retry-after", List.of("1500"))))
.toUnmodifiable();
var e = new CallException("err");
var context = Context.create();

ApplyHttpRetryInfoPlugin.applyRetryInfo(response, e, context);

assertThat(e.isRetrySafe(), is(RetrySafety.YES));
assertThat(e.isThrottle(), is(true));
assertThat(e.retryAfter(), equalTo(Duration.ofSeconds(10)));
assertThat(e.retryAfter(), equalTo(Duration.ofMillis(1500)));
}

@Test
public void appliesRetryAfterHeaderDate() {
public void ignoresInvalidXAmzRetryAfterHeader() {
var response = HttpResponse.create()
.setStatusCode(500)
.setHeaders(HttpHeaders.of(Map.of("retry-after", List.of("Wed, 21 Oct 2015 07:28:00 GMT"))))
.setHeaders(HttpHeaders.of(Map.of("x-amz-retry-after", List.of("invalid"))))
.toUnmodifiable();
var e = new CallException("err");
var context = Context.create();
context.put(ClockSetting.CLOCK, Clock.fixed(Instant.parse("2015-10-21T05:28:00Z"), ZoneId.of("UTC")));
context.put(CallContext.IDEMPOTENCY_TOKEN, "foo");

ApplyHttpRetryInfoPlugin.applyRetryInfo(response, e, context);

// Falls through to normal 5xx + idempotency handling
assertThat(e.isRetrySafe(), is(RetrySafety.YES));
assertThat(e.isThrottle(), is(true));
assertThat(e.retryAfter(), equalTo(Duration.ofHours(2)));
assertThat(e.retryAfter(), nullValue());
}

@Test
public void ignoresStandardRetryAfterHeader() {
// SEP: SDKs MUST ignore the standard HTTP Retry-After header
var response = HttpResponse.create()
.setStatusCode(500)
.setHeaders(HttpHeaders.of(Map.of("retry-after", List.of("10"))))
.toUnmodifiable();
var e = new CallException("err");
var context = Context.create();

ApplyHttpRetryInfoPlugin.applyRetryInfo(response, e, context);

// Standard Retry-After is ignored, falls through to non-retryable (no idempotency token)
assertThat(e.isRetrySafe(), is(RetrySafety.NO));
assertThat(e.retryAfter(), nullValue());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,37 @@

package software.amazon.smithy.java.retries.api;

import java.time.Duration;

/**
* Exception thrown by {@link RetryStrategy} when a new token cannot be acquired.
*/
public final class TokenAcquisitionFailedException extends RuntimeException {
private final transient RetryToken token;
private final transient Duration delay;

public TokenAcquisitionFailedException(String msg) {
super(msg);
token = null;
delay = Duration.ZERO;
}

public TokenAcquisitionFailedException(String msg, Throwable cause) {
super(msg, cause);
token = null;
delay = Duration.ZERO;
}

public TokenAcquisitionFailedException(String msg, RetryToken token, Throwable cause) {
super(msg, cause);
this.token = token;
this.delay = Duration.ZERO;
}

public TokenAcquisitionFailedException(String msg, RetryToken token, Throwable cause, Duration delay) {
super(msg, cause);
this.token = token;
this.delay = delay != null ? delay : Duration.ZERO;
}

/**
Expand All @@ -33,4 +45,13 @@ public TokenAcquisitionFailedException(String msg, RetryToken token, Throwable c
public RetryToken token() {
return token;
}

/**
* Returns the delay to wait before returning the error to the caller.
* This is non-zero for long-polling operations when retry quota is exhausted.
* @return the delay.
*/
public Duration delay() {
return delay;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public static Builder builder() {
return new Builder()
.maxAttempts(Constants.Adaptive.MAX_ATTEMPTS)
.backoffBaseDelay(Constants.Adaptive.BASE_DELAY)
.throttlingBackoffBaseDelay(Constants.Standard.THROTTLING_BASE_DELAY)
.backoffMaxBackoff(Constants.Adaptive.MAX_BACKOFF)
.retryCost(Constants.Adaptive.RETRY_COST)
.throttlingRetryCost(Constants.Adaptive.THROTTLING_RETRY_COST)
Expand Down Expand Up @@ -118,6 +119,17 @@ public Builder backoffBaseDelay(Duration baseDelay) {
return this;
}

/**
* Set the base delay for exponential backoff on throttling errors.
*
* @param baseDelay the throttling base delay.
* @return the builder.
*/
public Builder throttlingBackoffBaseDelay(Duration baseDelay) {
setThrottlingBackoffBaseDelay(baseDelay);
return this;
}

/**
* Set the maximum backoff delay.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@

import java.time.Duration;
import java.util.Objects;
import java.util.Random;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;
import software.amazon.smithy.java.logging.InternalLogger;
import software.amazon.smithy.java.retries.api.AcquireInitialTokenRequest;
import software.amazon.smithy.java.retries.api.AcquireInitialTokenResponse;
Expand All @@ -29,6 +31,7 @@ abstract class BaseRetryStrategy implements RetryStrategy, Claimable {
protected final InternalLogger log;
protected final int maxAttempts;
protected final ExponentialDelayWithJitter backoffStrategy;
protected final ExponentialDelayWithJitter throttlingBackoffStrategy;
protected final int retryCost;
protected final int throttlingRetryCost;
protected final int initialRetryTokens;
Expand All @@ -38,7 +41,21 @@ abstract class BaseRetryStrategy implements RetryStrategy, Claimable {
BaseRetryStrategy(InternalLogger log, Builder builder) {
this.log = log;
this.maxAttempts = validateIsPositive(builder.maxAttempts, "maxAttempts");
this.backoffStrategy = new ExponentialDelayWithJitter(builder.backoffBaseDelay, builder.backoffMaxDelay);
if (builder.randomSupplier != null) {
this.backoffStrategy = new ExponentialDelayWithJitter(
builder.randomSupplier,
builder.backoffBaseDelay,
builder.backoffMaxDelay);
this.throttlingBackoffStrategy = new ExponentialDelayWithJitter(
builder.randomSupplier,
builder.throttlingBackoffBaseDelay,
builder.backoffMaxDelay);
} else {
this.backoffStrategy = new ExponentialDelayWithJitter(builder.backoffBaseDelay, builder.backoffMaxDelay);
this.throttlingBackoffStrategy = new ExponentialDelayWithJitter(
builder.throttlingBackoffBaseDelay,
builder.backoffMaxDelay);
}
this.retryCost = Objects.requireNonNull(builder.retryCost, "retryCost");
this.throttlingRetryCost = Objects.requireNonNull(builder.throttlingRetryCost, "throttlingRetryCost");
this.initialRetryTokens = builder.initialRetryTokens;
Expand Down Expand Up @@ -140,12 +157,16 @@ protected Duration computeInitialBackoff(AcquireInitialTokenRequest request) {
* method to compute a different backoff depending on their logic.
*/
protected Duration computeBackoff(RefreshRetryTokenRequest request, DefaultRetryToken token) {
var backoff = backoffStrategy.computeDelay(token.attempt());
var isThrottle = treatAsThrottling(request.failure());
var strategy = isThrottle ? throttlingBackoffStrategy : backoffStrategy;
var backoff = strategy.computeDelay(token.attempt());
var suggested = request.suggestedDelay();
if (suggested == null) {
return backoff;
}
return maxOf(suggested, backoff);
// Clamp suggested delay: min bound is t_i, max bound is 5s + t_i
var maxBound = backoff.plus(Duration.ofSeconds(5));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - might be nice to make 5 a constant so it's easier to know about in the future

return minOf(maxOf(suggested, backoff), maxBound);
}

/**
Expand Down Expand Up @@ -183,7 +204,7 @@ private DefaultRetryToken refreshToken(RefreshRetryTokenRequest request, Acquire

private AcquireResponse requestAcquireCapacity(RefreshRetryTokenRequest request, DefaultRetryToken token) {
var tokenBucket = tokenBucketStore.tokenBucketForScope(token.scope());
return tokenBucket.tryAcquire(retryCost(request), token);
return tokenBucket.tryAcquire(retryCost(request));
}

private ReleaseResponse releaseTokenBucketCapacity(DefaultRetryToken token) {
Expand Down Expand Up @@ -238,6 +259,19 @@ private void throwOnAcquisitionFailure(RefreshRetryTokenRequest request, Acquire
var token = asDefaultRetryToken(request.token());
if (acquireResponse.acquisitionFailed()) {
var failure = request.failure();
// For long-polling operations, compute backoff delay before returning
if (token.isLongPolling()) {
var refreshedToken = token.toBuilder()
.increaseAttempt()
.capacityRemaining(acquireResponse.capacityRemaining())
.capacityAcquired(acquireResponse.capacityAcquired())
.addFailure(failure)
.build();
var backoff = computeBackoff(request, refreshedToken);
var message = acquisitionFailedForLongPollOperationMessage(acquireResponse);
log.debug(message, failure);
throw new TokenAcquisitionFailedException(message, refreshedToken, failure, backoff);
}
var refreshedToken = token.toBuilder()
.capacityRemaining(acquireResponse.capacityRemaining())
.capacityAcquired(acquireResponse.capacityAcquired())
Expand Down Expand Up @@ -336,6 +370,13 @@ static Duration maxOf(Duration left, Duration right) {
return right;
}

static Duration minOf(Duration left, Duration right) {
if (left.compareTo(right) <= 0) {
return left;
}
return right;
}

static DefaultRetryToken asDefaultRetryToken(RetryToken token) {
if (token instanceof DefaultRetryToken t) {
return t;
Expand Down Expand Up @@ -363,7 +404,9 @@ public abstract static class Builder implements RetryStrategy.Builder {
protected int initialRetryTokens;
protected int maxScopes;
protected Duration backoffBaseDelay;
protected Duration throttlingBackoffBaseDelay;
protected Duration backoffMaxDelay;
private Supplier<Random> randomSupplier;

Builder() {
this.initialRetryTokens = Constants.Standard.INITIAL_RETRY_TOKENS;
Expand All @@ -377,6 +420,7 @@ public abstract static class Builder implements RetryStrategy.Builder {
this.initialRetryTokens = strategy.initialRetryTokens;
this.maxScopes = strategy.maxScopes;
this.backoffBaseDelay = strategy.backoffStrategy.baseDelay();
this.throttlingBackoffBaseDelay = strategy.throttlingBackoffStrategy.baseDelay();
this.backoffMaxDelay = strategy.backoffStrategy.maxDelay();
}

Expand Down Expand Up @@ -404,8 +448,16 @@ void setBackoffBaseDelay(Duration backoffBaseDelay) {
this.backoffBaseDelay = backoffBaseDelay;
}

void setThrottlingBackoffBaseDelay(Duration throttlingBackoffBaseDelay) {
this.throttlingBackoffBaseDelay = throttlingBackoffBaseDelay;
}

void setBackoffMaxDelay(Duration backoffMaxDelay) {
this.backoffMaxDelay = backoffMaxDelay;
}

void setRandomSupplier(Supplier<Random> randomSupplier) {
this.randomSupplier = randomSupplier;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ final class Constants {
*/
static final class Standard {
static final Duration BASE_DELAY = Duration.ofMillis(50);
static final Duration THROTTLING_BASE_DELAY = Duration.ofMillis(1000);
static final Duration MAX_BACKOFF = Duration.ofSeconds(20);
static final int MAX_ATTEMPTS = 3;
static final int RETRY_COST = 14;
Expand Down
Loading