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

# Pull Request Template

## Description

At a high-level, this PR aims to unify the timeouts across PPAF and thin
client http retry policies for read only requests. Furthermore,
HttpTimeoutPolicies for PPAF and Thinclient will have segregated
timeouts for Point-Reads and Non-Point-Reads. With this change, we will
have the ability to fine-tune/optimize the timeouts for the respective
operations. In this context, requests are allowed to retry 3 times with
6s, 6s and 10s timeouts respectively for each retry. We chose these
timeouts based on the default backend response timeouts and it will help
requests being timeout with aggressive timeouts.

As next step, we have planned for DR drill to identify the appropriate
timeouts. Depending on the findings from the drill, we may go towards
the direction of configurable timeout for first retry.

## Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)

## Closing issues

To automatically close an issue: closes #5496

Author: Praveen-Msft

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++;
             }
         }