From 04da68e84752b4ed542d5cbd47152dfea9e0115a Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Mon, 3 Jan 2022 15:16:43 -0800 Subject: [PATCH 1/5] Don't send token --- Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs index daa85a46c2..e4ea632f15 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs @@ -265,10 +265,11 @@ internal static async Task ApplySessionTokenAsync( } string requestConsistencyLevel = request.Headers[HttpConstants.HttpHeaders.ConsistencyLevel]; + bool requestHasConsistencySet = !string.IsNullOrEmpty(requestConsistencyLevel); bool sessionConsistency = - defaultConsistencyLevel == ConsistencyLevel.Session || - (!string.IsNullOrEmpty(requestConsistencyLevel) + (!requestHasConsistencySet && defaultConsistencyLevel == ConsistencyLevel.Session) || + (requestHasConsistencySet && string.Equals(requestConsistencyLevel, GatewayStoreModel.sessionConsistencyAsString, StringComparison.OrdinalIgnoreCase)); bool isMultiMasterEnabledForRequest = globalEndpointManager.CanUseMultipleWriteLocations(request); From 6744540e73f85ddfc2a89386cd2da69e5c056ede Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Mon, 3 Jan 2022 15:16:47 -0800 Subject: [PATCH 2/5] test --- .../GatewayStoreModelTest.cs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs index d70d9d2501..0f5d53ffd9 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs @@ -414,6 +414,39 @@ await GatewayStoreModel.ApplySessionTokenAsync( }); } + [TestMethod] + public async Task TestRequestOverloadRemovesSessionToken() + { + INameValueCollection headers = new StoreRequestNameValueCollection(); + headers.Set(HttpConstants.HttpHeaders.ConsistencyLevel, ConsistencyLevel.Eventual.ToString()); + + DocumentServiceRequest dsrNoSessionToken = DocumentServiceRequest.CreateFromName( + OperationType.Read, + "Test", + ResourceType.Document, + AuthorizationTokenType.PrimaryMasterKey, + headers); + + string dsrSessionToken = Guid.NewGuid().ToString(); + Mock sMock = new Mock(); + sMock.Setup(x => x.ResolveGlobalSessionToken(dsrNoSessionToken)).Returns(dsrSessionToken); + + Mock globalEndpointManager = new Mock(); + await this.GetGatewayStoreModelForConsistencyTest(async (gatewayStoreModel) => + { + await GatewayStoreModel.ApplySessionTokenAsync( + dsrNoSessionToken, + ConsistencyLevel.Session, + sMock.Object, + partitionKeyRangeCache: new Mock(null, null, null).Object, + clientCollectionCache: new Mock(new SessionContainer("testhost"), gatewayStoreModel, null, null).Object, + globalEndpointManager: new Mock().Object); + + // Should not add the session token because the request is lower + Assert.IsNull(dsrNoSessionToken.Headers[HttpConstants.HttpHeaders.SessionToken]); + }); + } + [TestMethod] public async Task TestErrorResponsesProvideBody() { From 85049c49b37af18c83347d75b1f6edc175e03df7 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Tue, 4 Jan 2022 09:05:54 -0800 Subject: [PATCH 3/5] Adding more tests --- .../GatewayStoreModelTest.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs index 0f5d53ffd9..9141eb5b6e 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs @@ -414,14 +414,18 @@ await GatewayStoreModel.ApplySessionTokenAsync( }); } - [TestMethod] - public async Task TestRequestOverloadRemovesSessionToken() + [DataTestMethod] + [DataRow(false, false, DisplayName = "Single master - Read")] + [DataRow(true, false, DisplayName = "Multi master - Read")] + [DataRow(false, true, DisplayName = "Single master - Write")] + [DataRow(true, true, DisplayName = "Multi master - Write")] + public async Task TestRequestOverloadRemovesSessionToken(bool multiMaster, bool isWriteRequest) { INameValueCollection headers = new StoreRequestNameValueCollection(); headers.Set(HttpConstants.HttpHeaders.ConsistencyLevel, ConsistencyLevel.Eventual.ToString()); DocumentServiceRequest dsrNoSessionToken = DocumentServiceRequest.CreateFromName( - OperationType.Read, + isWriteRequest ? OperationType.Create : OperationType.Read, "Test", ResourceType.Document, AuthorizationTokenType.PrimaryMasterKey, @@ -432,6 +436,7 @@ public async Task TestRequestOverloadRemovesSessionToken() sMock.Setup(x => x.ResolveGlobalSessionToken(dsrNoSessionToken)).Returns(dsrSessionToken); Mock globalEndpointManager = new Mock(); + globalEndpointManager.Setup(gem => gem.CanUseMultipleWriteLocations(It.Is(drs => drs == dsrNoSessionToken))).Returns(multiMaster); await this.GetGatewayStoreModelForConsistencyTest(async (gatewayStoreModel) => { await GatewayStoreModel.ApplySessionTokenAsync( From 99dafe6f486e4c5d15cfdc5c1e1c283cdaf54be1 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Wed, 5 Jan 2022 14:06:46 -0800 Subject: [PATCH 4/5] Fixing multimaster writes --- Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs | 12 ++++++------ .../GatewayStoreModelTest.cs | 13 ++++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs index e4ea632f15..19835deadc 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs @@ -265,18 +265,18 @@ internal static async Task ApplySessionTokenAsync( } string requestConsistencyLevel = request.Headers[HttpConstants.HttpHeaders.ConsistencyLevel]; - bool requestHasConsistencySet = !string.IsNullOrEmpty(requestConsistencyLevel); - - bool sessionConsistency = + bool isConsideredAReadRequest = request.IsReadOnlyRequest || request.OperationType == OperationType.Batch; + bool requestHasConsistencySet = !string.IsNullOrEmpty(requestConsistencyLevel) && isConsideredAReadRequest; // Only read requests can have their consistency modified + + bool sessionConsistencyApplies = (!requestHasConsistencySet && defaultConsistencyLevel == ConsistencyLevel.Session) || (requestHasConsistencySet && string.Equals(requestConsistencyLevel, GatewayStoreModel.sessionConsistencyAsString, StringComparison.OrdinalIgnoreCase)); bool isMultiMasterEnabledForRequest = globalEndpointManager.CanUseMultipleWriteLocations(request); - if (!sessionConsistency - || (!request.IsReadOnlyRequest - && request.OperationType != OperationType.Batch + if (!sessionConsistencyApplies + || (!isConsideredAReadRequest && !isMultiMasterEnabledForRequest)) { return; // Only apply the session token in case of session consistency and the request is read only or read/write on multimaster diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs index 9141eb5b6e..d832c70cbe 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreModelTest.cs @@ -445,10 +445,17 @@ await GatewayStoreModel.ApplySessionTokenAsync( sMock.Object, partitionKeyRangeCache: new Mock(null, null, null).Object, clientCollectionCache: new Mock(new SessionContainer("testhost"), gatewayStoreModel, null, null).Object, - globalEndpointManager: new Mock().Object); + globalEndpointManager: globalEndpointManager.Object); - // Should not add the session token because the request is lower - Assert.IsNull(dsrNoSessionToken.Headers[HttpConstants.HttpHeaders.SessionToken]); + if (isWriteRequest && multiMaster) + { + // Multi master write requests should not lower the consistency and remove the session token + Assert.AreEqual(dsrSessionToken, dsrNoSessionToken.Headers[HttpConstants.HttpHeaders.SessionToken]); + } + else + { + Assert.IsNull(dsrNoSessionToken.Headers[HttpConstants.HttpHeaders.SessionToken]); + } }); } From 380f8c57c10489206ae0d1f0735687e21e126635 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Wed, 5 Jan 2022 14:35:47 -0800 Subject: [PATCH 5/5] renamed variable --- Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs index 19835deadc..d5623e9a8e 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs @@ -265,8 +265,8 @@ internal static async Task ApplySessionTokenAsync( } string requestConsistencyLevel = request.Headers[HttpConstants.HttpHeaders.ConsistencyLevel]; - bool isConsideredAReadRequest = request.IsReadOnlyRequest || request.OperationType == OperationType.Batch; - bool requestHasConsistencySet = !string.IsNullOrEmpty(requestConsistencyLevel) && isConsideredAReadRequest; // Only read requests can have their consistency modified + bool isReadOrBatchRequest = request.IsReadOnlyRequest || request.OperationType == OperationType.Batch; + bool requestHasConsistencySet = !string.IsNullOrEmpty(requestConsistencyLevel) && isReadOrBatchRequest; // Only read requests can have their consistency modified bool sessionConsistencyApplies = (!requestHasConsistencySet && defaultConsistencyLevel == ConsistencyLevel.Session) || @@ -276,7 +276,7 @@ internal static async Task ApplySessionTokenAsync( bool isMultiMasterEnabledForRequest = globalEndpointManager.CanUseMultipleWriteLocations(request); if (!sessionConsistencyApplies - || (!isConsideredAReadRequest + || (!isReadOrBatchRequest && !isMultiMasterEnabledForRequest)) { return; // Only apply the session token in case of session consistency and the request is read only or read/write on multimaster