Skip to content

HttpTimeoutPolicy Improvements Phase 1: Fixes QueryPlan requests retry gaps #5476

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.

Sign up for GitHub

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

Merged
microsoft-github-policy-service merged 9 commits into from
Nov 8, 2025
Merged

HttpTimeoutPolicy Improvements Phase 1: Fixes QueryPlan requests retry gaps #5476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ad6cf0f
Update safe retry detection using document service request resource t…
Praveen-Msft Nov 6, 2025
77da20a
Merge branch 'master' into fix-retry-for-http-timeout-policies
Praveen-Msft Nov 6, 2025
c92aa43
Use DocumentServiceRequest's IsReadOnlyRequest to determine - part1
Praveen-Msft Nov 6, 2025
b871967
Remove sln changes.
Praveen-Msft Nov 6, 2025
fe72a70
Update the safe retry to use address resource type as well.
Praveen-Msft Nov 6, 2025
bf9ace4
Merge branch 'master' into fix-retry-for-http-timeout-policies
kundadebdatta Nov 7, 2025
c1cd5cc
when DRS is null, allow retry, use convention for static methods.
Praveen-Msft Nov 7, 2025
d29594d
Merge branch 'fix-retry-for-http-timeout-policies' of https://github.…
Praveen-Msft Nov 7, 2025
07178e2
Add decision in the comments for IsSafeToRetry mechanism
Praveen-Msft Nov 7, 2025
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
25 changes: 18 additions & 7 deletions Microsoft.Azure.Cosmos/src/HttpClient/CosmosHttpClientCore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ private async Task<HttpResponseMessage> SendHttpHelperAsync(
return responseMessage;
}

bool isOutOfRetries = CosmosHttpClientCore.IsOutOfRetries(timeoutPolicy, startDateTimeUtc, timeoutEnumerator);
bool isOutOfRetries = CosmosHttpClientCore.IsOutOfRetries(timeoutEnumerator);
if (isOutOfRetries)
{
return responseMessage;
Expand All @@ -402,7 +402,7 @@ private async Task<HttpResponseMessage> SendHttpHelperAsync(
datum.RecordHttpException(requestMessage, e, resourceType, requestStartTime);
trace = datum.Trace;
}
bool isOutOfRetries = CosmosHttpClientCore.IsOutOfRetries(timeoutPolicy, startDateTimeUtc, timeoutEnumerator);
bool isOutOfRetries = CosmosHttpClientCore.IsOutOfRetries(timeoutEnumerator);

switch (e)
{
Expand All @@ -415,7 +415,7 @@ private async Task<HttpResponseMessage> SendHttpHelperAsync(

// Convert OperationCanceledException to 408 when the HTTP client throws it. This makes it clear that the
// the request timed out and was not user canceled operation.
if (isOutOfRetries || !timeoutPolicy.IsSafeToRetry(requestMessage.Method))
if (isOutOfRetries || !CosmosHttpClientCore.IsSafeToRetry(documentServiceRequest))
{
// throw current exception (caught in transport handler)
string message =
Expand All @@ -440,14 +440,14 @@ private async Task<HttpResponseMessage> SendHttpHelperAsync(

break;
case WebException webException:
if (isOutOfRetries || (!timeoutPolicy.IsSafeToRetry(requestMessage.Method) && !WebExceptionUtility.IsWebExceptionRetriable(webException)))
if (isOutOfRetries || (!CosmosHttpClientCore.IsSafeToRetry(documentServiceRequest) && !WebExceptionUtility.IsWebExceptionRetriable(webException)))
{
throw;
}

break;
case HttpRequestException httpRequestException:
if (isOutOfRetries || !timeoutPolicy.IsSafeToRetry(requestMessage.Method))
if (isOutOfRetries || !CosmosHttpClientCore.IsSafeToRetry(documentServiceRequest))
{
throw;
}
Expand Down Expand Up @@ -493,13 +493,24 @@ private async Task<HttpResponseMessage> SendHttpHelperAsync(
}

private static bool IsOutOfRetries(
HttpTimeoutPolicy timeoutPolicy,
DateTime startDateTimeUtc,
IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> timeoutEnumerator)
{
return !timeoutEnumerator.MoveNext(); // No more retries are configured
}

private static bool IsSafeToRetry(DocumentServiceRequest documentServiceRequest)
{
// Three scenarios are safely retriable:
// 1) If request is null since they are originated from GetAsync calls
// 2) If request is read-only
// 3) If request is an address request.
if (documentServiceRequest == null)
{
return true;
}
return documentServiceRequest.IsReadOnlyRequest || documentServiceRequest.ResourceType == ResourceType.Address;
}

private async Task<HttpResponseMessage> ExecuteHttpHelperAsync(
HttpRequestMessage requestMessage,
ResourceType resourceType,
Expand Down
1 change: 0 additions & 1 deletion Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ internal abstract class HttpTimeoutPolicy
public abstract string TimeoutPolicyName { get; }
public abstract int TotalRetryCount { get; }
public abstract IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> GetTimeoutEnumerator();
public abstract bool IsSafeToRetry(HttpMethod httpMethod);

public abstract bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, HttpResponseMessage responseMessage);

Expand Down
6 changes: 0 additions & 6 deletions Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRead.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ private HttpTimeoutPolicyControlPlaneRead()
return this.TimeoutsAndDelays.GetEnumerator();
}

// This is for control plane reads which should always be safe to retry on.
public override bool IsSafeToRetry(HttpMethod httpMethod)
{
return true;
}

public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, HttpResponseMessage responseMessage)
{
return false;
Expand Down
12 changes: 0 additions & 12 deletions Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,6 @@ private HttpTimeoutPolicyControlPlaneRetriableHotPath(bool shouldThrow503OnTimeo
return this.TimeoutsAndDelays.GetEnumerator();
}

// The hot path should always be safe to retires since it should be retrieving meta data
// information that is not idempotent.
public override bool IsSafeToRetry(HttpMethod httpMethod)
{
return true;
}

public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, HttpResponseMessage responseMessage)
{
if (responseMessage == null)
Expand All @@ -55,11 +48,6 @@ public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, Ht
return false;
}

if (!this.IsSafeToRetry(requestHttpMethod))
{
return false;
}

return true;
}

Expand Down
7 changes: 0 additions & 7 deletions Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,6 @@ private HttpTimeoutPolicyDefault(bool shouldThrow503OnTimeout)
return this.TimeoutsAndDelays.GetEnumerator();
}

// Assume that it is not safe to retry unless it is a get method.
// Create and other operations could have succeeded even though a timeout occurred.
public override bool IsSafeToRetry(HttpMethod httpMethod)
{
return httpMethod == HttpMethod.Get;
}

public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, HttpResponseMessage responseMessage)
{
return false;
Expand Down
7 changes: 0 additions & 7 deletions Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyForPartitionFailover.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,6 @@ private HttpTimeoutPolicyForPartitionFailover(bool shouldThrow503OnTimeout)
return this.TimeoutsAndDelays.GetEnumerator();
}

