From d45830c229ecfc8261e378167635805663f10dbd Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 27 Jun 2023 10:14:44 +0000 Subject: [PATCH 1/2] Fix HttpTimeoutPolicies to not accidentally suppress retries --- .../HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs | 2 +- .../src/HttpClient/HttpTimeoutPolicyDefault.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs index 8dbb8a2a44..52be600ac8 100644 --- a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs +++ b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs @@ -29,7 +29,7 @@ private HttpTimeoutPolicyControlPlaneRetriableHotPath(bool shouldThrow503OnTimeo public override string TimeoutPolicyName => HttpTimeoutPolicyControlPlaneRetriableHotPath.Name; - public override TimeSpan MaximumRetryTimeLimit => CosmosHttpClient.GatewayRequestTimeout; + public override TimeSpan MaximumRetryTimeLimit => CosmosHttpClient.GatewayRequestTimeout + TimeSpan.FromSeconds(7.5); // 0.5s + 5 s + 65s + 1s delay + 1s buffer public override int TotalRetryCount => this.TimeoutsAndDelays.Count; diff --git a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs index 974509c70c..5121152fa0 100644 --- a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs +++ b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs @@ -29,7 +29,7 @@ private HttpTimeoutPolicyDefault(bool shouldThrow503OnTimeout) public override string TimeoutPolicyName => HttpTimeoutPolicyDefault.Name; - public override TimeSpan MaximumRetryTimeLimit => CosmosHttpClient.GatewayRequestTimeout; + public override TimeSpan MaximumRetryTimeLimit => TimeSpan.FromSeconds((3 * CosmosHttpClient.GatewayRequestTimeout.TotalSeconds) + 1); // 3 times 65s + 1s delay + 1s buffer public override int TotalRetryCount => this.TimeoutsAndDelays.Count; From e48f2e0a7d5b6103c9a10a677062d4f762d964d0 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 27 Jun 2023 14:42:23 +0000 Subject: [PATCH 2/2] Removing HttpTimeoutPolicy.MaxRetryTimeLimit altogether --- Microsoft.Azure.Cosmos/src/HttpClient/CosmosHttpClientCore.cs | 3 +-- Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicy.cs | 1 - .../src/HttpClient/HttpTimeoutPolicyControlPlaneRead.cs | 2 -- .../HttpTimeoutPolicyControlPlaneRetriableHotPath.cs | 2 -- .../src/HttpClient/HttpTimeoutPolicyDefault.cs | 2 -- .../src/HttpClient/HttpTimeoutPolicyNoRetry.cs | 2 -- 6 files changed, 1 insertion(+), 11 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/HttpClient/CosmosHttpClientCore.cs b/Microsoft.Azure.Cosmos/src/HttpClient/CosmosHttpClientCore.cs index c29c3a5b94..0e7cc30c9e 100644 --- a/Microsoft.Azure.Cosmos/src/HttpClient/CosmosHttpClientCore.cs +++ b/Microsoft.Azure.Cosmos/src/HttpClient/CosmosHttpClientCore.cs @@ -438,8 +438,7 @@ private static bool IsOutOfRetries( DateTime startDateTimeUtc, IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> timeoutEnumerator) { - return (DateTime.UtcNow - startDateTimeUtc) > timeoutPolicy.MaximumRetryTimeLimit || // Maximum of time for all retries - !timeoutEnumerator.MoveNext(); // No more retries are configured + return !timeoutEnumerator.MoveNext(); // No more retries are configured } private async Task ExecuteHttpHelperAsync( diff --git a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicy.cs b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicy.cs index fa2fc25bf7..42742f86c1 100644 --- a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicy.cs @@ -11,7 +11,6 @@ namespace Microsoft.Azure.Cosmos internal abstract class HttpTimeoutPolicy { public abstract string TimeoutPolicyName { get; } - public abstract TimeSpan MaximumRetryTimeLimit { get; } public abstract int TotalRetryCount { get; } public abstract IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> GetTimeoutEnumerator(); public abstract bool IsSafeToRetry(HttpMethod httpMethod); diff --git a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRead.cs b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRead.cs index 757134a576..9f85ba33ab 100644 --- a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRead.cs +++ b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRead.cs @@ -25,8 +25,6 @@ private HttpTimeoutPolicyControlPlaneRead() public override string TimeoutPolicyName => HttpTimeoutPolicyControlPlaneRead.Name; - public override TimeSpan MaximumRetryTimeLimit => CosmosHttpClient.GatewayRequestTimeout; - public override int TotalRetryCount => this.TimeoutsAndDelays.Count; public override IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> GetTimeoutEnumerator() diff --git a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs index 52be600ac8..53a7c78e9e 100644 --- a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs +++ b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyControlPlaneRetriableHotPath.cs @@ -29,8 +29,6 @@ private HttpTimeoutPolicyControlPlaneRetriableHotPath(bool shouldThrow503OnTimeo public override string TimeoutPolicyName => HttpTimeoutPolicyControlPlaneRetriableHotPath.Name; - public override TimeSpan MaximumRetryTimeLimit => CosmosHttpClient.GatewayRequestTimeout + TimeSpan.FromSeconds(7.5); // 0.5s + 5 s + 65s + 1s delay + 1s buffer - public override int TotalRetryCount => this.TimeoutsAndDelays.Count; public override IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> GetTimeoutEnumerator() diff --git a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs index 5121152fa0..69a83707a1 100644 --- a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs +++ b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyDefault.cs @@ -29,8 +29,6 @@ private HttpTimeoutPolicyDefault(bool shouldThrow503OnTimeout) public override string TimeoutPolicyName => HttpTimeoutPolicyDefault.Name; - public override TimeSpan MaximumRetryTimeLimit => TimeSpan.FromSeconds((3 * CosmosHttpClient.GatewayRequestTimeout.TotalSeconds) + 1); // 3 times 65s + 1s delay + 1s buffer - public override int TotalRetryCount => this.TimeoutsAndDelays.Count; public override IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> GetTimeoutEnumerator() diff --git a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyNoRetry.cs b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyNoRetry.cs index 595498aec2..65ea12144b 100644 --- a/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyNoRetry.cs +++ b/Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyNoRetry.cs @@ -24,8 +24,6 @@ private HttpTimeoutPolicyNoRetry() public override string TimeoutPolicyName => HttpTimeoutPolicyNoRetry.Name; - public override TimeSpan MaximumRetryTimeLimit => TimeSpan.Zero; - public override int TotalRetryCount => 0; public override IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> GetTimeoutEnumerator()