HttpTimeoutPolicy Improvements Phase 3: Fixes the TimeoutPolicies to Have Segregated Timeouts for Point-Reads and Non-Point-Reads with Uniform Timeouts

Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicy.cs

@@ -48,9 +48,16 @@ public static HttpTimeoutPolicy GetTimeoutPolicy(
             {
                 if (isThinClientEnabled)
                 {
-                    return documentServiceRequest.IsReadOnlyRequest
-                        ? HttpTimeoutPolicyForThinClient.InstanceShouldRetryAndThrow503OnTimeout
-                        : HttpTimeoutPolicyForThinClient.InstanceShouldNotRetryAndThrow503OnTimeout;
+                    if (documentServiceRequest.IsReadOnlyRequest)
+                    {
+                        return documentServiceRequest.OperationType == OperationType.Read
+                            ? HttpTimeoutPolicyForThinClient.InstanceShouldRetryAndThrow503OnTimeoutForPointReads
+                            : HttpTimeoutPolicyForThinClient.InstanceShouldRetryAndThrow503OnTimeoutForNonPointReads;
+                    }
+                    else
+                    {
+                        return HttpTimeoutPolicyForThinClient.InstanceShouldNotRetryAndThrow503OnTimeoutForWrites;
+                    }
                 }
                 // Data Plane Reads.
                 else if (documentServiceRequest.IsReadOnlyRequest)

Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyForPartitionFailover.cs

@@ -20,13 +20,13 @@ private HttpTimeoutPolicyForPartitionFailover(bool isPointRead)
         }
 
         // Timeouts and delays are based on the following rationale:
-        // For point reads: 3 attempts with timeouts of 1s, 6s, and 6s respectively.
+        // For point reads: 3 attempts with timeouts of 6s, 6s, and 10s respectively.
         // For non-point reads: 3 attempts with timeouts of 6s, 6s, and 10s respectively.
         private readonly IReadOnlyList<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> TimeoutsAndDelaysForPointReads = new List<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)>()
         {
-            (TimeSpan.FromSeconds(1), TimeSpan.Zero),
             (TimeSpan.FromSeconds(6), TimeSpan.Zero),
             (TimeSpan.FromSeconds(6), TimeSpan.Zero),
+            (TimeSpan.FromSeconds(10), TimeSpan.Zero),
         };
 
         private readonly IReadOnlyList<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> TimeoutsAndDelaysForNonPointReads = new List<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)>()

Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyForThinClient.cs

@@ -11,32 +11,43 @@ internal sealed class HttpTimeoutPolicyForThinClient : HttpTimeoutPolicy
     {
         public bool shouldRetry;
         public bool shouldThrow503OnTimeout;
+        public bool isPointRead;
         private static readonly string Name = nameof(HttpTimeoutPolicyForThinClient);
-        public static readonly HttpTimeoutPolicy InstanceShouldRetryAndThrow503OnTimeout = new HttpTimeoutPolicyForThinClient(true, true);
-        public static readonly HttpTimeoutPolicy InstanceShouldNotRetryAndThrow503OnTimeout = new HttpTimeoutPolicyForThinClient(true, false);
+        public static readonly HttpTimeoutPolicy InstanceShouldRetryAndThrow503OnTimeoutForPointReads = new HttpTimeoutPolicyForThinClient(shouldThrow503OnTimeout: true, shouldRetry: true, isPointRead: true);
+        public static readonly HttpTimeoutPolicy InstanceShouldRetryAndThrow503OnTimeoutForNonPointReads = new HttpTimeoutPolicyForThinClient(shouldThrow503OnTimeout: true, shouldRetry: true, isPointRead: false);
+        public static readonly HttpTimeoutPolicy InstanceShouldNotRetryAndThrow503OnTimeoutForWrites = new HttpTimeoutPolicyForThinClient(shouldThrow503OnTimeout: true, shouldRetry: false, isPointRead: false);
 
         private HttpTimeoutPolicyForThinClient(
             bool shouldThrow503OnTimeout,
-            bool shouldRetry)
+            bool shouldRetry,
+            bool isPointRead)
         {
             this.shouldThrow503OnTimeout = shouldThrow503OnTimeout;
             this.shouldRetry = shouldRetry;
+            this.isPointRead = isPointRead;
         }
 
-        private readonly IReadOnlyList<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> TimeoutsAndDelays = new List<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)>()
+        private readonly IReadOnlyList<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> TimeoutsAndDelaysForPointReads = new List<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)>()
         {
-            (TimeSpan.FromSeconds(.5), TimeSpan.Zero),
-            (TimeSpan.FromSeconds(1), TimeSpan.Zero),
-            (TimeSpan.FromSeconds(5), TimeSpan.Zero),
+            (TimeSpan.FromSeconds(6), TimeSpan.Zero),
+            (TimeSpan.FromSeconds(6), TimeSpan.Zero),
+            (TimeSpan.FromSeconds(10), TimeSpan.Zero),
+        };
+
+        private readonly IReadOnlyList<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> TimeoutsAndDelaysForNonPointReads = new List<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)>()
+        {
+            (TimeSpan.FromSeconds(6), TimeSpan.Zero),
+            (TimeSpan.FromSeconds(6), TimeSpan.Zero),
+            (TimeSpan.FromSeconds(10), TimeSpan.Zero),
         };
 
         public override string TimeoutPolicyName => HttpTimeoutPolicyForThinClient.Name;
 
