Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.25k stars 4.59k forks source link

[QUERY][DNS]Retry policy for GetAllRecordDataAyns() #45524

Closed yunheMsft closed 3 weeks ago

yunheMsft commented 1 month ago

Library name and version

Azure.ResourceManager.Dns 1.1.1

Query/Question

I have a process to fetch all Dns records from a zone using GetAllRecordDataAsync().

public async IAsyncEnumerable<DnsRecordData> GetAllDnsRecordDataAsync(int limit = DnsConfigurationFactory.DefaultGetAllDnsRecordsPerPageLimit)
    {
        AsyncPageable<DnsRecordData> asyncRecordSetsPages = _dnsZoneResource.GetAllRecordDataAsync(top: limit);

        await foreach(DnsRecordData record in asyncRecordSetsPages)
        {
            yield return record;
        }
    }

When iterating through the result AsyncPageable, I see 429 error returned by DNS server side. Based on the logs from ARM, the SDK client initiated 3-4 retries for the call to fetch a particular page. However, the delay between each retry was only 10 seconds. This aligns with the "Retry-After: 10" header returned by DNS service. My questions:

  1. Why is the default retry policy not using exponential backoff for throttling error?
  2. Is it possible to attach a custom retry policy for the fetch page call?

Environment

No response

github-actions[bot] commented 1 month ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

HarveyLink commented 3 weeks ago

Hi @yunheMsft , For the questions you asked.

  1. Default retry policy will prioritize Error response's RetryAfter header. So if the returned error contains service defined retry, SDK will follow it. Check https://github.com/Azure/azure-sdk-for-net/blob/150082bd7ac135540c5e3ec145ca596cc7e20716/sdk/core/Azure.Core/src/Pipeline/RetryPolicy.cs#L122
                if (shouldRetry)
                {
                    var retryAfter = message.HasResponse ? message.Response.Headers.RetryAfter : default;
                    TimeSpan delay = async ? await GetNextDelayAsync(message, retryAfter).ConfigureAwait(false) : GetNextDelay(message, retryAfter);
                    if (delay > TimeSpan.Zero)
                    {
                        if (async)
                        {
                            await WaitAsync(delay, message.CancellationToken).ConfigureAwait(false);
                        }
                        else
                        {
                            Wait(delay, message.CancellationToken);
                        }
                    }
  2. Yes, SDK support customized RetryPolicy, you could add it like
    var option = new ArmClientOptions()
    {
    RetryPolicy = new YourPolicy()
    };
    var client = new ArmClient(new DefaultAzureCredential(), subscription, option);

    Your customized policy should inherit from HttpPipelinePolicy just like RetryPolicy

github-actions[bot] commented 3 weeks ago

Hi @yunheMsft. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

yunheMsft commented 3 weeks ago

Thanks @HarveyLink for the explanation.