Address PR feedback

dotnet · Oct 18, 2023 · 5f7c4fc · 5f7c4fc
1 parent 966785f
commit 5f7c4fc
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 7 deletions.
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs
@@ -36,9 +36,22 @@ internal partial class Http2Connection : IHttp2StreamLifetimeHandler, IHttpHeade
         private const string EnhanceYourCalmMaximumCountProperty = "Microsoft.AspNetCore.Server.Kestrel.Http2.EnhanceYourCalmCount";
         private const string MaximumFlowControlQueueSizeProperty = "Microsoft.AspNetCore.Server.Kestrel.Http2.MaxConnectionFlowControlQueueSize";
 
-        private static readonly int _enhanceYourCalmMaximumCount = AppContext.GetData(EnhanceYourCalmMaximumCountProperty) is int eycMaxCount
-            ? eycMaxCount
-            : 10;
+        private static readonly int _enhanceYourCalmMaximumCount = GetEnhanceYourCalmMaximumCount();
+
+        private static int GetEnhanceYourCalmMaximumCount()
+        {
+            var data = AppContext.GetData(EnhanceYourCalmMaximumCountProperty);
+            if (data is int count)
+            {
+                return count;
+            }
+            if (data is string countStr && int.TryParse(countStr, out var parsed))
+            {
+                return parsed;
+            }
+
+            return 20; // Empirically derived
+        }
 
         // Accumulate _enhanceYourCalmCount over the course of EnhanceYourCalmTickWindowCount ticks.
         // This should make bursts less likely to trigger disconnects.
@@ -176,10 +189,10 @@ public Http2Connection(HttpConnectionContext context)
                 ? 4 * http2Limits.MaxStreamsPerConnection
                 : (int)ConfiguredMaximumFlowControlQueueSize;
 
-            if (_maximumFlowControlQueueSize < http2Limits.MaxStreamsPerConnection)
+            if (IsMaximumFlowControlQueueSizeEnabled && _maximumFlowControlQueueSize < http2Limits.MaxStreamsPerConnection)
             {
+                Log.Http2FlowControlQueueMaximumTooLow(context.ConnectionId, http2Limits.MaxStreamsPerConnection, _maximumFlowControlQueueSize);
                 _maximumFlowControlQueueSize = http2Limits.MaxStreamsPerConnection;
-                Log.LogTrace($"The configured maximum flow control queue size {ConfiguredMaximumFlowControlQueueSize} is less than the maximum streams per connection {http2Limits.MaxStreamsPerConnection} - increasing to match.");
             }
 
             // Start pool off at a smaller size if the max number of streams is less than the InitialStreamPoolSize

diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IKestrelTrace.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IKestrelTrace.cs
@@ -84,6 +84,8 @@ internal interface IKestrelTrace : ILogger
 
         void Http2FlowControlQueueOperationsExceeded(string connectionId, int count);
 
+        void Http2FlowControlQueueMaximumTooLow(string connectionId, int expected, int actual);
+
         void InvalidResponseHeaderRemoved();
 
         void Http3ConnectionError(string connectionId, Http3ConnectionErrorException ex);

diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs
@@ -395,22 +395,30 @@ public void Http3GoAwayStreamId(string connectionId, long goAwayStreamId)
             Http3GoAwayStreamId(_http3Logger, connectionId, goAwayStreamId);
         }
 
-        [LoggerMessage(54, LogLevel.Error, @"Connection id ""{ConnectionId}"" aborted since at least ""{Count}"" ENHANCE_YOUR_CALM responses were required per second.", EventName = "Http2TooManyEnhanceYourCalms")]
+        [LoggerMessage(54, LogLevel.Debug, @"Connection id ""{ConnectionId}"" aborted since at least ""{Count}"" ENHANCE_YOUR_CALM responses were required per second.", EventName = "Http2TooManyEnhanceYourCalms")]
         private static partial void Http2TooManyEnhanceYourCalms(ILogger logger, string connectionId, int count);
 
         public void Http2TooManyEnhanceYourCalms(string connectionId, int count)
         {
             Http2TooManyEnhanceYourCalms(_http2Logger, connectionId, count);
         }
 
-        [LoggerMessage(55, LogLevel.Error, @"Connection id ""{ConnectionId}"" exceeded the output flow control maximum queue size of ""{Count}"".", EventName = "Http2FlowControlQueueOperationsExceeded")]
+        [LoggerMessage(55, LogLevel.Debug, @"Connection id ""{ConnectionId}"" exceeded the output flow control maximum queue size of ""{Count}"".", EventName = "Http2FlowControlQueueOperationsExceeded")]
         private static partial void Http2FlowControlQueueOperationsExceeded(ILogger logger, string connectionId, int count);
 
         public void Http2FlowControlQueueOperationsExceeded(string connectionId, int count)
         {
             Http2FlowControlQueueOperationsExceeded(_http3Logger, connectionId, count);
         }
 
+        [LoggerMessage(56, LogLevel.Debug, @"Connection id ""{ConnectionId}"" configured maximum flow control queue size ""{Actual}"" is less than the maximum streams per connection ""{Expected}"" - increasing to match.", EventName = "Http2FlowControlQueueMaximumTooLow")]
+        private static partial void Http2FlowControlQueueMaximumTooLow(ILogger logger, string connectionId, int expected, int actual);
+
+        public void Http2FlowControlQueueMaximumTooLow(string connectionId, int expected, int actual)
+        {
+            Http2FlowControlQueueMaximumTooLow(_http3Logger, connectionId, expected, actual);
+        }
+
         public virtual void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
             => _generalLogger.Log(logLevel, eventId, state, exception, formatter);
 

diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Mocks/MockTrace.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Mocks/MockTrace.cs
@@ -62,6 +62,7 @@ public void Http2FrameSending(string connectionId, Http2Frame frame) { }
         public void Http2TooManyEnhanceYourCalms(string connectionId, int count) { }
         public void Http2MaxConcurrentStreamsReached(string connectionId) { }
         public void Http2FlowControlQueueOperationsExceeded(string connectionId, int count) { }
+        public void Http2FlowControlQueueMaximumTooLow(string connectionId, int expected, int actual) { }
         public void InvalidResponseHeaderRemoved() { }
         public void Http3ConnectionError(string connectionId, Http3ConnectionErrorException ex) { }
         public void Http3ConnectionClosing(string connectionId) { }

diff --git a/src/Servers/Kestrel/shared/test/CompositeKestrelTrace.cs b/src/Servers/Kestrel/shared/test/CompositeKestrelTrace.cs
@@ -252,6 +252,12 @@ public void Http2FlowControlQueueOperationsExceeded(string connectionId, int cou
             _trace2.Http2FlowControlQueueOperationsExceeded(connectionId, count);
         }
 
+        public void Http2FlowControlQueueMaximumTooLow(string connectionId, int expected, int actual)
+        {
+            _trace1.Http2FlowControlQueueMaximumTooLow(connectionId, expected, actual);
+            _trace2.Http2FlowControlQueueMaximumTooLow(connectionId, expected, actual);
+        }
+
         public void InvalidResponseHeaderRemoved()
         {
             _trace1.InvalidResponseHeaderRemoved();