From 6ecb151c7cad9832758344273b05ef95eff2fd7d Mon Sep 17 00:00:00 2001 From: Jake Willey Date: Tue, 29 Dec 2020 12:05:28 -0800 Subject: [PATCH 1/2] Fix diagnostics item count exception --- .../CosmosDiagnosticsContextCore.cs | 2 +- .../CosmosDiagnosticsSerializerVisitor.cs | 14 +++++++++++ .../Query/v3Query/CosmosQueryContextCore.cs | 1 + .../CosmosDiagnosticsTests.cs | 15 ++++++------ .../Utils/DiagnosticValidators.cs | 11 ++++++--- .../CosmosDiagnosticsUnitTests.cs | 24 +++++++++++++++++++ 6 files changed, 56 insertions(+), 11 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsContextCore.cs b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsContextCore.cs index eaeb74238c..e0fba40012 100644 --- a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsContextCore.cs +++ b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsContextCore.cs @@ -182,7 +182,7 @@ internal override void AddDiagnosticsInternal(CosmosDiagnosticsContext newContex { this.AddSummaryInfo(newContext); - this.ContextList.AddRange(newContext); + this.ContextList.Add(newContext); } public override void Accept(CosmosDiagnosticsInternalVisitor cosmosDiagnosticsInternalVisitor) diff --git a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsSerializerVisitor.cs b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsSerializerVisitor.cs index 6d6a4b9780..f036c5a907 100644 --- a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsSerializerVisitor.cs +++ b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsSerializerVisitor.cs @@ -16,6 +16,7 @@ internal sealed class CosmosDiagnosticsSerializerVisitor : CosmosDiagnosticsInte { private const string DiagnosticsVersion = "2"; private readonly JsonWriter jsonWriter; + private bool rootDiagnosticContextVisited = false; public CosmosDiagnosticsSerializerVisitor(TextWriter textWriter) { @@ -63,6 +64,19 @@ public override void Visit(PointOperationStatistics pointOperationStatistics) public override void Visit(CosmosDiagnosticsContext cosmosDiagnosticsContext) { + // Nested diagnostics should not include the summary + if (this.rootDiagnosticContextVisited) + { + foreach (CosmosDiagnosticsInternal cosmosDiagnosticsInternal in cosmosDiagnosticsContext) + { + cosmosDiagnosticsInternal.Accept(this); + } + + return; + } + + this.rootDiagnosticContextVisited = true; + this.jsonWriter.WriteStartObject(); this.jsonWriter.WritePropertyName("DiagnosticVersion"); diff --git a/Microsoft.Azure.Cosmos/src/Query/v3Query/CosmosQueryContextCore.cs b/Microsoft.Azure.Cosmos/src/Query/v3Query/CosmosQueryContextCore.cs index bee3d841e0..d6e44a4bd6 100644 --- a/Microsoft.Azure.Cosmos/src/Query/v3Query/CosmosQueryContextCore.cs +++ b/Microsoft.Azure.Cosmos/src/Query/v3Query/CosmosQueryContextCore.cs @@ -57,6 +57,7 @@ internal CosmosDiagnosticsContext GetAndResetDiagnostics() { CosmosDiagnosticsContext current = this.diagnosticsContext; this.diagnosticsContext = CosmosDiagnosticsContext.Create(new RequestOptions()); + current.GetOverallScope().Dispose(); return current; } } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs index 2c6ede2425..2eb538681d 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs @@ -40,6 +40,7 @@ public async Task TestInitialize() this.containerSettings = new ContainerProperties(id: Guid.NewGuid().ToString(), partitionKeyPath: PartitionKey); ContainerResponse response = await this.database.CreateContainerAsync( this.containerSettings, + throughput: 20000, cancellationToken: this.cancellationToken); Assert.IsNotNull(response); Assert.IsNotNull(response.Container); @@ -510,7 +511,7 @@ public async Task BulkOperationDiagnostic() [TestMethod] public async Task GatewayQueryPlanDiagnostic() { - int totalItems = 3; + int totalItems = 10; IList itemList = await ToDoActivity.CreateRandomItems( this.Container, pkCount: totalItems, @@ -552,7 +553,7 @@ public async Task GatewayQueryPlanDiagnostic() } [TestMethod] - [DataRow(true)] + // [DataRow(true)] [DataRow(false)] public async Task QueryOperationDiagnostic(bool disableDiagnostics) { @@ -563,12 +564,12 @@ public async Task QueryOperationDiagnostic(bool disableDiagnostics) perPKItemCount: 1, randomPartitionKey: true); - long readFeedTotalOutputDocumentCount = await this.ExecuteQueryAndReturnOutputDocumentCount( - queryText: null, - expectedItemCount: totalItems, - disableDiagnostics: disableDiagnostics); + //long readFeedTotalOutputDocumentCount = await this.ExecuteQueryAndReturnOutputDocumentCount( + // queryText: null, + // expectedItemCount: totalItems, + // disableDiagnostics: disableDiagnostics); - Assert.AreEqual(totalItems, readFeedTotalOutputDocumentCount); + //Assert.AreEqual(totalItems, readFeedTotalOutputDocumentCount); //Checking query metrics on typed query long totalOutputDocumentCount = await this.ExecuteQueryAndReturnOutputDocumentCount( diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/DiagnosticValidators.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/DiagnosticValidators.cs index 1d0356ece5..73e9c71f75 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/DiagnosticValidators.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/DiagnosticValidators.cs @@ -379,12 +379,17 @@ public override void Visit(PointOperationStatistics pointOperationStatistics) public override void Visit(CosmosDiagnosticsContext cosmosDiagnosticsContext) { - Assert.IsFalse(this.isContextVisited, "Point operations should only have a single context"); - this.isContextVisited = true; this.StartTimeUtc = cosmosDiagnosticsContext.StartUtc; this.TotalElapsedTime = cosmosDiagnosticsContext.GetRunningElapsedTime(); - DiagnosticValidator.ValidateCosmosDiagnosticsContext(cosmosDiagnosticsContext); + // Only the first context needs to be validated. Inner contexts that were appended only + // require the content to validated. + if (!this.isContextVisited) + { + DiagnosticValidator.ValidateCosmosDiagnosticsContext(cosmosDiagnosticsContext); + } + + this.isContextVisited = true; foreach (CosmosDiagnosticsInternal diagnosticsInternal in cosmosDiagnosticsContext) { diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs index bca29127b9..ec8e078b76 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs @@ -5,6 +5,7 @@ namespace Microsoft.Azure.Cosmos.Tests { using System; + using System.Collections.Generic; using System.Diagnostics; using System.Net; using System.Net.Http; @@ -236,7 +237,30 @@ public void ValidateDiagnosticsAppendContext() Thread.Sleep(TimeSpan.FromMilliseconds(100)); } + bool insertIntoDiagnostics1 = true; + bool isInsertDiagnostics = false; + // Start a background thread and ensure that no exception occurs even if items are getting added to the context + // when 2 contexts are appended. + Task.Run(() => + { + isInsertDiagnostics = true; + CosmosSystemInfo cosmosSystemInfo = new CosmosSystemInfo( + cpuLoadHistory: new Documents.Rntbd.CpuLoadHistory(new List().AsReadOnly(), TimeSpan.FromSeconds(1))); + while (insertIntoDiagnostics1) + { + cosmosDiagnostics.AddDiagnosticsInternal(cosmosSystemInfo); + } + }); + + while (!isInsertDiagnostics) + { + Task.Delay(TimeSpan.FromMilliseconds(10)).Wait(); + } + cosmosDiagnostics2.AddDiagnosticsInternal(cosmosDiagnostics); + + // Stop the background inserts + insertIntoDiagnostics1 = false; } string diagnostics = cosmosDiagnostics2.ToString(); From 4bc96e95be100fb3cc44ec50ae48cd8eb67af661 Mon Sep 17 00:00:00 2001 From: Jake Willey Date: Tue, 29 Dec 2020 12:58:37 -0800 Subject: [PATCH 2/2] Revert test changes --- .../CosmosDiagnosticsTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs index 2eb538681d..46653f91cd 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs @@ -511,7 +511,7 @@ public async Task BulkOperationDiagnostic() [TestMethod] public async Task GatewayQueryPlanDiagnostic() { - int totalItems = 10; + int totalItems = 3; IList itemList = await ToDoActivity.CreateRandomItems( this.Container, pkCount: totalItems, @@ -553,7 +553,7 @@ public async Task GatewayQueryPlanDiagnostic() } [TestMethod] - // [DataRow(true)] + [DataRow(true)] [DataRow(false)] public async Task QueryOperationDiagnostic(bool disableDiagnostics) { @@ -564,12 +564,12 @@ public async Task QueryOperationDiagnostic(bool disableDiagnostics) perPKItemCount: 1, randomPartitionKey: true); - //long readFeedTotalOutputDocumentCount = await this.ExecuteQueryAndReturnOutputDocumentCount( - // queryText: null, - // expectedItemCount: totalItems, - // disableDiagnostics: disableDiagnostics); + long readFeedTotalOutputDocumentCount = await this.ExecuteQueryAndReturnOutputDocumentCount( + queryText: null, + expectedItemCount: totalItems, + disableDiagnostics: disableDiagnostics); - //Assert.AreEqual(totalItems, readFeedTotalOutputDocumentCount); + Assert.AreEqual(totalItems, readFeedTotalOutputDocumentCount); //Checking query metrics on typed query long totalOutputDocumentCount = await this.ExecuteQueryAndReturnOutputDocumentCount(