ClientRetryPolicy: Fixes behavior to Meta-data write operations in mu…

…ltimaster accounts (#3466)

* Changes behavior to LocationCache and ClientRetryPolicy for metadata write operations

* Changes behavior to LocationCache and ClientRetryPolicy for metadata write operations

* Suggested changes

* fixed added bugs

* Was not checking to see if request was actually metadata request.

* suggested changes and tests

* Cleaning up tests and removing global variable by adding functionality into retryContext

* remove unneccary code

* fixed IsMetadataWriteREquestOnMultimasterAccount

* added missed case to IsMetadataWriteREquestOnMultimasterAccount

* suggested changes

Co-authored-by: Nalu Tripician <ntripician@microsoft.com>

Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs

@@ -137,8 +137,15 @@ public void OnBeforeSendRequest(DocumentServiceRequest request)
 
             if (this.retryContext != null)
             {
-                // set location-based routing directive based on request retry context
-                request.RequestContext.RouteToLocation(this.retryContext.RetryLocationIndex, this.retryContext.RetryRequestOnPreferredLocations);
+                if (this.retryContext.RouteToHub)
+                {
+                    request.RequestContext.RouteToLocation(this.globalEndpointManager.GetHubUri());
+                }
+                else
+                {
+                    // set location-based routing directive based on request retry context
+                    request.RequestContext.RouteToLocation(this.retryContext.RetryLocationIndex, this.retryContext.RetryRequestOnPreferredLocations);
+                }
             }
 
             // Resolve the endpoint for the request and pin the resolution to the resolved endpoint
@@ -185,6 +192,31 @@ private async Task<ShouldRetryResult> ShouldRetryInternalAsync(
                     this.documentServiceRequest?.RequestContext?.LocationEndpointToRoute?.ToString() ?? string.Empty,
                     this.documentServiceRequest?.ResourceAddress ?? string.Empty);
 
+                if (this.globalEndpointManager.IsMultimasterMetadataWriteRequest(this.documentServiceRequest))
+                {
+                    bool forceRefresh = false;
+
+                    if (this.retryContext != null && this.retryContext.RouteToHub)
+                    {
+                        forceRefresh = true;
+
+                    }
+
+                    ShouldRetryResult retryResult = await this.ShouldRetryOnEndpointFailureAsync(
+                        isReadRequest: false,
+                        markBothReadAndWriteAsUnavailable: false,
+                        forceRefresh: forceRefresh,
+                        retryOnPreferredLocations: false,
+                        overwriteEndpointDiscovery: true);
+
+                    if (retryResult.ShouldRetry)
+                    {
+                        this.retryContext.RouteToHub = true;
+                    }
+
+                    return retryResult;
+                }
+
                 return await this.ShouldRetryOnEndpointFailureAsync(
                     isReadRequest: false,
                     markBothReadAndWriteAsUnavailable: false,
@@ -243,9 +275,10 @@ private async Task<ShouldRetryResult> ShouldRetryOnEndpointFailureAsync(
             bool isReadRequest,
             bool markBothReadAndWriteAsUnavailable,
             bool forceRefresh,
-            bool retryOnPreferredLocations)
+            bool retryOnPreferredLocations,
+            bool overwriteEndpointDiscovery = false)
         {
-            if (!this.enableEndpointDiscovery || this.failoverRetryCount > MaxRetryCount)
+            if (this.failoverRetryCount > MaxRetryCount || (!this.enableEndpointDiscovery && !overwriteEndpointDiscovery))
             {
                 DefaultTrace.TraceInformation("ClientRetryPolicy: ShouldRetryOnEndpointFailureAsync() Not retrying. Retry count = {0}, Endpoint = {1}", 
                     this.failoverRetryCount,
@@ -255,7 +288,7 @@ private async Task<ShouldRetryResult> ShouldRetryOnEndpointFailureAsync(
 
             this.failoverRetryCount++;
 
-            if (this.locationEndpoint != null)
+            if (this.locationEndpoint != null && !overwriteEndpointDiscovery)
             {
                 if (isReadRequest || markBothReadAndWriteAsUnavailable)
                 {
@@ -400,6 +433,8 @@ private sealed class RetryContext
         {
             public int RetryLocationIndex { get; set; }
             public bool RetryRequestOnPreferredLocations { get; set; }
+
+            public bool RouteToHub { get; set; }
         }
     }
 }

Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs

@@ -95,6 +95,16 @@ public GlobalEndpointManager(IDocumentClientInternal owner, ConnectionPolicy con
 
         public int PreferredLocationCount => this.connectionPolicy.PreferredLocations != null ? this.connectionPolicy.PreferredLocations.Count : 0;
 
+        public bool IsMultimasterMetadataWriteRequest(DocumentServiceRequest request)
+        {
+            return this.locationCache.IsMultimasterMetadataWriteRequest(request);
+        }
+
+        public Uri GetHubUri()
+        {
+            return this.locationCache.GetHubUri();
+        }
+
         /// <summary>
         /// This will get the account information.
         /// It will try the global endpoint first. 
@@ -541,7 +551,6 @@ private async Task RefreshDatabaseAccountInternalAsync(bool forceRefresh)
                 }
             }
         }
-
         internal async Task<AccountProperties> GetDatabaseAccountAsync(bool forceRefresh = false)
         {
 #nullable disable  // Needed because AsyncCache does not have nullable enabled

Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs

@@ -8,8 +8,10 @@ namespace Microsoft.Azure.Cosmos.Routing
     using System.Collections.Concurrent;
     using System.Collections.Generic;
     using System.Collections.ObjectModel;
+    using System.Globalization;
     using System.Linq;
     using System.Net;
+    using global::Azure.Core;
     using Microsoft.Azure.Cosmos.Core.Trace;
     using Microsoft.Azure.Documents;
 
@@ -206,6 +208,28 @@ public void OnLocationPreferenceChanged(ReadOnlyCollection<string> preferredLoca
                 preferenceList: preferredLocations);
         }
 
+        public bool IsMetaData(DocumentServiceRequest request)
+        {
+            return (request.OperationType != Documents.OperationType.ExecuteJavaScript && request.ResourceType == ResourceType.StoredProcedure) ||
+                request.ResourceType != ResourceType.Document;
+
+        }
+        public bool IsMultimasterMetadataWriteRequest(DocumentServiceRequest request)
+        {
+            return !request.IsReadOnlyRequest && this.locationInfo.AvailableWriteLocations.Count > 1
+                && this.IsMetaData(request) 
+                && this.CanUseMultipleWriteLocations();
+
+        }
+
+        public Uri GetHubUri()
+        {
+            DatabaseAccountLocationsInfo currentLocationInfo = this.locationInfo;
+            string writeLocation = currentLocationInfo.AvailableWriteLocations[0];
+            Uri locationEndpointToRoute = currentLocationInfo.AvailableWriteEndpointByLocation[writeLocation];
+            return locationEndpointToRoute;
+        }
+
         /// <summary>
         /// Resolves request to service endpoint. 
         /// 1. If this is a write request

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

@@ -1,25 +1,18 @@
 namespace Microsoft.Azure.Cosmos.Client.Tests
 {
     using System;
-    using Microsoft.VisualStudio.TestTools.UnitTesting;
     using Microsoft.Azure.Cosmos.Routing;
-    using Moq;
     using Microsoft.Azure.Documents;
+    using Microsoft.VisualStudio.TestTools.UnitTesting;
+    using Moq;
     using System.Collections.Generic;
     using System.Collections.ObjectModel;
     using System.Globalization;
     using System.Linq;
     using System.Net;
-    using System.Net.Http;
     using System.Threading;
     using System.Threading.Tasks;
-    using Microsoft.Azure.Cosmos.Core.Trace;
     using Microsoft.Azure.Documents.Collections;
-    using Microsoft.Azure.Documents.Routing;
-    using System.Net.WebSockets;
-    using System.Net.Http.Headers;
-    using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
-    using System.Collections.Specialized;
     using Microsoft.Azure.Documents.Client;
     using Microsoft.Azure.Cosmos.Common;
 
@@ -37,6 +30,60 @@ public sealed class ClientRetryPolicyTests
         private GlobalPartitionEndpointManager partitionKeyRangeLocationCache;
         private Mock<IDocumentClientInternal> mockedClient;
 
+        /// <summary>
+        /// Tests behavior of Multimaster Accounts on metadata writes where the default location is not the hub region
+        /// </summary>
+        [TestMethod]
+        public void MultimasterMetadataWriteRetryTest()
+        {
+            const bool enableEndpointDiscovery = false;
+
+            //Creates GlobalEndpointManager where enableEndpointDiscovery is False and
+            //Default location is false
+            using GlobalEndpointManager endpointManager = this.Initialize(
+                useMultipleWriteLocations: true,
+                enableEndpointDiscovery: enableEndpointDiscovery,
+                isPreferredLocationsListEmpty: true,
+                multimasterMetadataWriteRetryTest: true);
+
+
+            ClientRetryPolicy retryPolicy = new ClientRetryPolicy(endpointManager, this.partitionKeyRangeLocationCache, enableEndpointDiscovery, new RetryOptions());
+
+            //Creates a metadata write request
+            DocumentServiceRequest request = this.CreateRequest(false, true);
+
+            Assert.IsTrue(endpointManager.IsMultimasterMetadataWriteRequest(request));
+
+            //On first attempt should get incorrect (default/non hub) location
+            retryPolicy.OnBeforeSendRequest(request);
+            Assert.AreEqual(request.RequestContext.LocationEndpointToRoute, ClientRetryPolicyTests.Location2Endpoint);
+
+            //Creation of 403.3 Error
+            HttpStatusCode forbidden = HttpStatusCode.Forbidden;
+            SubStatusCodes writeForbidden = SubStatusCodes.WriteForbidden;
+            Exception forbiddenWriteFail = new Exception();
+            Mock<INameValueCollection> nameValueCollection = new Mock<INameValueCollection>();
+
+            DocumentClientException documentClientException = new DocumentClientException(
+                message: "Multimaster Metadata Write Fail",
+                innerException: forbiddenWriteFail,
+                statusCode: forbidden,
+                substatusCode: writeForbidden,
+                requestUri: request.RequestContext.LocationEndpointToRoute,
+                responseHeaders: nameValueCollection.Object);
+
+            CancellationToken cancellationToken = new CancellationToken();
+
+            //Tests behavior of should retry
+            Task<ShouldRetryResult> shouldRetry = retryPolicy.ShouldRetryAsync(documentClientException, cancellationToken);
+
+            Assert.IsTrue(shouldRetry.Result.ShouldRetry);
+
+            //Now since the retry context is not null, should route to the hub region
+            retryPolicy.OnBeforeSendRequest(request);
+            Assert.AreEqual(request.RequestContext.LocationEndpointToRoute, ClientRetryPolicyTests.Location1Endpoint);
+        }
+
         /// <summary>
         /// Tests to see if different 503 substatus codes are handeled correctly
         /// </summary>
@@ -338,6 +385,18 @@ private GlobalEndpointManager Initialize(
             return endpointManager;
         }
 
+        private DocumentServiceRequest CreateRequest(bool isReadRequest, bool isMasterResourceType)
+        {
+            if (isReadRequest)
+            {
+                return DocumentServiceRequest.Create(OperationType.Read, isMasterResourceType ? ResourceType.Database : ResourceType.Document, AuthorizationTokenType.PrimaryMasterKey);
+            }
+            else
+            {
+                return DocumentServiceRequest.Create(OperationType.Create, isMasterResourceType ? ResourceType.Database : ResourceType.Document, AuthorizationTokenType.PrimaryMasterKey);
+            }
+        }
+
         private MockDocumentClientContext InitializeMockedDocumentClient(
             bool useMultipleWriteLocations,
             bool isPreferredLocationsListEmpty)