// Assume that it is not safe to retry unless it is a get method.
// Create and other operations could have succeeded even though a timeout occurred.
public override bool IsSafeToRetry(HttpMethod httpMethod)
{
return httpMethod == HttpMethod.Get;
}

public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, HttpResponseMessage responseMessage)
{
return false;
Expand Down
7 changes: 1 addition & 6 deletions Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyForThinClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ private HttpTimeoutPolicyForThinClient(
return this.TimeoutsAndDelays.GetEnumerator();
}

public override bool IsSafeToRetry(HttpMethod httpMethod)
{
return this.shouldRetry;
}

public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, HttpResponseMessage responseMessage)
{
if (responseMessage == null)
Expand All @@ -56,7 +51,7 @@ public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, Ht
return false;
}

if (!this.IsSafeToRetry(requestHttpMethod))
if (!this.shouldRetry)
{
return false;
}
Expand Down
6 changes: 0 additions & 6 deletions Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyNoRetry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ private HttpTimeoutPolicyNoRetry()
return this.TimeoutsAndDelays.GetEnumerator();
}

// Always Unsafe to retry
public override bool IsSafeToRetry(HttpMethod httpMethod)
{
return false;
}

public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, HttpResponseMessage responseMessage)
{
return false;
Expand Down
34 changes: 22 additions & 12 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosHttpClientCoreTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ async Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, Cancellatio
resourceType: ResourceType.Collection,
timeoutPolicy: currentTimeoutPolicy.Key,
clientSideRequestStatistics: new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace),
cancellationToken: default);
cancellationToken: default,
documentServiceRequest: CreateDocumentServiceRequestByOperation(ResourceType.Collection, OperationType.Read));

