From 658c4c819f4260c4187ab4c2e2bb975ab2383cb0 Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Tue, 20 Sep 2022 16:58:42 -0400 Subject: [PATCH 01/11] Changes behavior to LocationCache and ClientRetryPolicy for metadata write operations --- .../src/ClientRetryPolicy.cs | 20 +++++++++++++++-- .../src/Routing/GlobalEndpointManager.cs | 6 ++++- .../src/Routing/LocationCache.cs | 11 +++++++++- .../GlobalEndpointManagerTest.cs | 22 +++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs index 4d5d352a41..25d958965e 100644 --- a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs @@ -139,6 +139,11 @@ public void OnBeforeSendRequest(DocumentServiceRequest request) { // set location-based routing directive based on request retry context request.RequestContext.RouteToLocation(this.retryContext.RetryLocationIndex, this.retryContext.RetryRequestOnPreferredLocations); + + if (!this.canUseMultipleWriteLocations && this.globalEndpointManager.GetTotalAvailableWriteLocations > 1 && !this.isReadRequest) + { + request.RequestContext.IsRetry = true; + } } // Resolve the endpoint for the request and pin the resolution to the resolved endpoint @@ -185,6 +190,16 @@ private async Task ShouldRetryInternalAsync( this.documentServiceRequest?.RequestContext?.LocationEndpointToRoute?.ToString() ?? string.Empty, this.documentServiceRequest?.ResourceAddress ?? string.Empty); + if (!this.canUseMultipleWriteLocations && this.globalEndpointManager.GetTotalAvailableWriteLocations > 1 && !this.isReadRequest) + { + return await this.ShouldRetryOnEndpointFailureAsync( + isReadRequest: false, + markBothReadAndWriteAsUnavailable: false, + forceRefresh: true, + retryOnPreferredLocations: false, + isMetadataWrite: true); + } + return await this.ShouldRetryOnEndpointFailureAsync( isReadRequest: false, markBothReadAndWriteAsUnavailable: false, @@ -237,9 +252,10 @@ private async Task ShouldRetryOnEndpointFailureAsync( bool isReadRequest, bool markBothReadAndWriteAsUnavailable, bool forceRefresh, - bool retryOnPreferredLocations) + bool retryOnPreferredLocations, + bool isMetadataWrite = false) { - if (!this.enableEndpointDiscovery || this.failoverRetryCount > MaxRetryCount) + if (this.failoverRetryCount > MaxRetryCount || (!this.enableEndpointDiscovery && !isMetadataWrite)) { DefaultTrace.TraceInformation("ClientRetryPolicy: ShouldRetryOnEndpointFailureAsync() Not retrying. Retry count = {0}, Endpoint = {1}", this.failoverRetryCount, diff --git a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs index 09d83aaa7d..9e89e50501 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs @@ -95,6 +95,11 @@ public GlobalEndpointManager(IDocumentClientInternal owner, ConnectionPolicy con public int PreferredLocationCount => this.connectionPolicy.PreferredLocations != null ? this.connectionPolicy.PreferredLocations.Count : 0; + /// + /// Gets total availible write locations from the current location information for a Cosmos account + /// + public int GetTotalAvailableWriteLocations => this.locationCache.GetTotalAvailableWriteLocations; + /// /// This will get the account information. /// It will try the global endpoint first. @@ -541,7 +546,6 @@ private async Task RefreshDatabaseAccountInternalAsync(bool forceRefresh) } } } - internal async Task GetDatabaseAccountAsync(bool forceRefresh = false) { #nullable disable // Needed because AsyncCache does not have nullable enabled diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index 6075a216ec..9ecb48257e 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -78,7 +78,10 @@ public LocationCache( #endif #endif } - + /// + /// Gets total availible write locations from the current location information for a Cosmos account + /// + public int GetTotalAvailableWriteLocations => this.locationInfo.AvailableWriteLocations.Count; /// /// Gets list of read endpoints ordered by /// 1. Preferred location @@ -249,6 +252,12 @@ public Uri ResolveServiceEndpoint(DocumentServiceRequest request) string writeLocation = currentLocationInfo.AvailableWriteLocations[locationIndex]; locationEndpointToRoute = currentLocationInfo.AvailableWriteEndpointByLocation[writeLocation]; } + else if (request.RequestContext.IsRetry && currentLocationInfo.AvailableWriteLocations.Count > 1 && request.ResourceType != ResourceType.Document) + { + Console.WriteLine(currentLocationInfo); + string writeLocation = currentLocationInfo.AvailableWriteLocations[0]; //Always want to rout to hub region, the first one in the list of available regions + locationEndpointToRoute = currentLocationInfo.AvailableWriteEndpointByLocation[writeLocation]; + } } else { diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs index 28e24d7e55..0cacd45cbf 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs @@ -562,6 +562,28 @@ void TraceHandler(string message) Environment.SetEnvironmentVariable("MinimumIntervalForNonForceRefreshLocationInMS", originalConfigValue); } + [TestMethod] + public async Task MultiMasterRoutingTest() + { + string EndpointUri = "https://ntripician-eastus.documents.azure.com:443/"; + string PrimaryKey = "LpUcGJC29hEyf9BqVFZVlZBGEHGhXMeY5k9BtJwwZY0xl7aXBAsPN5ilalkktiwdceHbKfyAShNoHpsBuPldLA=="; + + CosmosClient cosmosClient = new CosmosClient( + EndpointUri, + PrimaryKey, + new CosmosClientOptions + { + LimitToEndpoint = true + }); + string databaseString = Guid.NewGuid().ToString(); + Console.WriteLine(databaseString); + await cosmosClient.CreateDatabaseIfNotExistsAsync(databaseString); + Console.WriteLine("made database"); + Database database = cosmosClient.GetDatabase(databaseString); + Console.WriteLine(""); + await database.DeleteAsync(); + Console.WriteLine("deleted"); + } private class TestTraceListener : TraceListener { public Action Callback { get; set; } From eae2e3f71e624b25d0667b23a934d8a5fd695aae Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Tue, 20 Sep 2022 16:59:42 -0400 Subject: [PATCH 02/11] Changes behavior to LocationCache and ClientRetryPolicy for metadata write operations --- .../GlobalEndpointManagerTest.cs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs index 0cacd45cbf..28e24d7e55 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs @@ -562,28 +562,6 @@ void TraceHandler(string message) Environment.SetEnvironmentVariable("MinimumIntervalForNonForceRefreshLocationInMS", originalConfigValue); } - [TestMethod] - public async Task MultiMasterRoutingTest() - { - string EndpointUri = "https://ntripician-eastus.documents.azure.com:443/"; - string PrimaryKey = "LpUcGJC29hEyf9BqVFZVlZBGEHGhXMeY5k9BtJwwZY0xl7aXBAsPN5ilalkktiwdceHbKfyAShNoHpsBuPldLA=="; - - CosmosClient cosmosClient = new CosmosClient( - EndpointUri, - PrimaryKey, - new CosmosClientOptions - { - LimitToEndpoint = true - }); - string databaseString = Guid.NewGuid().ToString(); - Console.WriteLine(databaseString); - await cosmosClient.CreateDatabaseIfNotExistsAsync(databaseString); - Console.WriteLine("made database"); - Database database = cosmosClient.GetDatabase(databaseString); - Console.WriteLine(""); - await database.DeleteAsync(); - Console.WriteLine("deleted"); - } private class TestTraceListener : TraceListener { public Action Callback { get; set; } From 7928e6bfa0462bb9bc38aca93e9d301ea2fa76ef Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Fri, 23 Sep 2022 15:03:31 -0400 Subject: [PATCH 03/11] Suggested changes --- .../src/ClientRetryPolicy.cs | 29 +++++++++---------- .../src/Routing/GlobalEndpointManager.cs | 8 ++--- .../src/Routing/LocationCache.cs | 15 +++++----- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs index 25d958965e..3b2b42ca21 100644 --- a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs @@ -139,11 +139,6 @@ public void OnBeforeSendRequest(DocumentServiceRequest request) { // set location-based routing directive based on request retry context request.RequestContext.RouteToLocation(this.retryContext.RetryLocationIndex, this.retryContext.RetryRequestOnPreferredLocations); - - if (!this.canUseMultipleWriteLocations && this.globalEndpointManager.GetTotalAvailableWriteLocations > 1 && !this.isReadRequest) - { - request.RequestContext.IsRetry = true; - } } // Resolve the endpoint for the request and pin the resolution to the resolved endpoint @@ -190,14 +185,18 @@ private async Task ShouldRetryInternalAsync( this.documentServiceRequest?.RequestContext?.LocationEndpointToRoute?.ToString() ?? string.Empty, this.documentServiceRequest?.ResourceAddress ?? string.Empty); - if (!this.canUseMultipleWriteLocations && this.globalEndpointManager.GetTotalAvailableWriteLocations > 1 && !this.isReadRequest) + if (this.globalEndpointManager.IsMetadataWriteRequestMultimaster(this.documentServiceRequest)) { - return await this.ShouldRetryOnEndpointFailureAsync( - isReadRequest: false, - markBothReadAndWriteAsUnavailable: false, - forceRefresh: true, - retryOnPreferredLocations: false, - isMetadataWrite: true); + Task retryResult = this.ShouldRetryOnEndpointFailureAsync( + isReadRequest: false, + markBothReadAndWriteAsUnavailable: false, + forceRefresh: true, + retryOnPreferredLocations: false, + overwriteEndpointDiscovery: true); + + this.retryContext.RetryLocationIndex = 0; + + return await retryResult; } return await this.ShouldRetryOnEndpointFailureAsync( @@ -253,9 +252,9 @@ private async Task ShouldRetryOnEndpointFailureAsync( bool markBothReadAndWriteAsUnavailable, bool forceRefresh, bool retryOnPreferredLocations, - bool isMetadataWrite = false) + bool overwriteEndpointDiscovery = false) { - if (this.failoverRetryCount > MaxRetryCount || (!this.enableEndpointDiscovery && !isMetadataWrite)) + if (this.failoverRetryCount > MaxRetryCount || (!this.enableEndpointDiscovery && !overwriteEndpointDiscovery)) { DefaultTrace.TraceInformation("ClientRetryPolicy: ShouldRetryOnEndpointFailureAsync() Not retrying. Retry count = {0}, Endpoint = {1}", this.failoverRetryCount, @@ -265,7 +264,7 @@ private async Task ShouldRetryOnEndpointFailureAsync( this.failoverRetryCount++; - if (this.locationEndpoint != null) + if (this.locationEndpoint != null && !overwriteEndpointDiscovery) { if (isReadRequest || markBothReadAndWriteAsUnavailable) { diff --git a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs index 9e89e50501..96191966a9 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs @@ -95,10 +95,10 @@ public GlobalEndpointManager(IDocumentClientInternal owner, ConnectionPolicy con public int PreferredLocationCount => this.connectionPolicy.PreferredLocations != null ? this.connectionPolicy.PreferredLocations.Count : 0; - /// - /// Gets total availible write locations from the current location information for a Cosmos account - /// - public int GetTotalAvailableWriteLocations => this.locationCache.GetTotalAvailableWriteLocations; + public bool IsMetadataWriteRequestMultimaster(DocumentServiceRequest request) + { + return this.locationCache.IsMetadataWriteRequestOnMultimasterAccount(request); + } /// /// This will get the account information. diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index 9ecb48257e..8471f71578 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -81,7 +81,7 @@ public LocationCache( /// /// Gets total availible write locations from the current location information for a Cosmos account /// - public int GetTotalAvailableWriteLocations => this.locationInfo.AvailableWriteLocations.Count; + public int TotalAvailableWriteLocations => this.locationInfo.AvailableWriteLocations.Count; /// /// Gets list of read endpoints ordered by /// 1. Preferred location @@ -209,6 +209,11 @@ public void OnLocationPreferenceChanged(ReadOnlyCollection preferredLoca preferenceList: preferredLocations); } + public bool IsMetadataWriteRequestOnMultimasterAccount(DocumentServiceRequest request) + { + return !request.IsReadOnlyRequest && this.TotalAvailableWriteLocations > 1 && !this.CanUseMultipleWriteLocations(request); + } + /// /// Resolves request to service endpoint. /// 1. If this is a write request @@ -246,18 +251,12 @@ public Uri ResolveServiceEndpoint(DocumentServiceRequest request) // first and the second writable region in DatabaseAccount (for manual failover) DatabaseAccountLocationsInfo currentLocationInfo = this.locationInfo; - if (this.enableEndpointDiscovery && currentLocationInfo.AvailableWriteLocations.Count > 0) + if ((this.enableEndpointDiscovery && currentLocationInfo.AvailableWriteLocations.Count > 0) || this.IsMetadataWriteRequestOnMultimasterAccount(request)) { locationIndex = Math.Min(locationIndex % 2, currentLocationInfo.AvailableWriteLocations.Count - 1); string writeLocation = currentLocationInfo.AvailableWriteLocations[locationIndex]; locationEndpointToRoute = currentLocationInfo.AvailableWriteEndpointByLocation[writeLocation]; } - else if (request.RequestContext.IsRetry && currentLocationInfo.AvailableWriteLocations.Count > 1 && request.ResourceType != ResourceType.Document) - { - Console.WriteLine(currentLocationInfo); - string writeLocation = currentLocationInfo.AvailableWriteLocations[0]; //Always want to rout to hub region, the first one in the list of available regions - locationEndpointToRoute = currentLocationInfo.AvailableWriteEndpointByLocation[writeLocation]; - } } else { From 7548c2f62a2a3812f8657a6761a839344d694c5d Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Mon, 26 Sep 2022 17:36:04 -0400 Subject: [PATCH 04/11] fixed added bugs --- .../src/ClientRetryPolicy.cs | 8 +++++++- .../src/Routing/GlobalEndpointManager.cs | 5 +++++ .../src/Routing/LocationCache.cs | 18 ++++++++++++------ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs index 3b2b42ca21..e4b2acf733 100644 --- a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs @@ -34,6 +34,7 @@ internal sealed class ClientRetryPolicy : IDocumentClientRetryPolicy private int serviceUnavailableRetryCount; private bool isReadRequest; private bool canUseMultipleWriteLocations; + private bool retryMetadataMultiMasterWrite403dot3; private Uri locationEndpoint; private RetryContext retryContext; private DocumentServiceRequest documentServiceRequest; @@ -139,6 +140,11 @@ public void OnBeforeSendRequest(DocumentServiceRequest request) { // set location-based routing directive based on request retry context request.RequestContext.RouteToLocation(this.retryContext.RetryLocationIndex, this.retryContext.RetryRequestOnPreferredLocations); + + if (this.retryMetadataMultiMasterWrite403dot3) + { + request.RequestContext.RouteToLocation(this.globalEndpointManager.GetHubUri()); + } } // Resolve the endpoint for the request and pin the resolution to the resolved endpoint @@ -194,7 +200,7 @@ private async Task ShouldRetryInternalAsync( retryOnPreferredLocations: false, overwriteEndpointDiscovery: true); - this.retryContext.RetryLocationIndex = 0; + this.retryMetadataMultiMasterWrite403dot3 = true; return await retryResult; } diff --git a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs index 96191966a9..54f4940980 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs @@ -100,6 +100,11 @@ public bool IsMetadataWriteRequestMultimaster(DocumentServiceRequest request) return this.locationCache.IsMetadataWriteRequestOnMultimasterAccount(request); } + public Uri GetHubUri() + { + return this.locationCache.GetHubUri(); + } + /// /// This will get the account information. /// It will try the global endpoint first. diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index 8471f71578..d78720a578 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -11,6 +11,7 @@ namespace Microsoft.Azure.Cosmos.Routing using System.Linq; using System.Net; using Microsoft.Azure.Cosmos.Core.Trace; + using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Documents; /// @@ -78,10 +79,7 @@ public LocationCache( #endif #endif } - /// - /// Gets total availible write locations from the current location information for a Cosmos account - /// - public int TotalAvailableWriteLocations => this.locationInfo.AvailableWriteLocations.Count; + /// /// Gets list of read endpoints ordered by /// 1. Preferred location @@ -211,7 +209,15 @@ public void OnLocationPreferenceChanged(ReadOnlyCollection preferredLoca public bool IsMetadataWriteRequestOnMultimasterAccount(DocumentServiceRequest request) { - return !request.IsReadOnlyRequest && this.TotalAvailableWriteLocations > 1 && !this.CanUseMultipleWriteLocations(request); + return !request.IsReadOnlyRequest && this.locationInfo.AvailableWriteLocations.Count > 1 && !this.CanUseMultipleWriteLocations(request); + } + + public Uri GetHubUri() + { + DatabaseAccountLocationsInfo currentLocationInfo = this.locationInfo; + string writeLocation = currentLocationInfo.AvailableWriteLocations[0]; + Uri locationEndpointToRoute = currentLocationInfo.AvailableWriteEndpointByLocation[writeLocation]; + return locationEndpointToRoute; } /// @@ -251,7 +257,7 @@ public Uri ResolveServiceEndpoint(DocumentServiceRequest request) // first and the second writable region in DatabaseAccount (for manual failover) DatabaseAccountLocationsInfo currentLocationInfo = this.locationInfo; - if ((this.enableEndpointDiscovery && currentLocationInfo.AvailableWriteLocations.Count > 0) || this.IsMetadataWriteRequestOnMultimasterAccount(request)) + if (this.enableEndpointDiscovery && currentLocationInfo.AvailableWriteLocations.Count > 0) { locationIndex = Math.Min(locationIndex % 2, currentLocationInfo.AvailableWriteLocations.Count - 1); string writeLocation = currentLocationInfo.AvailableWriteLocations[locationIndex]; From 0effda3b449acc93f2fcf20e84de0e810a2c6c5b Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Tue, 27 Sep 2022 10:06:35 -0400 Subject: [PATCH 05/11] Was not checking to see if request was actually metadata request. --- Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index d78720a578..a6c1126e46 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -11,7 +11,6 @@ namespace Microsoft.Azure.Cosmos.Routing using System.Linq; using System.Net; using Microsoft.Azure.Cosmos.Core.Trace; - using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Documents; /// @@ -209,7 +208,8 @@ public void OnLocationPreferenceChanged(ReadOnlyCollection preferredLoca public bool IsMetadataWriteRequestOnMultimasterAccount(DocumentServiceRequest request) { - return !request.IsReadOnlyRequest && this.locationInfo.AvailableWriteLocations.Count > 1 && !this.CanUseMultipleWriteLocations(request); + return !request.IsReadOnlyRequest && this.locationInfo.AvailableWriteLocations.Count > 1 + && !this.CanUseMultipleWriteLocations(request) && request.ResourceType != ResourceType.Document; } public Uri GetHubUri() From 274fd17c48c6be182a7156086b02e7d880ae79a1 Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Fri, 30 Sep 2022 11:18:00 -0400 Subject: [PATCH 06/11] suggested changes and tests --- .../src/ClientRetryPolicy.cs | 6 +- .../src/Routing/LocationCache.cs | 1 + .../ClientRetryPolicyTests.cs | 66 +++++++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs index 7709dd335e..ccff92e917 100644 --- a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs @@ -140,7 +140,7 @@ public void OnBeforeSendRequest(DocumentServiceRequest request) { // set location-based routing directive based on request retry context request.RequestContext.RouteToLocation(this.retryContext.RetryLocationIndex, this.retryContext.RetryRequestOnPreferredLocations); - + if (this.retryMetadataMultiMasterWrite403dot3) { request.RequestContext.RouteToLocation(this.globalEndpointManager.GetHubUri()); @@ -193,7 +193,7 @@ private async Task ShouldRetryInternalAsync( if (this.globalEndpointManager.IsMetadataWriteRequestMultimaster(this.documentServiceRequest)) { - Task retryResult = this.ShouldRetryOnEndpointFailureAsync( + ShouldRetryResult retryResult = await this.ShouldRetryOnEndpointFailureAsync( isReadRequest: false, markBothReadAndWriteAsUnavailable: false, forceRefresh: true, @@ -202,7 +202,7 @@ private async Task ShouldRetryInternalAsync( this.retryMetadataMultiMasterWrite403dot3 = true; - return await retryResult; + return retryResult; } return await this.ShouldRetryOnEndpointFailureAsync( diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index a6c1126e46..874170bbea 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -8,6 +8,7 @@ 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 Microsoft.Azure.Cosmos.Core.Trace; diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs index 45064026fd..d066466d6d 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs @@ -37,6 +37,60 @@ public sealed class ClientRetryPolicyTests private GlobalPartitionEndpointManager partitionKeyRangeLocationCache; private Mock mockedClient; + /// + /// Tests behavior of Multimaster Accounts on metadata writes where the default location is not the hub region + /// + [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.IsMetadataWriteRequestMultimaster(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 nameValueCollection = new Mock(); + + 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 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); + } + /// /// Tests to see if different 503 substatus codes are handeled correctly /// @@ -338,6 +392,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) From bed2d12490edb3c4e50ea57876f40efcc6ba8ede Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Fri, 30 Sep 2022 11:25:39 -0400 Subject: [PATCH 07/11] Cleaning up tests and removing global variable by adding functionality into retryContext --- Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs | 7 ++++--- .../ClientRetryPolicyTests.cs | 11 ++--------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs index ccff92e917..3714dc3e07 100644 --- a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs @@ -34,7 +34,6 @@ internal sealed class ClientRetryPolicy : IDocumentClientRetryPolicy private int serviceUnavailableRetryCount; private bool isReadRequest; private bool canUseMultipleWriteLocations; - private bool retryMetadataMultiMasterWrite403dot3; private Uri locationEndpoint; private RetryContext retryContext; private DocumentServiceRequest documentServiceRequest; @@ -141,7 +140,7 @@ public void OnBeforeSendRequest(DocumentServiceRequest request) // set location-based routing directive based on request retry context request.RequestContext.RouteToLocation(this.retryContext.RetryLocationIndex, this.retryContext.RetryRequestOnPreferredLocations); - if (this.retryMetadataMultiMasterWrite403dot3) + if (this.retryContext.RouteToHub) { request.RequestContext.RouteToLocation(this.globalEndpointManager.GetHubUri()); } @@ -200,7 +199,7 @@ private async Task ShouldRetryInternalAsync( retryOnPreferredLocations: false, overwriteEndpointDiscovery: true); - this.retryMetadataMultiMasterWrite403dot3 = true; + this.retryContext.RouteToHub = true; return retryResult; } @@ -421,6 +420,8 @@ private sealed class RetryContext { public int RetryLocationIndex { get; set; } public bool RetryRequestOnPreferredLocations { get; set; } + + public bool RouteToHub { get; set; } } } } \ No newline at end of file diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs index d066466d6d..dec106a784 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs +++ b/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; From 79f97d25e71dc422468258c35ad5fa57c3746502 Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Fri, 30 Sep 2022 15:15:45 -0400 Subject: [PATCH 08/11] remove unneccary code --- Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index 874170bbea..34f007b57e 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -210,7 +210,7 @@ public void OnLocationPreferenceChanged(ReadOnlyCollection preferredLoca public bool IsMetadataWriteRequestOnMultimasterAccount(DocumentServiceRequest request) { return !request.IsReadOnlyRequest && this.locationInfo.AvailableWriteLocations.Count > 1 - && !this.CanUseMultipleWriteLocations(request) && request.ResourceType != ResourceType.Document; + && !this.CanUseMultipleWriteLocations(request); } public Uri GetHubUri() From db4c4ffe03abbb85f59a950a5936b7bb17b2fa6e Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Fri, 30 Sep 2022 16:24:53 -0400 Subject: [PATCH 09/11] fixed IsMetadataWriteREquestOnMultimasterAccount --- Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index 34f007b57e..e4c00c46c9 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -11,6 +11,7 @@ namespace Microsoft.Azure.Cosmos.Routing using System.Globalization; using System.Linq; using System.Net; + using global::Azure.Core; using Microsoft.Azure.Cosmos.Core.Trace; using Microsoft.Azure.Documents; @@ -210,7 +211,8 @@ public void OnLocationPreferenceChanged(ReadOnlyCollection preferredLoca public bool IsMetadataWriteRequestOnMultimasterAccount(DocumentServiceRequest request) { return !request.IsReadOnlyRequest && this.locationInfo.AvailableWriteLocations.Count > 1 - && !this.CanUseMultipleWriteLocations(request); + && request.ResourceType != ResourceType.Document && request.ResourceType != ResourceType.StoredProcedure + && request.OperationType != Documents.OperationType.ExecuteJavaScript && this.CanUseMultipleWriteLocations(); } public Uri GetHubUri() From 2ddf047b9a24a65940ac001e0991398441380eef Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Fri, 30 Sep 2022 16:34:53 -0400 Subject: [PATCH 10/11] added missed case to IsMetadataWriteREquestOnMultimasterAccount --- Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index e4c00c46c9..e11163984f 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -211,8 +211,10 @@ public void OnLocationPreferenceChanged(ReadOnlyCollection preferredLoca public bool IsMetadataWriteRequestOnMultimasterAccount(DocumentServiceRequest request) { return !request.IsReadOnlyRequest && this.locationInfo.AvailableWriteLocations.Count > 1 - && request.ResourceType != ResourceType.Document && request.ResourceType != ResourceType.StoredProcedure - && request.OperationType != Documents.OperationType.ExecuteJavaScript && this.CanUseMultipleWriteLocations(); + && (request.ResourceType != ResourceType.Document || + (request.OperationType != Documents.OperationType.ExecuteJavaScript && request.ResourceType == ResourceType.StoredProcedure)) + && this.CanUseMultipleWriteLocations(); + } public Uri GetHubUri() From de2228f770fea7cd0f29aa4ccda9f9cb213bfc2a Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Mon, 3 Oct 2022 14:51:27 -0400 Subject: [PATCH 11/11] suggested changes --- .../src/ClientRetryPolicy.cs | 27 ++++++++++++++----- .../src/Routing/GlobalEndpointManager.cs | 4 +-- .../src/Routing/LocationCache.cs | 13 ++++++--- .../ClientRetryPolicyTests.cs | 2 +- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs index 3714dc3e07..1f73aff4ea 100644 --- a/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs @@ -137,13 +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 @@ -190,17 +192,28 @@ private async Task ShouldRetryInternalAsync( this.documentServiceRequest?.RequestContext?.LocationEndpointToRoute?.ToString() ?? string.Empty, this.documentServiceRequest?.ResourceAddress ?? string.Empty); - if (this.globalEndpointManager.IsMetadataWriteRequestMultimaster(this.documentServiceRequest)) + 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: true, + forceRefresh: forceRefresh, retryOnPreferredLocations: false, overwriteEndpointDiscovery: true); - this.retryContext.RouteToHub = true; - + if (retryResult.ShouldRetry) + { + this.retryContext.RouteToHub = true; + } + return retryResult; } diff --git a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs index 54f4940980..956212d5a2 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs @@ -95,9 +95,9 @@ public GlobalEndpointManager(IDocumentClientInternal owner, ConnectionPolicy con public int PreferredLocationCount => this.connectionPolicy.PreferredLocations != null ? this.connectionPolicy.PreferredLocations.Count : 0; - public bool IsMetadataWriteRequestMultimaster(DocumentServiceRequest request) + public bool IsMultimasterMetadataWriteRequest(DocumentServiceRequest request) { - return this.locationCache.IsMetadataWriteRequestOnMultimasterAccount(request); + return this.locationCache.IsMultimasterMetadataWriteRequest(request); } public Uri GetHubUri() diff --git a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs index e11163984f..9c6308d8b6 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/LocationCache.cs @@ -208,11 +208,16 @@ public void OnLocationPreferenceChanged(ReadOnlyCollection preferredLoca preferenceList: preferredLocations); } - public bool IsMetadataWriteRequestOnMultimasterAccount(DocumentServiceRequest request) + public bool IsMetaData(DocumentServiceRequest request) { - return !request.IsReadOnlyRequest && this.locationInfo.AvailableWriteLocations.Count > 1 - && (request.ResourceType != ResourceType.Document || - (request.OperationType != Documents.OperationType.ExecuteJavaScript && request.ResourceType == ResourceType.StoredProcedure)) + 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(); } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs index dec106a784..7faeba2a35 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ClientRetryPolicyTests.cs @@ -52,7 +52,7 @@ public void MultimasterMetadataWriteRetryTest() //Creates a metadata write request DocumentServiceRequest request = this.CreateRequest(false, true); - Assert.IsTrue(endpointManager.IsMetadataWriteRequestMultimaster(request)); + Assert.IsTrue(endpointManager.IsMultimasterMetadataWriteRequest(request)); //On first attempt should get incorrect (default/non hub) location retryPolicy.OnBeforeSendRequest(request);