Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Introduce max delay duration limit in retry policy #43882

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mssfang
Copy link
Member

@mssfang mssfang commented Jan 22, 2025

The retry-after could have a delay duration of 24hrs.
With introducing the max delay duration check before retry the request will solve the hanging there "forever" issue.

@vcolin7
Copy link
Member

vcolin7 commented Jan 22, 2025

What's the reasoning behind setting an arbitrary limit on the Retry-After delay? What's the largest delay duration we realistically get in said header?

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@@ -313,6 +328,14 @@ private static void logRetryWithError(LoggingEventBuilder loggingEventBuilder, i
loggingEventBuilder.addKeyValue(LoggingKeys.TRY_COUNT_KEY, tryCount).log(format, throwable);
}

private static void throwIfDelayDurationExceedLimit(Duration delayDuration) {
Copy link
Member

Choose a reason for hiding this comment

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

If we do begin allowing this concept we should let this be configurable. But in the meanwhile, this can be worked around by configuring .setShouldRetryCondition when configuring the RetryOptions in a client. An example of this could be:

RetryOptions retryOptions = new RetryOptions(new ExponentialBackoffOptions()
    .setShouldRetryCondition( retryInfo -> {
        HttpResponse response = retryInfo.getResponse();
        if (response == null) {
            // return status based on the exception seen.
        }
        
        if (response.getStatusCode() != 429 && response.getStatusCode() != 503) {
            return false; // not a backoff scenarion, true error
        }

        Duration retryAfter = getRetryAfterDuration(response);
        return retryAfterBackoffIsAllowable(retryAfter); // logic to determine whether the backoff is too long.
    });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core azure-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants