Missing Thread#sleep call in the ExponentialBackoffRetryStrategy implementation?

[cbismuth]

Hi,

I may have missed something, but I can't see any Thread#sleep call in these implementations below.

And our production systems went down over the past week-end due to more than 22 000 requests with 429 HTTP status code.

autorest-clientruntime-for-java/client-runtime/src/main/java/com/microsoft/rest/retry/RetryHandler.java

Lines 78 to 85 in a55f87f

	while (retryStrategy.shouldRetry(tryCount, response)) {
	tryCount++;
	if (response.body() != null) {
	response.body().close();
	}
	// retry the request
	response = chain.proceed(request);
	}

autorest-clientruntime-for-java/client-runtime/src/main/java/com/microsoft/rest/retry/ExponentialBackoffRetryStrategy.java

Lines 99 to 104 in a55f87f

	public boolean shouldRetry(int retryCount, Response response) {
	int code = response.code();
	//CHECKSTYLE IGNORE MagicNumber FOR NEXT 2 LINES
	return retryCount < this.retryCount
	&& (code == 408 || (code >= 500 && code != 501 && code != 505));
	}

Thanks,
Christophe

[cbismuth]

Not sure it would be the best option, but here is what I would expect as an implementation.

final class ExponentialBackoffRetryStrategy extends RetryStrategy {

    private static final Logger LOGGER = LoggerFactory.getLogger(ExponentialBackoffRetryStrategy.class);

    private final int maxRetryCount;
    private final long initialIntervalInMillis;
    private final long maxIntervalInMillis;
    private final double multiplier;

    ExponentialBackoffRetryStrategy(final int maxRetryCount,
                                    final int initialIntervalDuration,
                                    final TimeUnit initialIntervalTimeUnit,
                                    final int maxIntervalDuration,
                                    final TimeUnit maxIntervalTimeUnit,
                                    final double multiplier) {
        super(ExponentialBackoffRetryStrategy.class.getName(), false);

        this.maxRetryCount = maxRetryCount;
        this.multiplier = multiplier;

        initialIntervalInMillis = initialIntervalTimeUnit.toMillis(initialIntervalDuration);
        maxIntervalInMillis = maxIntervalTimeUnit.toMillis(maxIntervalDuration);
    }

    @Override
    public boolean shouldRetry(final int retryCount, final Response response) {
        final int code = response.code();

        final boolean isRetryable = code == HttpStatus.REQUEST_TIMEOUT.value() || code >= HttpStatus.INTERNAL_SERVER_ERROR.value()
                                                                                  && code != HttpStatus.NOT_IMPLEMENTED.value()
                                                                                  && code != HttpStatus.HTTP_VERSION_NOT_SUPPORTED.value();

        if (isRetryable && retryCount < maxRetryCount) {
            try {
                final long expected = (long) Math.floor((double) retryCount * (double) initialIntervalInMillis * multiplier);
                final long actual = Math.min(expected, maxIntervalInMillis);

                LOGGER.info("Status code [{}] received, sleeping [{}] ms before retrying ...", code, actual);

                Thread.sleep(actual);

                return true;
            } catch (final InterruptedException e) {
                LOGGER.warn("Can't cause the currently executing thread to sleep, retry attempt canceled");
            }
        }

        return false;
    }
}