Incorporate code review feedback

Microsoft.Azure.Cosmos/src/Diagnostics/BoundedList.cs

@@ -3,6 +3,7 @@
 //------------------------------------------------------------
 namespace Microsoft.Azure.Cosmos.Diagnostics
 {
+ using System;
  using System.Collections;
  using System.Collections.Generic;
 
@@ -19,24 +20,16 @@ internal sealed class BoundedList<T> : IEnumerable<T>
 
  private CircularQueue<T> circularQueue;
 
- private BoundedList(int capacity)
+ public BoundedList(int capacity)
  {
- this.capacity = capacity;
- this.elementList = new List<T>();
- this.circularQueue = null;
- }
-
- internal static bool TryCreate(int capacity, out BoundedList<T> boundedList)
- {
- boundedList = null;
-
- if (capacity > 0)
+ if (capacity < 1)
  {
- boundedList = new BoundedList<T>(capacity);
- return true;
+ throw new ArgumentOutOfRangeException("BoundedList capacity must be positive");
  }
 
- return false;
+ this.capacity = capacity;
+ this.elementList = new List<T>();
+ this.circularQueue = null;
  }
 
  public void Add(T element)
@@ -51,7 +44,7 @@ public void Add(T element)
  }
  else
  {
- _ = CircularQueue<T>.TryCreate(this.capacity, out this.circularQueue);
+ this.circularQueue = new CircularQueue<T>(this.capacity);
  this.circularQueue.AddRange(this.elementList);
  this.elementList = null;
  this.circularQueue.Add(element);

Microsoft.Azure.Cosmos/src/Diagnostics/CircularQueue.cs

@@ -36,29 +36,16 @@ internal sealed class CircularQueue<T> : IEnumerable<T>
  /// Initializes a new instance of the <see cref="CircularQueue{T}"/> class.
  /// </summary>
  /// <param name="capacity"></param>
- private CircularQueue(int capacity)
+ public CircularQueue(int capacity)
  {
- this.head = 0;
- this.tail = 0;
- this.buffer = new T[capacity];
- }
-
- /// <summary>
- /// Initializes a new instance of the <see cref="CircularQueue{T}"/> class.
- /// </summary>
- /// <param name="capacity"></param>
- /// <param name="queue">the newly created queue</param>
- /// <returns>True if successful, false otherwise</returns>
- public static bool TryCreate(int capacity, out CircularQueue<T> queue)
- {
- queue = null;
- if (capacity > 0)
+ if (capacity < 1)
  {
- queue = new CircularQueue<T>(capacity + 1);
- return true;
+ throw new ArgumentOutOfRangeException("circular queue capacity must be positive");
  }
 
- return false;
+ this.head = 0;
+ this.tail = 0;
+ this.buffer = new T[capacity + 1]; // one empty slot
  }
 
  /// <summary>
@@ -96,14 +83,11 @@ private int GetNextIndex(int index)
  private bool TryPop(out T element)
  {
  element = default;
- if (!this.Empty)
- {
- element = this.buffer[this.head];
- this.head = this.GetNextIndex(this.head);
- return true;
- }
+ if (this.Empty) return false;
 
- return false;
+ element = this.buffer[this.head];
+ this.head = this.GetNextIndex(this.head);
+ return true;
  }
 
  /// <inheritdoc/>

Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnosticsContextCore.cs

@@ -18,7 +18,7 @@ namespace Microsoft.Azure.Cosmos
  internal sealed class CosmosDiagnosticsContextCore : CosmosDiagnosticsContext
  {
  private static readonly string DefaultUserAgentString;
- internal static Func<int> Capacity = () => 256;
+ internal static Func<int> Capacity = () => 5120;
  private readonly CosmosDiagnosticScope overallScope;
 
  /// <summary>
@@ -50,8 +50,7 @@ public CosmosDiagnosticsContextCore(
  this.UserAgent = userAgentString ?? throw new ArgumentNullException(nameof(userAgentString));
  this.OperationName = operationName ?? throw new ArgumentNullException(nameof(operationName));
  this.StartUtc = DateTime.UtcNow;
- _ = BoundedList<CosmosDiagnosticsInternal>.TryCreate(Capacity(), out BoundedList<CosmosDiagnosticsInternal> boundedList);
- this.ContextList = boundedList;
+ this.ContextList = new BoundedList<CosmosDiagnosticsInternal>(Capacity());
  this.Diagnostics = new CosmosDiagnosticsCore(this);
  this.overallScope = new CosmosDiagnosticScope("Overall");
  }

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

@@ -230,22 +230,6 @@ public void ValidateDiagnosticsAppendContext()
 
  using (cosmosDiagnostics.GetOverallScope())
  {
- // Test all the different operations on diagnostics context
- using (cosmosDiagnostics.CreateScope("ValidateScope"))
- {
- Thread.Sleep(TimeSpan.FromSeconds(2));
- }
-
- cosmosDiagnostics2 = new CosmosDiagnosticsContextCore(
- nameof(ValidateDiagnosticsAppendContext),
- "MyCustomUserAgentString");
- cosmosDiagnostics2.GetOverallScope().Dispose();
-
- using (cosmosDiagnostics.CreateScope("CosmosDiagnostics2Scope"))
- {
- 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
@@ -257,13 +241,30 @@ public void ValidateDiagnosticsAppendContext()
  cpuLoadHistory: new Documents.Rntbd.CpuLoadHistory(new List<Documents.Rntbd.CpuLoad>().AsReadOnly(), TimeSpan.FromSeconds(1)));
  while (insertIntoDiagnostics1)
  {
+ Task.Delay(TimeSpan.FromMilliseconds(1)).Wait();
  cosmosDiagnostics.AddDiagnosticsInternal(cosmosSystemInfo);
  }
  });
 
  while (!isInsertDiagnostics)
  {
- Task.Delay(TimeSpan.FromMilliseconds(10)).Wait();
+ Task.Delay(TimeSpan.FromMilliseconds(5)).Wait();
+ }
+
+ // Test all the different operations on diagnostics context
+ using (cosmosDiagnostics.CreateScope("ValidateScope"))
+ {
+ Thread.Sleep(TimeSpan.FromMilliseconds(3));
+ }
+
+ cosmosDiagnostics2 = new CosmosDiagnosticsContextCore(
+ nameof(ValidateDiagnosticsAppendContext),
+ "MyCustomUserAgentString");
+ cosmosDiagnostics2.GetOverallScope().Dispose();
+
+ using (cosmosDiagnostics.CreateScope("CosmosDiagnostics2Scope"))
+ {
+ Thread.Sleep(TimeSpan.FromMilliseconds(3));
  }
 
  cosmosDiagnostics2.AddDiagnosticsInternal(cosmosDiagnostics);
@@ -280,7 +281,7 @@ public void ValidateDiagnosticsAppendContext()
 
  [TestMethod]
  [DataRow(10, 100000, 1000001, DisplayName = "Unbounded")]
- [DataRow(10, 100000, 256, DisplayName = "Bounded")]
+ [DataRow(10, 100000, 5120, DisplayName = "Bounded")]
  public void ValidateDiagnosticsAppendContextConcurrentCalls(int threadCount, int itemCountPerThread, int expectedCount)
  {
  CosmosDiagnosticsContextCore.Capacity = () => expectedCount;

Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Diagnostics/BoundedListTests.cs

@@ -3,6 +3,7 @@
 //------------------------------------------------------------
 namespace Microsoft.Azure.Cosmos.Diagnostics
 {
+ using System;
  using System.Linq;
  using Microsoft.VisualStudio.TestTools.UnitTesting;
 
@@ -14,24 +15,24 @@ public void CapacityValidationTests()
  {
  foreach (int x in new[] {-512, -256, -1, 0 })
  {
- Assert.IsFalse(BoundedList<int>.TryCreate(x, out _));
+ Assert.ThrowsException<ArgumentOutOfRangeException>(() => new BoundedList<int>(x));
  }
 
  foreach (int x in new[] { 1, 2, 8, 256 })
  {
- Assert.IsTrue(BoundedList<int>.TryCreate(x, out _));
+ Assert.IsNotNull(new BoundedList<int>(x));
  }
  }
 
  [TestMethod]
- [DataRow( 3, 21, DisplayName = "Extra small")]
- [DataRow( 5, 25, DisplayName = "Small")]
- [DataRow( 64, 256, DisplayName = "Medium")]
- [DataRow( 256, 1024, DisplayName = "Large")]
- [DataRow(2048, 4096, DisplayName = "Extra large")]
+ [DataRow(  3,  21, DisplayName = "Extra small")]
+ [DataRow(  5,  25, DisplayName = "Small")]
+ [DataRow( 256, 1024, DisplayName = "Medium")]
+ [DataRow( 5120, 10240, DisplayName = "Large")]
+ [DataRow(10240, 20480, DisplayName = "Large")]
  public void BasicTests(int capacity, int numElements)
  {
- Assert.IsTrue(BoundedList<int>.TryCreate(capacity, out BoundedList<int> boundedList));
+ BoundedList<int> boundedList = new BoundedList<int>(capacity);
 
  for (int i = 0; i < numElements; ++i)
  {

Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Diagnostics/CircularQueueTests.cs

@@ -12,7 +12,7 @@ public class CircularQueueTests
  [TestMethod]
  public void BasicTests()
  {
- Assert.IsTrue(CircularQueue<int>.TryCreate(4, out CircularQueue<int> queue));
+ CircularQueue<int> queue = new CircularQueue<int>(4);
 
  Assert.IsTrue(queue.Empty);
  Assert.IsFalse(queue.Full);