-        public override int TotalRetryCount => this.TimeoutsAndDelays.Count;
+        public override int TotalRetryCount => this.isPointRead ? this.TimeoutsAndDelaysForPointReads.Count : this.TimeoutsAndDelaysForNonPointReads.Count;
 
         public override IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> GetTimeoutEnumerator()
         {
-            return this.TimeoutsAndDelays.GetEnumerator();
+            return this.isPointRead ? this.TimeoutsAndDelaysForPointReads.GetEnumerator() : this.TimeoutsAndDelaysForNonPointReads.GetEnumerator();
         }
 
         public override bool ShouldRetryBasedOnResponse(HttpMethod requestHttpMethod, HttpResponseMessage responseMessage)

Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosHttpClientCoreTests.cs

@@ -619,23 +619,71 @@ public void HttpTimeoutPolicyForParitionFailoverForQueries()
         [TestMethod]
         public void HttpTimeoutPolicyForParitionFailoverForReads()
         {
-            HttpTimeoutPolicy httpTimeoutPolicyForQuery = HttpTimeoutPolicy.GetTimeoutPolicy(
+            HttpTimeoutPolicy httpTimeoutPolicyForPointReads = HttpTimeoutPolicy.GetTimeoutPolicy(
                     documentServiceRequest: CosmosHttpClientCoreTests.CreateDocumentServiceRequestByOperation(ResourceType.Document, OperationType.Read),
                     isPartitionLevelFailoverEnabled: true,
                     isThinClientEnabled: false);
-            IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> availableRetries = httpTimeoutPolicyForQuery.GetTimeoutEnumerator();
+            IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> availableRetries = httpTimeoutPolicyForPointReads.GetTimeoutEnumerator();
 
             int count = 0;
             while (availableRetries.MoveNext())
             {
-                if (count == 0)
+                if (count <= 1)
+                {
+                    Assert.AreEqual(new TimeSpan(0, 0, 6), availableRetries.Current.requestTimeout);
+                }
+                else if (count == 2)
+                {
+                    Assert.AreEqual(new TimeSpan(0, 0, 10), availableRetries.Current.requestTimeout);
+                }
+                count++;
+            }
+        }
+
+        [TestMethod]
+        public void HttpTimeoutPolicyWhenThinClientEnabledForPointReads()
+        {
+            HttpTimeoutPolicy httpTimeoutPolicyForPointReads = HttpTimeoutPolicy.GetTimeoutPolicy(
+                    documentServiceRequest: CosmosHttpClientCoreTests.CreateDocumentServiceRequestByOperation(ResourceType.Document, OperationType.Read),
+                    isPartitionLevelFailoverEnabled: false,
+                    isThinClientEnabled: true);
+            IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> availableRetries = httpTimeoutPolicyForPointReads.GetTimeoutEnumerator();
+
+            int count = 0;
+            while (availableRetries.MoveNext())
+            {
+                if (count <= 1)
                 {
-                    Assert.AreEqual(new TimeSpan(0, 0, 1), availableRetries.Current.requestTimeout);
+                    Assert.AreEqual(new TimeSpan(0, 0, 6), availableRetries.Current.requestTimeout);
+                }
+                else if (count == 2)
+                {
+                    Assert.AreEqual(new TimeSpan(0, 0, 10), availableRetries.Current.requestTimeout);
                 }
-                else if (count == 1 || count ==2 )
+                count++;
+            }
+        }
+
+        [TestMethod]
+        public void HttpTimeoutPolicyWhenThinClientEnabledForNonPointReads()
+        {
+            HttpTimeoutPolicy httpTimeoutPolicyForQuery = HttpTimeoutPolicy.GetTimeoutPolicy(
+                    documentServiceRequest: CosmosHttpClientCoreTests.CreateDocumentServiceRequestByOperation(ResourceType.Document, OperationType.Query),
+                    isPartitionLevelFailoverEnabled: false,
+                    isThinClientEnabled: true);
+            IEnumerator<(TimeSpan requestTimeout, TimeSpan delayForNextRequest)> availableRetries = httpTimeoutPolicyForQuery.GetTimeoutEnumerator();
+
+            int count = 0;
+            while (availableRetries.MoveNext())
+            {
+                if (count <= 1)
                 {
                     Assert.AreEqual(new TimeSpan(0, 0, 6), availableRetries.Current.requestTimeout);
                 }
+                else if (count == 2)
+                {
+                    Assert.AreEqual(new TimeSpan(0, 0, 10), availableRetries.Current.requestTimeout);
+                }
                 count++;
             }
         }