Assert.AreEqual(HttpStatusCode.OK, responseMessage.StatusCode);
}
Expand Down Expand Up @@ -266,7 +267,8 @@ Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, CancellationToken
resourceType: ResourceType.Collection,
timeoutPolicy: HttpTimeoutPolicyDefault.Instance,
clientSideRequestStatistics: new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace),
cancellationToken: default);
cancellationToken: default,
documentServiceRequest: CreateDocumentServiceRequestByOperation(ResourceType.Collection, OperationType.Read));
}
}
catch (Exception)
Expand All @@ -281,7 +283,7 @@ Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, CancellationToken
public async Task HttpTimeoutThrow503TestAsync()
{

async Task TestScenarioAsync(HttpMethod method, ResourceType resourceType, HttpTimeoutPolicy timeoutPolicy, Type expectedException, int expectedNumberOfRetrys)
async Task TestScenarioAsync(HttpMethod method, ResourceType resourceType, OperationType operationType, HttpTimeoutPolicy timeoutPolicy, Type expectedException, int expectedNumberOfRetrys)
{
int count = 0;
Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, CancellationToken cancellationToken)
Expand All @@ -306,7 +308,8 @@ Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, CancellationToken
resourceType: resourceType,
timeoutPolicy: timeoutPolicy,
clientSideRequestStatistics: new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace),
cancellationToken: default);
cancellationToken: default,
documentServiceRequest: CreateDocumentServiceRequestByOperation(resourceType, operationType));
}
}
catch (Exception e)
Expand All @@ -328,19 +331,19 @@ Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, CancellationToken
}

//Data plane read
await TestScenarioAsync(HttpMethod.Get, ResourceType.Document, HttpTimeoutPolicyDefault.InstanceShouldThrow503OnTimeout, typeof(CosmosException), 3);
await TestScenarioAsync(HttpMethod.Get, ResourceType.Document, OperationType.Read, HttpTimeoutPolicyDefault.InstanceShouldThrow503OnTimeout, typeof(CosmosException), 3);

//Data plane write (Should throw a 408 OperationCanceledException rather than a 503)
await TestScenarioAsync(HttpMethod.Post, ResourceType.Document, HttpTimeoutPolicyDefault.Instance, typeof(TaskCanceledException), 1);
await TestScenarioAsync(HttpMethod.Post, ResourceType.Document, OperationType.Upsert, HttpTimeoutPolicyDefault.Instance, typeof(TaskCanceledException), 1);

//Meta data read
await TestScenarioAsync(HttpMethod.Get, ResourceType.Database, HttpTimeoutPolicyDefault.InstanceShouldThrow503OnTimeout, typeof(CosmosException), 3);
await TestScenarioAsync(HttpMethod.Get, ResourceType.Database, OperationType.Read, HttpTimeoutPolicyDefault.InstanceShouldThrow503OnTimeout, typeof(CosmosException), 3);

//Query plan read (note all query plan operations are reads).
await TestScenarioAsync(HttpMethod.Get, ResourceType.Document, HttpTimeoutPolicyDefault.InstanceShouldThrow503OnTimeout, typeof(CosmosException), 3);
await TestScenarioAsync(HttpMethod.Get, ResourceType.Document, OperationType.Read, HttpTimeoutPolicyDefault.InstanceShouldThrow503OnTimeout, typeof(CosmosException), 3);

//Metadata Write (Should throw a 408 OperationCanceledException rather than a 503)
await TestScenarioAsync(HttpMethod.Post, ResourceType.Document, HttpTimeoutPolicyDefault.Instance, typeof(TaskCanceledException), 1);
await TestScenarioAsync(HttpMethod.Post, ResourceType.Document, OperationType.Upsert, HttpTimeoutPolicyDefault.Instance, typeof(TaskCanceledException), 1);
}

[TestMethod]
Expand Down Expand Up @@ -433,7 +436,8 @@ async Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, Cancellatio
resourceType: ResourceType.Document,
timeoutPolicy: HttpTimeoutPolicyControlPlaneRetriableHotPath.Instance,
clientSideRequestStatistics: new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace),
cancellationToken: default);
cancellationToken: default,
documentServiceRequest: documentServiceRequest);

