From 2469f315e672e6b261ce9d1b76904cd075c589b6 Mon Sep 17 00:00:00 2001 From: Debdatta Kunda Date: Tue, 30 Aug 2022 13:33:08 -0700 Subject: [PATCH 1/3] Code changes to fix duplicate trace key generation during PartitionKeyRangeCache Computation. --- .../src/Routing/PartitionKeyRangeCache.cs | 2 +- .../PartitionKeyRangeCacheTest.cs | 119 ++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs diff --git a/Microsoft.Azure.Cosmos/src/Routing/PartitionKeyRangeCache.cs b/Microsoft.Azure.Cosmos/src/Routing/PartitionKeyRangeCache.cs index a617d5efe3..4373d61f56 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/PartitionKeyRangeCache.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/PartitionKeyRangeCache.cs @@ -264,7 +264,7 @@ private async Task GetRoutingMapForCollectionAsync( throw new NotFoundException($"{DateTime.UtcNow.ToString("o", CultureInfo.InvariantCulture)}: GetRoutingMapForCollectionAsync(collectionRid: {collectionRid}), Range information either doesn't exist or is not complete."); } - trace.AddDatum($"PKRangeCache Info({DateTime.UtcNow.ToString("o", CultureInfo.InvariantCulture)})", + trace.AddDatum($"PKRangeCache Info({previousRoutingMap?.ChangeFeedNextIfNoneMatch}#{DateTime.UtcNow.ToString("o", CultureInfo.InvariantCulture)})", new PartitionKeyRangeCacheTraceDatum( previousContinuationToken: previousRoutingMap?.ChangeFeedNextIfNoneMatch, continuationToken: routingMap.ChangeFeedNextIfNoneMatch)); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs new file mode 100644 index 0000000000..344a7d6daa --- /dev/null +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs @@ -0,0 +1,119 @@ +//------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +//------------------------------------------------------------ + +namespace Microsoft.Azure.Cosmos.Tests +{ + using System.IO; + using System.Net; + using System.Text; + using System.Threading; + using System.Threading.Tasks; + using System.Collections.Generic; + using Microsoft.Azure.Cosmos.Common; + using Microsoft.Azure.Cosmos.Routing; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using Moq; + using Microsoft.Azure.Documents; + using Microsoft.Azure.Documents.Collections; + using Microsoft.Azure.Cosmos.Tracing; + using Microsoft.Azure.Cosmos.Diagnostics; + using TraceLevel = Cosmos.Tracing.TraceLevel; + using Newtonsoft.Json.Linq; + using Newtonsoft.Json; + + /// + /// Unit Tests for . + /// + [TestClass] + public class PartitionKeyRangeCacheTest + { + /// + /// Gets and Sets the current . + /// + public TestContext TestContext { get; set; } + + /// + /// Initializes the test class with the current test context. + /// + [TestInitialize] + public void Initialize() + { + } + + /// + /// Test to validate that when a fresh container is created, TryGetOverlappingRangesAsync() should not + /// add same key to the trace dictionary. The same key could be regenerated, if the CPU computation is + /// fast enough to invoke TryLookupAsync within .5 to 15 ms timeframe, which may lead to get the exact + /// same UTC DateTime, which is nothing but the trace key. + /// + /// + /// A object indicating the test is finished. + /// + [TestMethod] + public async Task TryGetOverlappingRangesAsync_WithFreshContainer_ShouldNotAddSameTraceKey() + { + // Arrange. + string eTag = "483"; + string authToken = "token!"; + string containerRId = "kjhsAA=="; + string singlePkCollectionCache = "{\"_rid\":\"3FIlAOzjvyg=\",\"PartitionKeyRanges\":[{\"_rid\":\"3FIlAOzjvygCAAAAAAAAUA==\",\"id\":\"0\",\"_etag\":\"\\\"00005565-0000-0800-0000-621fd98a0000\\\"\",\"minInclusive\":\"\",\"maxExclusive\":\"FF\",\"ridPrefix\":0,\"_self\":\"dbs/3FIlAA==/colls/3FIlAOzjvyg=/pkranges/3FIlAOzjvygCAAAAAAAAUA==/\",\"throughputFraction\":1,\"status\":\"splitting\",\"parents\":[],\"_ts\":1646254474,\"_lsn\":44}],\"_count\":1}"; + byte[] singlePkCollectionCacheByte = Encoding.UTF8.GetBytes(singlePkCollectionCache); + ITrace trace = Trace.GetRootTrace(this.TestContext.TestName, TraceComponent.Unknown, TraceLevel.Info); + + Mock mockStoreModel = new(); + Mock mockCollectioNCache = new(); + Mock mockTokenProvider = new(); + NameValueCollectionWrapper headers = new() + { + [HttpConstants.HttpHeaders.ETag] = eTag, + }; + + mockStoreModel.SetupSequence(x => x.ProcessMessageAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new DocumentServiceResponse(new MemoryStream(singlePkCollectionCacheByte), + new StoreResponseNameValueCollection() + { + ETag = eTag, + }, + HttpStatusCode.OK)) + .ReturnsAsync(new DocumentServiceResponse(null, headers, HttpStatusCode.NotModified, null)) + .ReturnsAsync(new DocumentServiceResponse(null, headers, HttpStatusCode.NotModified, null)); + + mockTokenProvider.Setup(x => x.GetUserAuthorizationTokenAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(new ValueTask(authToken)); + + // Act. + PartitionKeyRangeCache partitionKeyRangeCache = new (mockTokenProvider.Object, mockStoreModel.Object, mockCollectioNCache.Object); + IReadOnlyList partitionKeyRanges = await partitionKeyRangeCache.TryGetOverlappingRangesAsync( + containerRId, + FeedRangeEpk.FullRange.Range, + trace, + forceRefresh: true); + + // Assert. + string dataPath = "children[0].data", previousContinuationTokenKey = "['Previous Continuation Token']", diagnostics = new CosmosTraceDiagnostics(trace).ToString(); + JObject traceData = JsonConvert.DeserializeObject(diagnostics); + + Assert.IsNotNull(traceData); + + string firstPkRangeCacheKeyName = ((JProperty)traceData.SelectToken(dataPath)?.First).Name; + string secondPkRangeCacheKeyName = ((JProperty)traceData.SelectToken(dataPath)?.Last).Name; + + Assert.IsNotNull(firstPkRangeCacheKeyName); + Assert.IsNotNull(secondPkRangeCacheKeyName); + Assert.IsTrue(firstPkRangeCacheKeyName.Length > 0); + Assert.IsTrue(secondPkRangeCacheKeyName.Length > 0); + Assert.AreNotEqual(firstPkRangeCacheKeyName, secondPkRangeCacheKeyName); + + string firstPkRangeValue = ((JProperty)traceData.SelectToken(dataPath).First)?.Value?.SelectToken(previousContinuationTokenKey)?.ToString(); + string secondPkRangeValue = ((JProperty)traceData.SelectToken(dataPath).Last)?.Value?.SelectToken(previousContinuationTokenKey)?.ToString(); + + Assert.IsNotNull(firstPkRangeValue); + Assert.IsNotNull(secondPkRangeValue); + Assert.IsTrue(firstPkRangeValue.Length == 0); + Assert.AreEqual(eTag, secondPkRangeValue); + + trace.Dispose(); + } + } +} From 575681bb55a16c3fdf6ba89e0d3284090a5426b5 Mon Sep 17 00:00:00 2001 From: Debdatta Kunda Date: Tue, 30 Aug 2022 15:56:07 -0700 Subject: [PATCH 2/3] Code changes to address review comments. --- .../PartitionKeyRangeCacheTest.cs | 104 ++++++++++-------- 1 file changed, 58 insertions(+), 46 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs index 344a7d6daa..de3da9f650 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs @@ -45,7 +45,23 @@ public void Initialize() /// Test to validate that when a fresh container is created, TryGetOverlappingRangesAsync() should not /// add same key to the trace dictionary. The same key could be regenerated, if the CPU computation is /// fast enough to invoke TryLookupAsync within .5 to 15 ms timeframe, which may lead to get the exact - /// same UTC DateTime, which is nothing but the trace key. + /// same UTC DateTime, which is nothing but the trace key. An example trace JSON with the PKRange Info + /// nodes can be found below: + /// + /// "name": "Try Get Overlapping Ranges", + /// "id": "9d28a8cb-9669-422b-9619-b91cbbf61dff", + /// "start time": "06:16:03:990", + /// "duration in milliseconds": 50.8808, + /// "data": { + /// "PKRangeCache Info(#2022-08-30T18:16:04.0364155Z)": { + /// "Previous Continuation Token": null, + /// "Continuation Token": "483" + /// }, + /// "PKRangeCache Info(483#2022-08-30T18:16:04.0394322Z)": { + /// "Previous Continuation Token": "483", + /// "Continuation Token": "483" + /// } + /// }, /// /// /// A object indicating the test is finished. @@ -59,61 +75,57 @@ public async Task TryGetOverlappingRangesAsync_WithFreshContainer_ShouldNotAddSa string containerRId = "kjhsAA=="; string singlePkCollectionCache = "{\"_rid\":\"3FIlAOzjvyg=\",\"PartitionKeyRanges\":[{\"_rid\":\"3FIlAOzjvygCAAAAAAAAUA==\",\"id\":\"0\",\"_etag\":\"\\\"00005565-0000-0800-0000-621fd98a0000\\\"\",\"minInclusive\":\"\",\"maxExclusive\":\"FF\",\"ridPrefix\":0,\"_self\":\"dbs/3FIlAA==/colls/3FIlAOzjvyg=/pkranges/3FIlAOzjvygCAAAAAAAAUA==/\",\"throughputFraction\":1,\"status\":\"splitting\",\"parents\":[],\"_ts\":1646254474,\"_lsn\":44}],\"_count\":1}"; byte[] singlePkCollectionCacheByte = Encoding.UTF8.GetBytes(singlePkCollectionCache); - ITrace trace = Trace.GetRootTrace(this.TestContext.TestName, TraceComponent.Unknown, TraceLevel.Info); - - Mock mockStoreModel = new(); - Mock mockCollectioNCache = new(); - Mock mockTokenProvider = new(); - NameValueCollectionWrapper headers = new() + using (ITrace trace = Trace.GetRootTrace(this.TestContext.TestName, TraceComponent.Unknown, TraceLevel.Info)) { - [HttpConstants.HttpHeaders.ETag] = eTag, - }; - - mockStoreModel.SetupSequence(x => x.ProcessMessageAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync(new DocumentServiceResponse(new MemoryStream(singlePkCollectionCacheByte), - new StoreResponseNameValueCollection() - { - ETag = eTag, - }, - HttpStatusCode.OK)) - .ReturnsAsync(new DocumentServiceResponse(null, headers, HttpStatusCode.NotModified, null)) - .ReturnsAsync(new DocumentServiceResponse(null, headers, HttpStatusCode.NotModified, null)); + Mock mockStoreModel = new(); + Mock mockCollectioNCache = new(); + Mock mockTokenProvider = new(); + NameValueCollectionWrapper headers = new() + { + [HttpConstants.HttpHeaders.ETag] = eTag, + }; - mockTokenProvider.Setup(x => x.GetUserAuthorizationTokenAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(new ValueTask(authToken)); + mockStoreModel.SetupSequence(x => x.ProcessMessageAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new DocumentServiceResponse(new MemoryStream(singlePkCollectionCacheByte), + new StoreResponseNameValueCollection() + { + ETag = eTag, + }, + HttpStatusCode.OK)) + .ReturnsAsync(new DocumentServiceResponse(null, headers, HttpStatusCode.NotModified, null)) + .ReturnsAsync(new DocumentServiceResponse(null, headers, HttpStatusCode.NotModified, null)); - // Act. - PartitionKeyRangeCache partitionKeyRangeCache = new (mockTokenProvider.Object, mockStoreModel.Object, mockCollectioNCache.Object); - IReadOnlyList partitionKeyRanges = await partitionKeyRangeCache.TryGetOverlappingRangesAsync( - containerRId, - FeedRangeEpk.FullRange.Range, - trace, - forceRefresh: true); + mockTokenProvider.Setup(x => x.GetUserAuthorizationTokenAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(new ValueTask(authToken)); - // Assert. - string dataPath = "children[0].data", previousContinuationTokenKey = "['Previous Continuation Token']", diagnostics = new CosmosTraceDiagnostics(trace).ToString(); - JObject traceData = JsonConvert.DeserializeObject(diagnostics); + // Act. + PartitionKeyRangeCache partitionKeyRangeCache = new(mockTokenProvider.Object, mockStoreModel.Object, mockCollectioNCache.Object); + IReadOnlyList partitionKeyRanges = await partitionKeyRangeCache.TryGetOverlappingRangesAsync( + containerRId, + FeedRangeEpk.FullRange.Range, + trace, + forceRefresh: true); - Assert.IsNotNull(traceData); + // Assert. + string dataPath = "children[0].data", previousContinuationTokenKey = "['Previous Continuation Token']", diagnostics = new CosmosTraceDiagnostics(trace).ToString(); + JObject traceData = JsonConvert.DeserializeObject(diagnostics); - string firstPkRangeCacheKeyName = ((JProperty)traceData.SelectToken(dataPath)?.First).Name; - string secondPkRangeCacheKeyName = ((JProperty)traceData.SelectToken(dataPath)?.Last).Name; + Assert.IsNotNull(traceData); - Assert.IsNotNull(firstPkRangeCacheKeyName); - Assert.IsNotNull(secondPkRangeCacheKeyName); - Assert.IsTrue(firstPkRangeCacheKeyName.Length > 0); - Assert.IsTrue(secondPkRangeCacheKeyName.Length > 0); - Assert.AreNotEqual(firstPkRangeCacheKeyName, secondPkRangeCacheKeyName); + string firstPkRangeCacheKeyName = ((JProperty)traceData.SelectToken(dataPath)?.First).Name; + string secondPkRangeCacheKeyName = ((JProperty)traceData.SelectToken(dataPath)?.Last).Name; - string firstPkRangeValue = ((JProperty)traceData.SelectToken(dataPath).First)?.Value?.SelectToken(previousContinuationTokenKey)?.ToString(); - string secondPkRangeValue = ((JProperty)traceData.SelectToken(dataPath).Last)?.Value?.SelectToken(previousContinuationTokenKey)?.ToString(); + Assert.IsTrue(!string.IsNullOrWhiteSpace(firstPkRangeCacheKeyName)); + Assert.IsTrue(!string.IsNullOrWhiteSpace(secondPkRangeCacheKeyName)); + Assert.AreNotEqual(firstPkRangeCacheKeyName, secondPkRangeCacheKeyName); - Assert.IsNotNull(firstPkRangeValue); - Assert.IsNotNull(secondPkRangeValue); - Assert.IsTrue(firstPkRangeValue.Length == 0); - Assert.AreEqual(eTag, secondPkRangeValue); + string firstPkRangeValue = ((JProperty)traceData.SelectToken(dataPath).First)?.Value?.SelectToken(previousContinuationTokenKey)?.ToString(); + string secondPkRangeValue = ((JProperty)traceData.SelectToken(dataPath).Last)?.Value?.SelectToken(previousContinuationTokenKey)?.ToString(); - trace.Dispose(); + Assert.IsTrue(!string.IsNullOrWhiteSpace(secondPkRangeValue)); + Assert.IsTrue(string.IsNullOrEmpty(firstPkRangeValue)); + Assert.AreEqual(eTag, secondPkRangeValue); + } } } } From 8d703362b14f9f9c21d1ba0a24927f4490825ad7 Mon Sep 17 00:00:00 2001 From: Debdatta Kunda Date: Wed, 31 Aug 2022 09:43:33 -0700 Subject: [PATCH 3/3] Code changes to remove TestInitialize annotation. --- .../PartitionKeyRangeCacheTest.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs index de3da9f650..8d7b32a7b7 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeCacheTest.cs @@ -33,14 +33,6 @@ public class PartitionKeyRangeCacheTest /// public TestContext TestContext { get; set; } - /// - /// Initializes the test class with the current test context. - /// - [TestInitialize] - public void Initialize() - { - } - /// /// Test to validate that when a fresh container is created, TryGetOverlappingRangesAsync() should not /// add same key to the trace dictionary. The same key could be regenerated, if the CPU computation is