Assert.AreEqual(HttpStatusCode.OK, responseMessage.StatusCode);
}
Expand Down Expand Up @@ -492,7 +496,7 @@ public void CreateHttpClientHandlerCreatesCorrectValueType()
public async Task HttpTimeoutPolicyForThinClientOn503TestAsync()
{

async Task TestScenarioAsync(HttpMethod method, ResourceType resourceType, HttpTimeoutPolicy timeoutPolicy, Type expectedException, int expectedNumberOfRetrys)
async Task TestScenarioAsync(HttpMethod method, ResourceType resourceType, OperationType operationType, HttpTimeoutPolicy timeoutPolicy, Type expectedException, int expectedNumberOfRetrys)
{
int count = 0;
Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, CancellationToken cancellationToken)
Expand All @@ -517,7 +521,8 @@ Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, CancellationToken
resourceType: resourceType,
timeoutPolicy: timeoutPolicy,
clientSideRequestStatistics: new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace),
cancellationToken: default);
cancellationToken: default,
documentServiceRequest: CreateDocumentServiceRequestByOperation(resourceType, operationType));
}
}
catch (Exception e)
Expand All @@ -542,6 +547,7 @@ Task<HttpResponseMessage> sendFunc(HttpRequestMessage request, CancellationToken
await TestScenarioAsync(
method: HttpMethod.Get,
resourceType: ResourceType.Document,
operationType: OperationType.Read,
timeoutPolicy: HttpTimeoutPolicy.GetTimeoutPolicy(
documentServiceRequest: CosmosHttpClientCoreTests.CreateDocumentServiceRequestByOperation(ResourceType.Document, OperationType.Read),
isPartitionLevelFailoverEnabled: false,
Expand All @@ -553,6 +559,7 @@ await TestScenarioAsync(
await TestScenarioAsync(
method: HttpMethod.Get,
resourceType: ResourceType.Document,
operationType: OperationType.Read,
timeoutPolicy: HttpTimeoutPolicy.GetTimeoutPolicy(
documentServiceRequest: CosmosHttpClientCoreTests.CreateDocumentServiceRequestByOperation(ResourceType.Document, OperationType.Query),
isPartitionLevelFailoverEnabled: false,
Expand All @@ -564,6 +571,7 @@ await TestScenarioAsync(
await TestScenarioAsync(
method: HttpMethod.Post,
resourceType: ResourceType.Document,
operationType: OperationType.Upsert,
timeoutPolicy: HttpTimeoutPolicy.GetTimeoutPolicy(
documentServiceRequest: CosmosHttpClientCoreTests.CreateDocumentServiceRequestByOperation(ResourceType.Document, OperationType.Create),
isPartitionLevelFailoverEnabled: false,
Expand All @@ -575,6 +583,7 @@ await TestScenarioAsync(
await TestScenarioAsync(
method: HttpMethod.Get,
resourceType: ResourceType.Database,
operationType: OperationType.Read,
timeoutPolicy: HttpTimeoutPolicy.GetTimeoutPolicy(
documentServiceRequest: CosmosHttpClientCoreTests.CreateDocumentServiceRequestByOperation(ResourceType.Database, OperationType.Read),
isPartitionLevelFailoverEnabled: false,
Expand All @@ -583,6 +592,7 @@ await TestScenarioAsync(
expectedNumberOfRetrys: 3);
}


private static DocumentServiceRequest CreateDocumentServiceRequestByOperation(
ResourceType resourceType,
OperationType operationType)
Expand Down
13 changes: 10 additions & 3 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,15 +889,22 @@ public async Task GatewayStatsDurationTest()
DocumentClientEventSource.Instance);

using (ITrace trace = Tracing.Trace.GetRootTrace(nameof(GatewayStatsDurationTest)))
{

{
Tracing.TraceData.ClientSideRequestStatisticsTraceDatum clientSideRequestStatistics = new Tracing.TraceData.ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace);

await cosmosHttpClient.SendHttpAsync(() => new ValueTask<HttpRequestMessage>(new HttpRequestMessage(HttpMethod.Get, "http://someuri.com")),
ResourceType.Document,
HttpTimeoutPolicyDefault.InstanceShouldThrow503OnTimeout,
clientSideRequestStatistics,
CancellationToken.None);
CancellationToken.None,
documentServiceRequest: new DocumentServiceRequest(
OperationType.Read,
ResourceType.Document,
$"dbs/dummy_db_id/colls/dummy_ct_id",
body: null,
AuthorizationTokenType.PrimaryMasterKey,
headers: null));

Assert.AreEqual(clientSideRequestStatistics.HttpResponseStatisticsList.Count, 2);
// The duration is calculated using date times which can cause the duration to be slightly off. This allows for up to 15 Ms of variance.
Expand Down
Loading