From 338904cb3768740f1e50dfdd0f75a397faa9fdd2 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Wed, 31 Jul 2019 04:18:29 -0700 Subject: [PATCH 01/11] [Blazor][Fixes #11964] Limit the amount of pending renders --- .../Components/src/Rendering/Renderer.Log.cs | 12 +- .../Components/src/Rendering/Renderer.cs | 12 +- src/Components/Server/src/CircuitOptions.cs | 12 + .../Server/src/Circuits/CircuitHost.cs | 7 +- .../src/Circuits/DefaultCircuitFactory.cs | 9 +- .../Server/src/Circuits/RemoteRenderer.cs | 97 +++++- src/Components/Server/src/ComponentHub.cs | 2 +- .../Server/test/Circuits/CircuitHostTest.cs | 2 +- .../test/Circuits/RemoteRendererTest.cs | 2 + .../Server/test/Circuits/TestCircuitHost.cs | 1 + .../RemoteRendererBufferLimitTest.cs | 290 ++++++++++++++++++ .../BasicTestApp/HighFrequencyComponent.razor | 29 ++ .../test/testassets/BasicTestApp/Index.razor | 2 + .../BasicTestApp/LimitCounterComponent.razor | 25 ++ .../test/testassets/Ignitor/BlazorClient.cs | 19 +- 15 files changed, 499 insertions(+), 22 deletions(-) create mode 100644 src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs create mode 100644 src/Components/test/testassets/BasicTestApp/HighFrequencyComponent.razor create mode 100644 src/Components/test/testassets/BasicTestApp/LimitCounterComponent.razor diff --git a/src/Components/Components/src/Rendering/Renderer.Log.cs b/src/Components/Components/src/Rendering/Renderer.Log.cs index 3b70f589736a..f822b06f8b45 100644 --- a/src/Components/Components/src/Rendering/Renderer.Log.cs +++ b/src/Components/Components/src/Rendering/Renderer.Log.cs @@ -25,6 +25,9 @@ internal static class Log private static readonly Action _handlingEvent = LoggerMessage.Define(LogLevel.Debug, new EventId(5, "HandlingEvent"), "Handling event {EventId} of type '{EventType}'"); + private static readonly Action _noPendingComponentRenderRequests = + LoggerMessage.Define(LogLevel.Debug, new EventId(6, "NoPendingComponentRenderRequests"), "No pending component render requests."); + public static void InitializingComponent(ILogger logger, ComponentState componentState, ComponentState parentComponentState) { if (logger.IsEnabled(LogLevel.Debug)) // This is almost always false, so skip the evaluations @@ -48,7 +51,7 @@ public static void RenderingComponent(ILogger logger, ComponentState componentSt } } - internal static void DisposingComponent(ILogger logger, ComponentState componentState) + public static void DisposingComponent(ILogger logger, ComponentState componentState) { if (logger.IsEnabled(LogLevel.Debug)) // This is almost always false, so skip the evaluations { @@ -56,10 +59,15 @@ internal static void DisposingComponent(ILogger logger, ComponentState } } - internal static void HandlingEvent(ILogger logger, ulong eventHandlerId, EventArgs eventArgs) + public static void HandlingEvent(ILogger logger, ulong eventHandlerId, EventArgs eventArgs) { _handlingEvent(logger, eventHandlerId, eventArgs?.GetType().Name ?? "null", null); } + + public static void NoPendingComponentRenderRequests(ILogger logger) + { + _noPendingComponentRenderRequests(logger, null); + } } } } diff --git a/src/Components/Components/src/Rendering/Renderer.cs b/src/Components/Components/src/Rendering/Renderer.cs index 6e0ed1633ed0..0ec8c23c32e9 100644 --- a/src/Components/Components/src/Rendering/Renderer.cs +++ b/src/Components/Components/src/Rendering/Renderer.cs @@ -398,13 +398,22 @@ private ComponentState GetOptionalComponentState(int componentId) ? componentState : null; - private void ProcessRenderQueue() + /// + /// Processses pending renders requests from components. + /// + protected virtual void ProcessRenderQueue() { _isBatchInProgress = true; var updateDisplayTask = Task.CompletedTask; try { + if (_batchBuilder.ComponentRenderQueue.Count == 0) + { + Log.NoPendingComponentRenderRequests(_logger); + return; + } + // Process render queue until empty while (_batchBuilder.ComponentRenderQueue.Count > 0) { @@ -423,6 +432,7 @@ private void ProcessRenderQueue() { // Ensure we catch errors while running the render functions of the components. HandleException(e); + return; } finally { diff --git a/src/Components/Server/src/CircuitOptions.cs b/src/Components/Server/src/CircuitOptions.cs index 30c1b9c407ff..68ca25c85aa8 100644 --- a/src/Components/Server/src/CircuitOptions.cs +++ b/src/Components/Server/src/CircuitOptions.cs @@ -64,5 +64,17 @@ public sealed class CircuitOptions /// Defaults to 1 minute. /// public TimeSpan JSInteropDefaultCallTimeout { get; set; } = TimeSpan.FromMinutes(1); + + /// + /// Gets or sets the maximum number of render batches that a circuit will buffer until an acknowledgement for the batch is + /// received. + /// + /// + /// When the limit of buffered render batches is reached components will stop rendering and will wait until either the + /// circuit is disconnected and disposed or at least one batch gets acknowledged. + /// + /// + /// Defaults to 10. + public int MaxBufferedUnacknowledgedRenderBatches { get; set; } = 10; } } diff --git a/src/Components/Server/src/Circuits/CircuitHost.cs b/src/Components/Server/src/Circuits/CircuitHost.cs index 563cacbff6c1..93679c0bfe3d 100644 --- a/src/Components/Server/src/Circuits/CircuitHost.cs +++ b/src/Components/Server/src/Circuits/CircuitHost.cs @@ -106,7 +106,10 @@ internal void SendPendingBatches() // Dispatch any buffered renders we accumulated during a disconnect. // Note that while the rendering is async, we cannot await it here. The Task returned by ProcessBufferedRenderBatches relies on // OnRenderCompleted to be invoked to complete, and SignalR does not allow concurrent hub method invocations. - _ = Renderer.Dispatcher.InvokeAsync(() => Renderer.ProcessBufferedRenderBatches()); + _ = Renderer.Dispatcher.InvokeAsync(() => + { + return Renderer.ProcessBufferedRenderBatches(); + }); } public async Task EndInvokeJSFromDotNet(long asyncCall, bool succeded, string arguments) @@ -230,7 +233,7 @@ await Renderer.Dispatcher.InvokeAsync(() => catch (Exception ex) { // We don't expect any of this code to actually throw, because DotNetDispatcher.BeginInvoke doesn't throw - // however, we still want this to get logged if we do. + // however, we still want this to get logged if we do. UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); } } diff --git a/src/Components/Server/src/Circuits/DefaultCircuitFactory.cs b/src/Components/Server/src/Circuits/DefaultCircuitFactory.cs index d353d5dc2c1e..df16c45c4e4a 100644 --- a/src/Components/Server/src/Circuits/DefaultCircuitFactory.cs +++ b/src/Components/Server/src/Circuits/DefaultCircuitFactory.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Security.Claims; using System.Text.Encodings.Web; +using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Web; using Microsoft.AspNetCore.Components.Web.Rendering; using Microsoft.AspNetCore.Components.Routing; @@ -13,8 +14,8 @@ using Microsoft.AspNetCore.Http.Features; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Microsoft.JSInterop; -using System.Threading.Tasks; namespace Microsoft.AspNetCore.Components.Server.Circuits { @@ -24,16 +25,19 @@ internal class DefaultCircuitFactory : CircuitFactory private readonly ILoggerFactory _loggerFactory; private readonly ILogger _logger; private readonly CircuitIdFactory _circuitIdFactory; + private readonly CircuitOptions _options; public DefaultCircuitFactory( IServiceScopeFactory scopeFactory, ILoggerFactory loggerFactory, - CircuitIdFactory circuitIdFactory) + CircuitIdFactory circuitIdFactory, + IOptions options) { _scopeFactory = scopeFactory ?? throw new ArgumentNullException(nameof(scopeFactory)); _loggerFactory = loggerFactory; _logger = _loggerFactory.CreateLogger(); _circuitIdFactory = circuitIdFactory ?? throw new ArgumentNullException(nameof(circuitIdFactory)); + _options = options.Value; } public override CircuitHost CreateCircuitHost( @@ -80,6 +84,7 @@ public override CircuitHost CreateCircuitHost( scope.ServiceProvider, _loggerFactory, rendererRegistry, + _options, jsRuntime, client, encoder, diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index 8b9ebb341a10..f61b2ed7bd2b 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -9,6 +9,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Rendering; +using Microsoft.AspNetCore.Components.Server; using Microsoft.AspNetCore.Components.Server.Circuits; using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.Internal; @@ -23,10 +24,13 @@ internal class RemoteRenderer : HtmlRenderer private readonly IJSRuntime _jsRuntime; private readonly CircuitClientProxy _client; + private readonly CircuitOptions _options; private readonly RendererRegistry _rendererRegistry; private readonly ILogger _logger; private long _nextRenderId = 1; private bool _disposing = false; + private bool _queueIsFullNotified; + private ConcurrentQueue _unacknowledgedRenderBatches = new ConcurrentQueue(); /// /// Notifies when a rendering exception occured. @@ -40,6 +44,7 @@ public RemoteRenderer( IServiceProvider serviceProvider, ILoggerFactory loggerFactory, RendererRegistry rendererRegistry, + CircuitOptions options, IJSRuntime jsRuntime, CircuitClientProxy client, HtmlEncoder encoder, @@ -49,13 +54,12 @@ public RemoteRenderer( _rendererRegistry = rendererRegistry; _jsRuntime = jsRuntime; _client = client; + _options = options; Id = _rendererRegistry.Add(this); _logger = logger; } - internal ConcurrentQueue UnacknowledgedRenderBatches = new ConcurrentQueue(); - public override Dispatcher Dispatcher { get; } = Dispatcher.CreateDefault(); public int Id { get; } @@ -81,6 +85,43 @@ public Task AddComponentAsync(Type componentType, string domElementSelector) return RenderRootComponentAsync(componentId); } + protected override void ProcessRenderQueue() + { + // If we got here it means we are at max capacity, so we don't want to actually process the queue, + // as we have a client that is not acknowledging render batches fast enough (something we consider needs + // to be fast). + // The result is somethign as follows: + // Lets imagine an extreme case where the server produces a new batch every milisecond. + // Lets say the client is able to ACK a batch every 100 miliseconds. + // When the app starts the client might see the sequence 0.000->0.{MAXUnacknowledgeRenderBatches} and then + // after 100 miliseconds it sees it jump to 0.1xx, then to 0.2xx where xx is something between {0..99} the + // reason for this is that the server slows down rendering new batches to as fast as the client can consume + // them. + // Similarly, if a client were to send events at a faster pace than the server can consume them, the server + // would still proces the events, but would not produce new renders until it gets an ack that frees up space + // for a new render. + if (_unacknowledgedRenderBatches.Count >= _options.MaxBufferedUnacknowledgedRenderBatches) + { + // We should never see UnacknowledgedRenderBatches.Count > _options.MaxBufferedUnacknowledgedRenderBatches + // But if we do, it's safer to simply disable the rendering in that case too instead of allow batches to + // keep piling up. + if (!_queueIsFullNotified) + { + // We skip logging after the first time we run into this situation to avoid filling the logs with + // entries on high frequency render scenarios. + // We try our best to log the queue is full at most once, although we don't guarantee it due to + // concurrency. + // When we complete a render we set this flag to false, so the next time the queue gets filled it + // also shows up. + _queueIsFullNotified = true; + Log.FullUnacknowledgedRenderBatchesQueue(_logger); + } + + return; + } + base.ProcessRenderQueue(); + } + /// protected override void HandleException(Exception exception) { @@ -104,7 +145,7 @@ protected override void Dispose(bool disposing) { _disposing = true; _rendererRegistry.TryRemove(Id); - while (UnacknowledgedRenderBatches.TryDequeue(out var entry)) + while (_unacknowledgedRenderBatches.TryDequeue(out var entry)) { entry.CompletionSource.TrySetCanceled(); entry.Data.Dispose(); @@ -146,7 +187,7 @@ protected override Task UpdateDisplayAsync(in RenderBatch batch) // Buffer the rendered batches no matter what. We'll send it down immediately when the client // is connected or right after the client reconnects. - UnacknowledgedRenderBatches.Enqueue(pendingRender); + _unacknowledgedRenderBatches.Enqueue(pendingRender); } catch { @@ -167,7 +208,7 @@ public Task ProcessBufferedRenderBatches() // All the batches are sent in order based on the fact that SignalR // provides ordering for the underlying messages and that the batches // are always in order. - return Task.WhenAll(UnacknowledgedRenderBatches.Select(b => WriteBatchBytesAsync(b))); + return Task.WhenAll(_unacknowledgedRenderBatches.Select(b => WriteBatchBytesAsync(b))); } private async Task WriteBatchBytesAsync(UnacknowledgedRenderBatch pending) @@ -234,7 +275,7 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) // synchronizes calls to hub methods. That is, it won't issue more than one call to this method from the same hub // at the same time on different threads. - if (!UnacknowledgedRenderBatches.TryPeek(out var nextUnacknowledgedBatch) || incomingBatchId < nextUnacknowledgedBatch.BatchId) + if (!_unacknowledgedRenderBatches.TryPeek(out var nextUnacknowledgedBatch) || incomingBatchId < nextUnacknowledgedBatch.BatchId) { Log.ReceivedDuplicateBatchAck(_logger, incomingBatchId); } @@ -242,10 +283,13 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) { var lastBatchId = nextUnacknowledgedBatch.BatchId; // Order is important here so that we don't prematurely dequeue the last nextUnacknowledgedBatch - while (UnacknowledgedRenderBatches.TryPeek(out nextUnacknowledgedBatch) && nextUnacknowledgedBatch.BatchId <= incomingBatchId) + while (_unacknowledgedRenderBatches.TryPeek(out nextUnacknowledgedBatch) && nextUnacknowledgedBatch.BatchId <= incomingBatchId) { lastBatchId = nextUnacknowledgedBatch.BatchId; - UnacknowledgedRenderBatches.TryDequeue(out _); + // At this point the queue is no longer full, we have at least emptied one slot, so we allow a further + // full queue log entry the next time it fills up. + _queueIsFullNotified = false; + _unacknowledgedRenderBatches.TryDequeue(out _); ProcessPendingBatch(errorMessageOrNull, nextUnacknowledgedBatch); } @@ -253,7 +297,18 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) { HandleException( new InvalidOperationException($"Received an acknowledgement for batch with id '{incomingBatchId}' when the last batch produced was '{lastBatchId}'.")); + return; } + + // Normally we will not have pending renders, but it might happen that we reached the limit of + // available buffered renders and new renders got queued. + // Invke ProcessBufferedRenderRequests so that we might produce any additional batch that is + // missing. + // Its also safe to use the discard as ProcessRenderQueue won't throw. + _ = Dispatcher.InvokeAsync(() => + { + ProcessRenderQueue(); + }); } } @@ -321,6 +376,7 @@ private static class Log private static readonly Action _completingBatchWithError; private static readonly Action _completingBatchWithoutError; private static readonly Action _receivedDuplicateBatchAcknowledgement; + private static readonly Action _fullUnacknowledgedRenderBatchesQueue; private static class EventIds { @@ -331,6 +387,7 @@ private static class EventIds public static readonly EventId CompletingBatchWithError = new EventId(104, "CompletingBatchWithError"); public static readonly EventId CompletingBatchWithoutError = new EventId(105, "CompletingBatchWithoutError"); public static readonly EventId ReceivedDuplicateBatchAcknowledgement = new EventId(106, "ReceivedDuplicateBatchAcknowledgement"); + public static readonly EventId FullUnacknowledgedRenderBatchesQueue = new EventId(107, "FullUnacknowledgedRenderBatchesQueue"); } static Log() @@ -369,6 +426,11 @@ static Log() LogLevel.Debug, EventIds.ReceivedDuplicateBatchAcknowledgement, "Received a duplicate ACK for batch id '{IncomingBatchId}'."); + + _fullUnacknowledgedRenderBatchesQueue = LoggerMessage.Define( + LogLevel.Debug, + EventIds.FullUnacknowledgedRenderBatchesQueue, + "The queue of unacknowledged render batches is full."); } public static void SendBatchDataFailed(ILogger logger, Exception exception) @@ -421,10 +483,27 @@ public static void CompletingBatchWithoutError(ILogger logger, long batchId, Tim null); } - internal static void ReceivedDuplicateBatchAck(ILogger logger, long incomingBatchId) + public static void ReceivedDuplicateBatchAck(ILogger logger, long incomingBatchId) { _receivedDuplicateBatchAcknowledgement(logger, incomingBatchId, null); } + + public static void FullUnacknowledgedRenderBatchesQueue(ILogger logger) + { + _fullUnacknowledgedRenderBatchesQueue(logger, null); + } } } + + internal readonly struct PendingRender + { + public PendingRender(int componentId, RenderFragment renderFragment) + { + ComponentId = componentId; + RenderFragment = renderFragment; + } + + public int ComponentId { get; } + public RenderFragment RenderFragment { get; } + } } diff --git a/src/Components/Server/src/ComponentHub.cs b/src/Components/Server/src/ComponentHub.cs index 4da9a3391ef2..00e26ce2be75 100644 --- a/src/Components/Server/src/ComponentHub.cs +++ b/src/Components/Server/src/ComponentHub.cs @@ -177,7 +177,7 @@ public async Task ConnectCircuit(string circuitId) CircuitHost.UnhandledException += CircuitHost_UnhandledException; circuitHost.SetCircuitUser(Context.User); - circuitHost.SendPendingBatches(); + circuitHost.ResumeCircuit(); return true; } diff --git a/src/Components/Server/test/Circuits/CircuitHostTest.cs b/src/Components/Server/test/Circuits/CircuitHostTest.cs index 58803a56b2f3..a6c78f8683b1 100644 --- a/src/Components/Server/test/Circuits/CircuitHostTest.cs +++ b/src/Components/Server/test/Circuits/CircuitHostTest.cs @@ -240,7 +240,7 @@ private static TestRemoteRenderer GetRemoteRenderer() private class TestRemoteRenderer : RemoteRenderer { public TestRemoteRenderer(IServiceProvider serviceProvider, RendererRegistry rendererRegistry, IJSRuntime jsRuntime, IClientProxy client) - : base(serviceProvider, NullLoggerFactory.Instance, rendererRegistry, jsRuntime, new CircuitClientProxy(client, "connection"), HtmlEncoder.Default, NullLogger.Instance) + : base(serviceProvider, NullLoggerFactory.Instance, rendererRegistry, new CircuitOptions(), jsRuntime, new CircuitClientProxy(client, "connection"), HtmlEncoder.Default, NullLogger.Instance) { } diff --git a/src/Components/Server/test/Circuits/RemoteRendererTest.cs b/src/Components/Server/test/Circuits/RemoteRendererTest.cs index ffb913544ace..8d19d72f1a92 100644 --- a/src/Components/Server/test/Circuits/RemoteRendererTest.cs +++ b/src/Components/Server/test/Circuits/RemoteRendererTest.cs @@ -8,6 +8,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Rendering; +using Microsoft.AspNetCore.Components.Server; using Microsoft.AspNetCore.Components.Server.Circuits; using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.DependencyInjection; @@ -389,6 +390,7 @@ private RemoteRenderer GetRemoteRenderer(IServiceProvider serviceProvider, Circu serviceProvider, NullLoggerFactory.Instance, new RendererRegistry(), + new CircuitOptions(), jsRuntime.Object, circuitClientProxy, HtmlEncoder.Default, diff --git a/src/Components/Server/test/Circuits/TestCircuitHost.cs b/src/Components/Server/test/Circuits/TestCircuitHost.cs index f8f5996d95e9..328120d70c0e 100644 --- a/src/Components/Server/test/Circuits/TestCircuitHost.cs +++ b/src/Components/Server/test/Circuits/TestCircuitHost.cs @@ -46,6 +46,7 @@ public static CircuitHost Create( serviceScope.ServiceProvider ?? Mock.Of(), NullLoggerFactory.Instance, new RendererRegistry(), + new CircuitOptions(), jsRuntime, clientProxy, HtmlEncoder.Default, diff --git a/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs b/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs new file mode 100644 index 000000000000..bbad44e12d7d --- /dev/null +++ b/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs @@ -0,0 +1,290 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading.Tasks; +using Ignitor; +using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; +using Xunit; +using Xunit.Sdk; + +namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests +{ + public class RemoteRendererBufferLimitTest : IClassFixture, IDisposable + { + private static readonly TimeSpan DefaultLatencyTimeout = Debugger.IsAttached ? TimeSpan.FromSeconds(60) : TimeSpan.FromMilliseconds(500); + + private AspNetSiteServerFixture _serverFixture; + + public RemoteRendererBufferLimitTest(AspNetSiteServerFixture serverFixture) + { + serverFixture.BuildWebHostMethod = TestServer.Program.BuildWebHost; + _serverFixture = serverFixture; + + // Needed here for side-effects + _ = _serverFixture.RootUri; + + Client = new BlazorClient() { DefaultLatencyTimeout = DefaultLatencyTimeout }; + Client.RenderBatchReceived += (rendererId, id, data) => Batches.Add(new Batch(rendererId, id, data)); + Client.OnCircuitError += (error) => Errors.Add(error); + + Sink = _serverFixture.Host.Services.GetRequiredService(); + Sink.MessageLogged += LogMessages; + } + + public BlazorClient Client { get; set; } + + private IList Batches { get; set; } = new List(); + + // We use a stack so that we can search the logs in reverse order + private ConcurrentStack Logs { get; set; } = new ConcurrentStack(); + + private IList Errors { get; set; } = new List(); + + public TestSink Sink { get; private set; } + + [Fact] + public async Task NoNewBatchesAreCreated_WhenThereAreNoPendingRenderRequestsFromComponents() + { + var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + // Act + await Client.SelectAsync("test-selector-select", "BasicTestApp.LimitCounterComponent"); + + // Assert + Assert.Equal(2, Batches.Count); + Assert.Contains(Logs.ToArray(), l => (LogLevel.Debug, "No pending component render requests.") == (l.LogLevel, l.Message)); + } + + [Fact] + public async Task NotAcknowledgingRenders_ProducesBatches_UpToTheLimit() + { + var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + // Act + Client.ConfirmRenderBatch = false; + var tcs = new TaskCompletionSource(); + void SignalOnBatchNumber(int batchId, int rendererId, byte[] data) + { + // We wait for the batch Id 11 because its 1 batch for the initial render + 10 after we stop + // unacknowledging batches. + if (batchId == 11) + { + tcs.TrySetResult(null); + } + } + Client.RenderBatchReceived += SignalOnBatchNumber; + + await Client.SelectAsync("test-selector-select", "BasicTestApp.HighFrequencyComponent"); + + await Task.WhenAny(Task.Delay(1000), tcs.Task); + + // Assert + // We check that the value is 9 because the ticker starts at 0, there is a max buffer of 10 unacknowledged + // batches and we don't acknowledge the initial batch to render the ticker component. + Assert.Contains(Client.FindElementById("tick-value").Children, c => c is TextNode tn && tn.TextContent == "9"); + Assert.Equal(11, Batches.Count); + } + + [Fact] + public async Task ProducesNewBatch_WhenABatchGetsAcknowledged() + { + // Arrange + var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + Client.ConfirmRenderBatch = false; + var tcs = new TaskCompletionSource(); + void SignalOnBatchNumber(int batchId, int rendererId, byte[] data) + { + // We wait for the batch Id 11 because its 1 batch for the initial render + 10 after we stop + // unacknowledging batches. + if (batchId == 11) + { + tcs.TrySetResult(null); + } + } + Client.RenderBatchReceived += SignalOnBatchNumber; + + await Client.SelectAsync("test-selector-select", "BasicTestApp.HighFrequencyComponent"); + + await Task.WhenAny(Task.Delay(1000), tcs.Task); + + Assert.Contains(Client.FindElementById("tick-value").Children, c => c is TextNode tn && tn.TextContent == "9"); + Assert.Equal(11, Batches.Count); + + // Act + await Client.ExpectRenderBatch(() => Client.ConfirmBatch(3)); // This is the batch after the initial batch + + // Assert + Assert.Contains( + Client.FindElementById("tick-value").Children, + c => c is TextNode tn && int.Parse(tn.TextContent) >= 10); + Assert.Equal(12, Batches.Count); + } + + [Fact] + public async Task ContinuesProducingBatches_UntilItStopsReceivingAcksAndResumesWhenItReceivesAcksAgain() + { + // Arrange + var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + await Client.SelectAsync("test-selector-select", "BasicTestApp.HighFrequencyComponent"); + await Task.Delay(100); + + var tcs = new TaskCompletionSource(); + var foundLogMessage = false; + void SignalOnFullQueue(WriteContext context) + { + // We wait for the batch Id 11 because its 1 batch for the initial render + 10 after we stop + // unacknowledging batches. + if (context.Message == "The queue of unacknowledged render batches is full.") + { + tcs.TrySetResult(null); + foundLogMessage = true; + } + } + + Sink.MessageLogged+= SignalOnFullQueue; + Client.ConfirmRenderBatch = false; // This will stop acknowledging batches, so we should see a queue full message. + + await Task.WhenAny(Task.Delay(1000), tcs.Task); + + Sink.MessageLogged -= SignalOnFullQueue; + Assert.True(foundLogMessage, "Log entry 'The queue of unacknowledged render batches is full.' not found."); + + var currentCount = Batches.Count; + var lastBatchId = Batches[^1].Id; + var lastRenderedValue = ((TextNode)Client.FindElementById("tick-value").Children.Single()).TextContent; + // Act + Client.ConfirmRenderBatch = true; + + // This will resume the render batches. + await Client.ExpectRenderBatch(() => Client.ConfirmBatch(lastBatchId)); + + // An indeterminate amount of renders will happen in this time. + await Task.Delay(100); + + Sink.MessageLogged += SignalOnFullQueue; + tcs = new TaskCompletionSource(); + foundLogMessage = false; + Logs.Clear(); + + // This will cause the renders to stop. + Client.ConfirmRenderBatch = false; + await Task.WhenAny(Task.Delay(1000), tcs.Task); + + Sink.MessageLogged -= SignalOnFullQueue; + Assert.True(foundLogMessage, "Log entry 'The queue of unacknowledged render batches is full.' not found."); + + // Assert + Assert.True(lastBatchId + 10 < Batches[^1].Id, "We didn't produce more than 10 batches"); + + var finalRenderedValue = ((TextNode)Client.FindElementById("tick-value").Children.Single()).TextContent; + Assert.True(int.Parse(finalRenderedValue) > int.Parse(lastRenderedValue), "Ticker count didn't increase enough"); + } + + [Fact] + public async Task DispatchedEventsWillKeepBeingProcessed_ButUpdatedWillBeDelayedUntilARenderIsAcknowledged() + { + // Arrange + var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + await Client.SelectAsync("test-selector-select", "BasicTestApp.LimitCounterComponent"); + Client.ConfirmRenderBatch = false; + + for (int i = 0; i < 10; i++) + { + await Client.ClickAsync("increment"); + } + await Client.ClickAsync("increment", expectRenderBatch: false); + + Assert.Single(Logs, l => (LogLevel.Debug, "The queue of unacknowledged render batches is full.") == (l.LogLevel, l.Message)); + Assert.Equal("10", ((TextNode)Client.FindElementById("the-count").Children.Single()).TextContent); + var fullCount = Batches.Count; + + // Act + await Client.ClickAsync("increment", expectRenderBatch: false); + + Assert.Single(Logs, l => (LogLevel.Debug, "The queue of unacknowledged render batches is full.") == (l.LogLevel, l.Message)); + Assert.Equal(fullCount, Batches.Count); + Client.ConfirmRenderBatch = true; + + // This will resume the render batches. + await Client.ExpectRenderBatch(() => Client.ConfirmBatch(Batches[^1].Id)); + + // Assert + Assert.Equal("12", ((TextNode)Client.FindElementById("the-count").Children.Single()).TextContent); + Assert.Equal(fullCount + 1, Batches.Count); + } + + private void LogMessages(WriteContext context) + { + Logs.Push(new LogMessage(context.LogLevel, context.Message, context.Exception)); + } + + [DebuggerDisplay("{Message,nq}")] + private class LogMessage : IEquatable + { + public LogMessage(LogLevel logLevel, string message, Exception exception) + { + LogLevel = logLevel; + Message = message; + Exception = exception; + } + + public LogLevel LogLevel { get; set; } + public string Message { get; set; } + public Exception Exception { get; set; } + + public override bool Equals(object obj) => Equals(obj as LogMessage); + + public bool Equals([AllowNull] LogMessage other) => + other != null && + LogLevel == other.LogLevel && + Message == other.Message && + EqualityComparer.Default.Equals(Exception, other.Exception); + + public override int GetHashCode() => HashCode.Combine(LogLevel, Message, Exception); + } + + private class Batch + { + public Batch(int rendererId, int id, byte[] data) + { + Id = id; + RendererId = rendererId; + Data = data; + } + + public int Id { get; } + public int RendererId { get; } + public byte[] Data { get; } + } + + public void Dispose() + { + if (Sink != null) + { + Sink.MessageLogged -= LogMessages; + } + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/HighFrequencyComponent.razor b/src/Components/test/testassets/BasicTestApp/HighFrequencyComponent.razor new file mode 100644 index 000000000000..50daa848bd1a --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/HighFrequencyComponent.razor @@ -0,0 +1,29 @@ +@page "/highfrequency" +@implements IDisposable +@using System.Threading + +

High frequency render component

+

This component renders updates at a really high frequency. We use it to test the server-side limits on the amount + of pending batches we keep buffered in the server while we wait for a client to acknowledge them.

+

@tick

+ +@code { + int tick; + private bool _isDisposed; + + protected override void OnInitialized() + { + Task.Run(() => InvokeAsync(async () => + { + while (!_isDisposed) + { + await Task.Delay(TimeSpan.FromMilliseconds(1)); + tick++; + StateHasChanged(); + } + })); + } + + + public void Dispose() => _isDisposed = true; +} diff --git a/src/Components/test/testassets/BasicTestApp/Index.razor b/src/Components/test/testassets/BasicTestApp/Index.razor index fe8608105c8a..79edea70fac4 100644 --- a/src/Components/test/testassets/BasicTestApp/Index.razor +++ b/src/Components/test/testassets/BasicTestApp/Index.razor @@ -31,6 +31,7 @@ + @@ -43,6 +44,7 @@ + diff --git a/src/Components/test/testassets/BasicTestApp/LimitCounterComponent.razor b/src/Components/test/testassets/BasicTestApp/LimitCounterComponent.razor new file mode 100644 index 000000000000..bb60eb48cd49 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/LimitCounterComponent.razor @@ -0,0 +1,25 @@ +

Counter

+ +

This counter component is used in the server-side limit tests to showcase that we keep dispatching and processing + events when the unacknowledged render batches queue is full and that we produce the update UI once we receive an + acknowledgement.

+ +

Current count: @currentCount

+

+ + +@code { + int currentCount = 0; + bool handleClicks = true; + + void IncrementCount() + { + currentCount++; + } + + public void Reset() + { + currentCount = 0; + StateHasChanged(); + } +} diff --git a/src/Components/test/testassets/Ignitor/BlazorClient.cs b/src/Components/test/testassets/Ignitor/BlazorClient.cs index c11c2951ddd9..fa28c7f1b9fe 100644 --- a/src/Components/test/testassets/Ignitor/BlazorClient.cs +++ b/src/Components/test/testassets/Ignitor/BlazorClient.cs @@ -112,14 +112,20 @@ public Task PrepareForNextCircuitError(TimeSpan? timeout) return NextErrorReceived.Completion.Task; } - public async Task ClickAsync(string elementId) + public Task ClickAsync(string elementId, bool expectRenderBatch = true) { if (!Hive.TryFindElementById(elementId, out var elementNode)) { throw new InvalidOperationException($"Could not find element with id {elementId}."); } - - await ExpectRenderBatch(() => elementNode.ClickAsync(HubConnection)); + if (expectRenderBatch) + { + return ExpectRenderBatch(() => elementNode.ClickAsync(HubConnection)); + } + else + { + return elementNode.ClickAsync(HubConnection); + } } public async Task SelectAsync(string elementId, string value) @@ -292,7 +298,7 @@ private void OnRenderBatch(int browserRendererId, int batchId, byte[] batchData) if (ConfirmRenderBatch) { - HubConnection.InvokeAsync("OnRenderCompleted", batchId, /* error */ null); + _ = ConfirmBatch(batchId); } NextBatchReceived?.Completion?.TrySetResult(null); @@ -303,6 +309,11 @@ private void OnRenderBatch(int browserRendererId, int batchId, byte[] batchData) } } + public Task ConfirmBatch(int batchId, string error = null) + { + return HubConnection.InvokeAsync("OnRenderCompleted", batchId, error); + } + private void OnError(string error) { try From 27fd01cf9f71b20c33e6361c86afe175a485f22c Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Wed, 31 Jul 2019 08:49:14 -0700 Subject: [PATCH 02/11] Cleanups --- .../Server/src/Circuits/CircuitHost.cs | 5 +-- .../Server/src/Circuits/RemoteRenderer.cs | 34 +++++++++---------- src/Components/Server/src/ComponentHub.cs | 2 +- .../RemoteRendererBufferLimitTest.cs | 17 ++-------- 4 files changed, 20 insertions(+), 38 deletions(-) diff --git a/src/Components/Server/src/Circuits/CircuitHost.cs b/src/Components/Server/src/Circuits/CircuitHost.cs index 93679c0bfe3d..c2a38e25b99c 100644 --- a/src/Components/Server/src/Circuits/CircuitHost.cs +++ b/src/Components/Server/src/Circuits/CircuitHost.cs @@ -106,10 +106,7 @@ internal void SendPendingBatches() // Dispatch any buffered renders we accumulated during a disconnect. // Note that while the rendering is async, we cannot await it here. The Task returned by ProcessBufferedRenderBatches relies on // OnRenderCompleted to be invoked to complete, and SignalR does not allow concurrent hub method invocations. - _ = Renderer.Dispatcher.InvokeAsync(() => - { - return Renderer.ProcessBufferedRenderBatches(); - }); + _ = Renderer.Dispatcher.InvokeAsync(() => Renderer.ProcessBufferedRenderBatches()); } public async Task EndInvokeJSFromDotNet(long asyncCall, bool succeded, string arguments) diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index f61b2ed7bd2b..41444e2b92a4 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -87,23 +87,24 @@ public Task AddComponentAsync(Type componentType, string domElementSelector) protected override void ProcessRenderQueue() { - // If we got here it means we are at max capacity, so we don't want to actually process the queue, - // as we have a client that is not acknowledging render batches fast enough (something we consider needs - // to be fast). - // The result is somethign as follows: - // Lets imagine an extreme case where the server produces a new batch every milisecond. - // Lets say the client is able to ACK a batch every 100 miliseconds. - // When the app starts the client might see the sequence 0.000->0.{MAXUnacknowledgeRenderBatches} and then - // after 100 miliseconds it sees it jump to 0.1xx, then to 0.2xx where xx is something between {0..99} the - // reason for this is that the server slows down rendering new batches to as fast as the client can consume - // them. - // Similarly, if a client were to send events at a faster pace than the server can consume them, the server - // would still proces the events, but would not produce new renders until it gets an ack that frees up space - // for a new render. if (_unacknowledgedRenderBatches.Count >= _options.MaxBufferedUnacknowledgedRenderBatches) { + // If we got here it means we are at max capacity, so we don't want to actually process the queue, + // as we have a client that is not acknowledging render batches fast enough (something we consider needs + // to be fast). + // The result is somethign as follows: + // Lets imagine an extreme case where the server produces a new batch every milisecond. + // Lets say the client is able to ACK a batch every 100 miliseconds. + // When the app starts the client might see the sequence 0.000->0.{MAXUnacknowledgeRenderBatches} and then + // after 100 miliseconds it sees it jump to 0.1xx, then to 0.2xx where xx is something between {0..99} the + // reason for this is that the server slows down rendering new batches to as fast as the client can consume + // them. + // Similarly, if a client were to send events at a faster pace than the server can consume them, the server + // would still proces the events, but would not produce new renders until it gets an ack that frees up space + // for a new render. // We should never see UnacknowledgedRenderBatches.Count > _options.MaxBufferedUnacknowledgedRenderBatches - // But if we do, it's safer to simply disable the rendering in that case too instead of allow batches to + + // But if we do, it's safer to simply disable the rendering in that case too instead of allowing batches to // keep piling up. if (!_queueIsFullNotified) { @@ -305,10 +306,7 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) // Invke ProcessBufferedRenderRequests so that we might produce any additional batch that is // missing. // Its also safe to use the discard as ProcessRenderQueue won't throw. - _ = Dispatcher.InvokeAsync(() => - { - ProcessRenderQueue(); - }); + _ = Dispatcher.InvokeAsync(() => ProcessRenderQueue()); } } diff --git a/src/Components/Server/src/ComponentHub.cs b/src/Components/Server/src/ComponentHub.cs index 00e26ce2be75..4da9a3391ef2 100644 --- a/src/Components/Server/src/ComponentHub.cs +++ b/src/Components/Server/src/ComponentHub.cs @@ -177,7 +177,7 @@ public async Task ConnectCircuit(string circuitId) CircuitHost.UnhandledException += CircuitHost_UnhandledException; circuitHost.SetCircuitUser(Context.User); - circuitHost.ResumeCircuit(); + circuitHost.SendPendingBatches(); return true; } diff --git a/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs b/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs index bbad44e12d7d..297bd041e158 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs @@ -235,13 +235,10 @@ public async Task DispatchedEventsWillKeepBeingProcessed_ButUpdatedWillBeDelayed Assert.Equal(fullCount + 1, Batches.Count); } - private void LogMessages(WriteContext context) - { - Logs.Push(new LogMessage(context.LogLevel, context.Message, context.Exception)); - } + private void LogMessages(WriteContext context) => Logs.Push(new LogMessage(context.LogLevel, context.Message, context.Exception)); [DebuggerDisplay("{Message,nq}")] - private class LogMessage : IEquatable + private class LogMessage { public LogMessage(LogLevel logLevel, string message, Exception exception) { @@ -253,16 +250,6 @@ public LogMessage(LogLevel logLevel, string message, Exception exception) public LogLevel LogLevel { get; set; } public string Message { get; set; } public Exception Exception { get; set; } - - public override bool Equals(object obj) => Equals(obj as LogMessage); - - public bool Equals([AllowNull] LogMessage other) => - other != null && - LogLevel == other.LogLevel && - Message == other.Message && - EqualityComparer.Default.Equals(Exception, other.Exception); - - public override int GetHashCode() => HashCode.Combine(LogLevel, Message, Exception); } private class Batch From 84493f0b95219edb47d4b4c5e0d20ea7012b01f5 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Thu, 1 Aug 2019 11:28:58 -0700 Subject: [PATCH 03/11] More cleanups --- .../E2ETest/ServerExecutionTests/InteropReliabilityTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs b/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs index 8b46ee7e0a12..cf1be22179ec 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs @@ -507,15 +507,13 @@ public async Task EventHandlerThrowsSyncExceptionTerminatesTheCircuit() sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name, wc.Exception)); // Act - await Client.ClickAsync("event-handler-throw-sync"); + await Client.ClickAsync("event-handler-throw-sync", expectRenderBatch: false); Assert.Contains( logEvents, e => LogLevel.Warning == e.logLevel && "UnhandledExceptionInCircuit" == e.eventIdName && "Handler threw an exception" == e.exception.Message); - - await ValidateClientKeepsWorking(Client, batches); } private Task ValidateClientKeepsWorking(BlazorClient Client, List<(int, int, byte[])> batches) => From 733a4469a82b2c217c3afeeb8fac02a0f9e4c9f2 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Fri, 2 Aug 2019 16:18:00 +0200 Subject: [PATCH 04/11] Apply suggestions from code review Suggestions from @stevesandersonms Co-Authored-By: Steve Sanderson --- src/Components/Server/src/Circuits/RemoteRenderer.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index 41444e2b92a4..ab5b7ae8d1b9 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -30,7 +30,7 @@ internal class RemoteRenderer : HtmlRenderer private long _nextRenderId = 1; private bool _disposing = false; private bool _queueIsFullNotified; - private ConcurrentQueue _unacknowledgedRenderBatches = new ConcurrentQueue(); + private readonly ConcurrentQueue _unacknowledgedRenderBatches = new ConcurrentQueue(); /// /// Notifies when a rendering exception occured. @@ -92,11 +92,11 @@ protected override void ProcessRenderQueue() // If we got here it means we are at max capacity, so we don't want to actually process the queue, // as we have a client that is not acknowledging render batches fast enough (something we consider needs // to be fast). - // The result is somethign as follows: + // The result is something as follows: // Lets imagine an extreme case where the server produces a new batch every milisecond. // Lets say the client is able to ACK a batch every 100 miliseconds. - // When the app starts the client might see the sequence 0.000->0.{MAXUnacknowledgeRenderBatches} and then - // after 100 miliseconds it sees it jump to 0.1xx, then to 0.2xx where xx is something between {0..99} the + // When the app starts the client might see the sequence 0->(MaxUnacknowledgedRenderBatches-1) and then + // after 100 miliseconds it sees it jump to 1xx, then to 2xx where xx is something between {0..99} the // reason for this is that the server slows down rendering new batches to as fast as the client can consume // them. // Similarly, if a client were to send events at a faster pace than the server can consume them, the server @@ -287,7 +287,7 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) while (_unacknowledgedRenderBatches.TryPeek(out nextUnacknowledgedBatch) && nextUnacknowledgedBatch.BatchId <= incomingBatchId) { lastBatchId = nextUnacknowledgedBatch.BatchId; - // At this point the queue is no longer full, we have at least emptied one slot, so we allow a further + // At this point the queue is definitely not full, we have at least emptied one slot, so we allow a further // full queue log entry the next time it fills up. _queueIsFullNotified = false; _unacknowledgedRenderBatches.TryDequeue(out _); From e20172521901b53f3c002b19739bcdd612df7551 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Fri, 2 Aug 2019 07:57:11 -0700 Subject: [PATCH 05/11] Fix compilation error --- .../Server/src/Circuits/RemoteRenderer.cs | 2 +- .../Server/test/Circuits/RemoteRendererTest.cs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index ab5b7ae8d1b9..dc018853c61b 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -27,10 +27,10 @@ internal class RemoteRenderer : HtmlRenderer private readonly CircuitOptions _options; private readonly RendererRegistry _rendererRegistry; private readonly ILogger _logger; + internal readonly ConcurrentQueue _unacknowledgedRenderBatches = new ConcurrentQueue(); private long _nextRenderId = 1; private bool _disposing = false; private bool _queueIsFullNotified; - private readonly ConcurrentQueue _unacknowledgedRenderBatches = new ConcurrentQueue(); /// /// Notifies when a rendering exception occured. diff --git a/src/Components/Server/test/Circuits/RemoteRendererTest.cs b/src/Components/Server/test/Circuits/RemoteRendererTest.cs index 8d19d72f1a92..2c52e52d7a5a 100644 --- a/src/Components/Server/test/Circuits/RemoteRendererTest.cs +++ b/src/Components/Server/test/Circuits/RemoteRendererTest.cs @@ -49,7 +49,7 @@ public void WritesAreBufferedWhenTheClientIsOffline() component.TriggerRender(); // Assert - Assert.Equal(2, renderer.UnacknowledgedRenderBatches.Count); + Assert.Equal(2, renderer._unacknowledgedRenderBatches.Count); } [Fact] @@ -153,7 +153,7 @@ public async Task OnRenderCompletedAsync_DoesNotThrowWhenReceivedDuplicateAcks() }; // This produces an additional batch (id = 3) trigger.TriggerRender(); - var originallyQueuedBatches = renderer.UnacknowledgedRenderBatches.Count; + var originallyQueuedBatches = renderer._unacknowledgedRenderBatches.Count; // Act offlineClient.Transfer(onlineClient.Object, "new-connection"); @@ -216,7 +216,7 @@ public async Task OnRenderCompletedAsync_DoesNotThrowWhenThereAreNoPendingBatche }; // This produces an additional batch (id = 3) trigger.TriggerRender(); - var originallyQueuedBatches = renderer.UnacknowledgedRenderBatches.Count; + var originallyQueuedBatches = renderer._unacknowledgedRenderBatches.Count; // Act offlineClient.Transfer(onlineClient.Object, "new-connection"); @@ -279,7 +279,7 @@ public async Task ConsumesAllPendingBatchesWhenReceivingAHigherSequenceBatchId() }; // This produces an additional batch (id = 3) trigger.TriggerRender(); - var originallyQueuedBatches = renderer.UnacknowledgedRenderBatches.Count; + var originallyQueuedBatches = renderer._unacknowledgedRenderBatches.Count; // Act var exceptions = new List(); @@ -295,7 +295,7 @@ public async Task ConsumesAllPendingBatchesWhenReceivingAHigherSequenceBatchId() // Assert Assert.Empty(exceptions); - Assert.Empty(renderer.UnacknowledgedRenderBatches); + Assert.Empty(renderer._unacknowledgedRenderBatches); } [Fact] @@ -336,7 +336,7 @@ public async Task ThrowsIfWeReceivedAnAcknowledgeForANeverProducedBatch() }; // This produces an additional batch (id = 3) trigger.TriggerRender(); - var originallyQueuedBatches = renderer.UnacknowledgedRenderBatches.Count; + var originallyQueuedBatches = renderer._unacknowledgedRenderBatches.Count; // Act var exceptions = new List(); @@ -373,7 +373,7 @@ public async Task PrerendersMultipleComponentsSuccessfully() // Assert Assert.Equal(0, first.ComponentId); Assert.Equal(1, second.ComponentId); - Assert.Equal(2, renderer.UnacknowledgedRenderBatches.Count); + Assert.Equal(2, renderer._unacknowledgedRenderBatches.Count); } private RemoteRenderer GetRemoteRenderer(IServiceProvider serviceProvider, CircuitClientProxy circuitClientProxy) From b2eb95b537bb1283281de78242bf631f5fad90e2 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Mon, 5 Aug 2019 07:09:15 -0700 Subject: [PATCH 06/11] Address feedback --- .../Components/src/Rendering/Renderer.cs | 3 ++- .../Server/src/Circuits/RemoteRenderer.cs | 18 +++--------------- yarn.lock | 4 ++++ 3 files changed, 9 insertions(+), 16 deletions(-) create mode 100644 yarn.lock diff --git a/src/Components/Components/src/Rendering/Renderer.cs b/src/Components/Components/src/Rendering/Renderer.cs index 0ec8c23c32e9..cdf13506dd9a 100644 --- a/src/Components/Components/src/Rendering/Renderer.cs +++ b/src/Components/Components/src/Rendering/Renderer.cs @@ -403,6 +403,8 @@ private ComponentState GetOptionalComponentState(int componentId) /// protected virtual void ProcessRenderQueue() { + EnsureSynchronizationContext(); + _isBatchInProgress = true; var updateDisplayTask = Task.CompletedTask; @@ -410,7 +412,6 @@ protected virtual void ProcessRenderQueue() { if (_batchBuilder.ComponentRenderQueue.Count == 0) { - Log.NoPendingComponentRenderRequests(_logger); return; } diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index dc018853c61b..951f7bc767c8 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Concurrent; -using System.Diagnostics; using System.Linq; using System.Text.Encodings.Web; using System.Threading; @@ -30,7 +29,6 @@ internal class RemoteRenderer : HtmlRenderer internal readonly ConcurrentQueue _unacknowledgedRenderBatches = new ConcurrentQueue(); private long _nextRenderId = 1; private bool _disposing = false; - private bool _queueIsFullNotified; /// /// Notifies when a rendering exception occured. @@ -105,21 +103,11 @@ protected override void ProcessRenderQueue() // We should never see UnacknowledgedRenderBatches.Count > _options.MaxBufferedUnacknowledgedRenderBatches // But if we do, it's safer to simply disable the rendering in that case too instead of allowing batches to - // keep piling up. - if (!_queueIsFullNotified) - { - // We skip logging after the first time we run into this situation to avoid filling the logs with - // entries on high frequency render scenarios. - // We try our best to log the queue is full at most once, although we don't guarantee it due to - // concurrency. - // When we complete a render we set this flag to false, so the next time the queue gets filled it - // also shows up. - _queueIsFullNotified = true; - Log.FullUnacknowledgedRenderBatchesQueue(_logger); - } + Log.FullUnacknowledgedRenderBatchesQueue(_logger); return; } + base.ProcessRenderQueue(); } @@ -303,7 +291,7 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) // Normally we will not have pending renders, but it might happen that we reached the limit of // available buffered renders and new renders got queued. - // Invke ProcessBufferedRenderRequests so that we might produce any additional batch that is + // Invoke ProcessBufferedRenderRequests so that we might produce any additional batch that is // missing. // Its also safe to use the discard as ProcessRenderQueue won't throw. _ = Dispatcher.InvokeAsync(() => ProcessRenderQueue()); diff --git a/yarn.lock b/yarn.lock new file mode 100644 index 000000000000..fb57ccd13afb --- /dev/null +++ b/yarn.lock @@ -0,0 +1,4 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + From 2a555e5bf70382dac9d0bba51267415537a195f2 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Mon, 5 Aug 2019 07:58:29 -0700 Subject: [PATCH 07/11] Update public API --- src/Components/Components/src/Rendering/Renderer.cs | 12 ++++++++---- src/Components/Server/src/Circuits/RemoteRenderer.cs | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Components/Components/src/Rendering/Renderer.cs b/src/Components/Components/src/Rendering/Renderer.cs index cdf13506dd9a..a1eddfd91f38 100644 --- a/src/Components/Components/src/Rendering/Renderer.cs +++ b/src/Components/Components/src/Rendering/Renderer.cs @@ -243,7 +243,7 @@ public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo fiel // Since the task has yielded - process any queued rendering work before we return control // to the caller. - ProcessRenderQueue(); + ProcessPendingRender(); } // Task completed synchronously or is still running. We already processed all of the rendering @@ -399,12 +399,16 @@ private ComponentState GetOptionalComponentState(int componentId) : null; /// - /// Processses pending renders requests from components. + /// Processses pending renders requests from components if there are any. /// - protected virtual void ProcessRenderQueue() + protected virtual void ProcessPendingRender() { - EnsureSynchronizationContext(); + ProcessRenderQueue(); + } + private void ProcessRenderQueue() + { + EnsureSynchronizationContext(); _isBatchInProgress = true; var updateDisplayTask = Task.CompletedTask; diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index 951f7bc767c8..b265e91e5bcb 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -83,7 +83,7 @@ public Task AddComponentAsync(Type componentType, string domElementSelector) return RenderRootComponentAsync(componentId); } - protected override void ProcessRenderQueue() + protected override void ProcessPendingRender() { if (_unacknowledgedRenderBatches.Count >= _options.MaxBufferedUnacknowledgedRenderBatches) { From 56397a2c5a588b171fb295f82966c7af85ecae32 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Mon, 5 Aug 2019 08:53:36 -0700 Subject: [PATCH 08/11] Cleanups --- .../Components/src/Rendering/Renderer.cs | 4 +- .../Server/src/Circuits/RemoteRenderer.cs | 12 +- src/Components/Server/src/ComponentHub.cs | 2 +- .../test/Circuits/RemoteRendererTest.cs | 96 +++++++++-- .../RemoteRendererBufferLimitTest.cs | 155 +----------------- yarn.lock | 4 - 6 files changed, 96 insertions(+), 177 deletions(-) delete mode 100644 yarn.lock diff --git a/src/Components/Components/src/Rendering/Renderer.cs b/src/Components/Components/src/Rendering/Renderer.cs index a1eddfd91f38..048ba9a868ad 100644 --- a/src/Components/Components/src/Rendering/Renderer.cs +++ b/src/Components/Components/src/Rendering/Renderer.cs @@ -334,7 +334,7 @@ internal void AssignEventHandlerId(ref RenderTreeFrame frame) /// /// The ID of the component to render. /// A that will supply the updated UI contents. - protected internal virtual void AddToRenderQueue(int componentId, RenderFragment renderFragment) + internal void AddToRenderQueue(int componentId, RenderFragment renderFragment) { EnsureSynchronizationContext(); @@ -351,7 +351,7 @@ protected internal virtual void AddToRenderQueue(int componentId, RenderFragment if (!_isBatchInProgress) { - ProcessRenderQueue(); + ProcessPendingRender(); } } diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index b265e91e5bcb..46a003766cf9 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -108,7 +108,7 @@ protected override void ProcessPendingRender() return; } - base.ProcessRenderQueue(); + base.ProcessPendingRender(); } /// @@ -233,12 +233,12 @@ private async Task WriteBatchBytesAsync(UnacknowledgedRenderBatch pending) // disposed. } - public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) + public Task OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) { if (_disposing) { // Disposing so don't do work. - return; + return Task.CompletedTask; } // When clients send acks we know for sure they received and applied the batch. @@ -267,6 +267,7 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) if (!_unacknowledgedRenderBatches.TryPeek(out var nextUnacknowledgedBatch) || incomingBatchId < nextUnacknowledgedBatch.BatchId) { Log.ReceivedDuplicateBatchAck(_logger, incomingBatchId); + return Task.CompletedTask; } else { @@ -277,7 +278,6 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) lastBatchId = nextUnacknowledgedBatch.BatchId; // At this point the queue is definitely not full, we have at least emptied one slot, so we allow a further // full queue log entry the next time it fills up. - _queueIsFullNotified = false; _unacknowledgedRenderBatches.TryDequeue(out _); ProcessPendingBatch(errorMessageOrNull, nextUnacknowledgedBatch); } @@ -286,7 +286,7 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) { HandleException( new InvalidOperationException($"Received an acknowledgement for batch with id '{incomingBatchId}' when the last batch produced was '{lastBatchId}'.")); - return; + return Task.CompletedTask; } // Normally we will not have pending renders, but it might happen that we reached the limit of @@ -294,7 +294,7 @@ public void OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) // Invoke ProcessBufferedRenderRequests so that we might produce any additional batch that is // missing. // Its also safe to use the discard as ProcessRenderQueue won't throw. - _ = Dispatcher.InvokeAsync(() => ProcessRenderQueue()); + return Dispatcher.InvokeAsync(() => ProcessPendingRender()); } } diff --git a/src/Components/Server/src/ComponentHub.cs b/src/Components/Server/src/ComponentHub.cs index 4da9a3391ef2..b559e590c3c5 100644 --- a/src/Components/Server/src/ComponentHub.cs +++ b/src/Components/Server/src/ComponentHub.cs @@ -242,7 +242,7 @@ public void OnRenderCompleted(long renderId, string errorMessageOrNull) } Log.ReceivedConfirmationForBatch(_logger, renderId); - CircuitHost.Renderer.OnRenderCompleted(renderId, errorMessageOrNull); + _ = CircuitHost.Renderer.OnRenderCompleted(renderId, errorMessageOrNull); } public void OnLocationChanged(string uri, bool intercepted) diff --git a/src/Components/Server/test/Circuits/RemoteRendererTest.cs b/src/Components/Server/test/Circuits/RemoteRendererTest.cs index 2c52e52d7a5a..b6475ba1fe53 100644 --- a/src/Components/Server/test/Circuits/RemoteRendererTest.cs +++ b/src/Components/Server/test/Circuits/RemoteRendererTest.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Components.Server.Circuits; using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.JSInterop; using Moq; @@ -52,6 +53,81 @@ public void WritesAreBufferedWhenTheClientIsOffline() Assert.Equal(2, renderer._unacknowledgedRenderBatches.Count); } + [Fact] + public void NotAcknowledgingRenders_ProducesBatches_UpToTheLimit() + { + var serviceProvider = new ServiceCollection().BuildServiceProvider(); + var renderer = (RemoteRenderer)GetHtmlRenderer(serviceProvider); + var component = new TestComponent(builder => + { + builder.OpenElement(0, "my element"); + builder.AddContent(1, "some text"); + builder.CloseElement(); + }); + + // Act + var componentId = renderer.AssignRootComponentId(component); + for (int i = 0; i < 20; i++) + { + component.TriggerRender(); + + } + + // Assert + Assert.Equal(10, renderer._unacknowledgedRenderBatches.Count); + } + + [Fact] + public async Task NoNewBatchesAreCreated_WhenThereAreNoPendingRenderRequestsFromComponents() + { + var serviceProvider = new ServiceCollection().BuildServiceProvider(); + var renderer = (RemoteRenderer)GetHtmlRenderer(serviceProvider); + var component = new TestComponent(builder => + { + builder.OpenElement(0, "my element"); + builder.AddContent(1, "some text"); + builder.CloseElement(); + }); + + // Act + var componentId = renderer.AssignRootComponentId(component); + for (var i = 0; i < 10; i++) + { + component.TriggerRender(); + } + + await renderer.OnRenderCompleted(2, null); + + // Assert + Assert.Equal(9, renderer._unacknowledgedRenderBatches.Count); + } + + + [Fact] + public async Task ProducesNewBatch_WhenABatchGetsAcknowledged() + { + var serviceProvider = new ServiceCollection().BuildServiceProvider(); + var renderer = (RemoteRenderer)GetHtmlRenderer(serviceProvider); + var i = 0; + var component = new TestComponent(builder => + { + builder.AddContent(0, $"Value {i}"); + }); + + // Act + var componentId = renderer.AssignRootComponentId(component); + for (i = 0; i < 20; i++) + { + component.TriggerRender(); + } + Assert.Equal(10, renderer._unacknowledgedRenderBatches.Count); + + await renderer.OnRenderCompleted(2, null); + + // Assert + Assert.Equal(10, renderer._unacknowledgedRenderBatches.Count); + } + [Fact] public async Task ProcessBufferedRenderBatches_WritesRenders() { @@ -84,7 +160,7 @@ public async Task ProcessBufferedRenderBatches_WritesRenders() var componentId = renderer.AssignRootComponentId(component); component.TriggerRender(); - renderer.OnRenderCompleted(2, null); + _ = renderer.OnRenderCompleted(2, null); @event.Reset(); firstBatchTCS.SetResult(null); @@ -102,7 +178,7 @@ public async Task ProcessBufferedRenderBatches_WritesRenders() foreach (var id in renderIds.ToArray()) { - renderer.OnRenderCompleted(id, null); + _ = renderer.OnRenderCompleted(id, null); } secondBatchTCS.SetResult(null); @@ -165,14 +241,14 @@ public async Task OnRenderCompletedAsync_DoesNotThrowWhenReceivedDuplicateAcks() }; // Receive the ack for the intial batch - renderer.OnRenderCompleted(2, null); + _ = renderer.OnRenderCompleted(2, null); // Receive the ack for the second batch - renderer.OnRenderCompleted(3, null); + _ = renderer.OnRenderCompleted(3, null); firstBatchTCS.SetResult(null); secondBatchTCS.SetResult(null); // Repeat the ack for the third batch - renderer.OnRenderCompleted(3, null); + _ = renderer.OnRenderCompleted(3, null); // Assert Assert.Empty(exceptions); @@ -228,14 +304,14 @@ public async Task OnRenderCompletedAsync_DoesNotThrowWhenThereAreNoPendingBatche }; // Receive the ack for the intial batch - renderer.OnRenderCompleted(2, null); + _ = renderer.OnRenderCompleted(2, null); // Receive the ack for the second batch - renderer.OnRenderCompleted(2, null); + _ = renderer.OnRenderCompleted(2, null); firstBatchTCS.SetResult(null); secondBatchTCS.SetResult(null); // Repeat the ack for the third batch - renderer.OnRenderCompleted(3, null); + _ = renderer.OnRenderCompleted(3, null); // Assert Assert.Empty(exceptions); @@ -289,7 +365,7 @@ public async Task ConsumesAllPendingBatchesWhenReceivingAHigherSequenceBatchId() }; // Pretend that we missed the ack for the initial batch - renderer.OnRenderCompleted(3, null); + _ = renderer.OnRenderCompleted(3, null); firstBatchTCS.SetResult(null); secondBatchTCS.SetResult(null); @@ -345,7 +421,7 @@ public async Task ThrowsIfWeReceivedAnAcknowledgeForANeverProducedBatch() exceptions.Add(e); }; - renderer.OnRenderCompleted(4, null); + _ = renderer.OnRenderCompleted(4, null); firstBatchTCS.SetResult(null); secondBatchTCS.SetResult(null); diff --git a/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs b/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs index 297bd041e158..7377b28a9d52 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/RemoteRendererBufferLimitTest.cs @@ -5,7 +5,6 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading.Tasks; using Ignitor; @@ -14,7 +13,6 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; using Xunit; -using Xunit.Sdk; namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests { @@ -34,7 +32,6 @@ public RemoteRendererBufferLimitTest(AspNetSiteServerFixture serverFixture) Client = new BlazorClient() { DefaultLatencyTimeout = DefaultLatencyTimeout }; Client.RenderBatchReceived += (rendererId, id, data) => Batches.Add(new Batch(rendererId, id, data)); - Client.OnCircuitError += (error) => Errors.Add(error); Sink = _serverFixture.Host.Services.GetRequiredService(); Sink.MessageLogged += LogMessages; @@ -47,158 +44,8 @@ public RemoteRendererBufferLimitTest(AspNetSiteServerFixture serverFixture) // We use a stack so that we can search the logs in reverse order private ConcurrentStack Logs { get; set; } = new ConcurrentStack(); - private IList Errors { get; set; } = new List(); - public TestSink Sink { get; private set; } - [Fact] - public async Task NoNewBatchesAreCreated_WhenThereAreNoPendingRenderRequestsFromComponents() - { - var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); - Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); - Assert.Single(Batches); - - // Act - await Client.SelectAsync("test-selector-select", "BasicTestApp.LimitCounterComponent"); - - // Assert - Assert.Equal(2, Batches.Count); - Assert.Contains(Logs.ToArray(), l => (LogLevel.Debug, "No pending component render requests.") == (l.LogLevel, l.Message)); - } - - [Fact] - public async Task NotAcknowledgingRenders_ProducesBatches_UpToTheLimit() - { - var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); - Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); - Assert.Single(Batches); - - // Act - Client.ConfirmRenderBatch = false; - var tcs = new TaskCompletionSource(); - void SignalOnBatchNumber(int batchId, int rendererId, byte[] data) - { - // We wait for the batch Id 11 because its 1 batch for the initial render + 10 after we stop - // unacknowledging batches. - if (batchId == 11) - { - tcs.TrySetResult(null); - } - } - Client.RenderBatchReceived += SignalOnBatchNumber; - - await Client.SelectAsync("test-selector-select", "BasicTestApp.HighFrequencyComponent"); - - await Task.WhenAny(Task.Delay(1000), tcs.Task); - - // Assert - // We check that the value is 9 because the ticker starts at 0, there is a max buffer of 10 unacknowledged - // batches and we don't acknowledge the initial batch to render the ticker component. - Assert.Contains(Client.FindElementById("tick-value").Children, c => c is TextNode tn && tn.TextContent == "9"); - Assert.Equal(11, Batches.Count); - } - - [Fact] - public async Task ProducesNewBatch_WhenABatchGetsAcknowledged() - { - // Arrange - var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); - Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); - Assert.Single(Batches); - - Client.ConfirmRenderBatch = false; - var tcs = new TaskCompletionSource(); - void SignalOnBatchNumber(int batchId, int rendererId, byte[] data) - { - // We wait for the batch Id 11 because its 1 batch for the initial render + 10 after we stop - // unacknowledging batches. - if (batchId == 11) - { - tcs.TrySetResult(null); - } - } - Client.RenderBatchReceived += SignalOnBatchNumber; - - await Client.SelectAsync("test-selector-select", "BasicTestApp.HighFrequencyComponent"); - - await Task.WhenAny(Task.Delay(1000), tcs.Task); - - Assert.Contains(Client.FindElementById("tick-value").Children, c => c is TextNode tn && tn.TextContent == "9"); - Assert.Equal(11, Batches.Count); - - // Act - await Client.ExpectRenderBatch(() => Client.ConfirmBatch(3)); // This is the batch after the initial batch - - // Assert - Assert.Contains( - Client.FindElementById("tick-value").Children, - c => c is TextNode tn && int.Parse(tn.TextContent) >= 10); - Assert.Equal(12, Batches.Count); - } - - [Fact] - public async Task ContinuesProducingBatches_UntilItStopsReceivingAcksAndResumesWhenItReceivesAcksAgain() - { - // Arrange - var baseUri = new Uri(_serverFixture.RootUri, "/subdir"); - Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); - Assert.Single(Batches); - - await Client.SelectAsync("test-selector-select", "BasicTestApp.HighFrequencyComponent"); - await Task.Delay(100); - - var tcs = new TaskCompletionSource(); - var foundLogMessage = false; - void SignalOnFullQueue(WriteContext context) - { - // We wait for the batch Id 11 because its 1 batch for the initial render + 10 after we stop - // unacknowledging batches. - if (context.Message == "The queue of unacknowledged render batches is full.") - { - tcs.TrySetResult(null); - foundLogMessage = true; - } - } - - Sink.MessageLogged+= SignalOnFullQueue; - Client.ConfirmRenderBatch = false; // This will stop acknowledging batches, so we should see a queue full message. - - await Task.WhenAny(Task.Delay(1000), tcs.Task); - - Sink.MessageLogged -= SignalOnFullQueue; - Assert.True(foundLogMessage, "Log entry 'The queue of unacknowledged render batches is full.' not found."); - - var currentCount = Batches.Count; - var lastBatchId = Batches[^1].Id; - var lastRenderedValue = ((TextNode)Client.FindElementById("tick-value").Children.Single()).TextContent; - // Act - Client.ConfirmRenderBatch = true; - - // This will resume the render batches. - await Client.ExpectRenderBatch(() => Client.ConfirmBatch(lastBatchId)); - - // An indeterminate amount of renders will happen in this time. - await Task.Delay(100); - - Sink.MessageLogged += SignalOnFullQueue; - tcs = new TaskCompletionSource(); - foundLogMessage = false; - Logs.Clear(); - - // This will cause the renders to stop. - Client.ConfirmRenderBatch = false; - await Task.WhenAny(Task.Delay(1000), tcs.Task); - - Sink.MessageLogged -= SignalOnFullQueue; - Assert.True(foundLogMessage, "Log entry 'The queue of unacknowledged render batches is full.' not found."); - - // Assert - Assert.True(lastBatchId + 10 < Batches[^1].Id, "We didn't produce more than 10 batches"); - - var finalRenderedValue = ((TextNode)Client.FindElementById("tick-value").Children.Single()).TextContent; - Assert.True(int.Parse(finalRenderedValue) > int.Parse(lastRenderedValue), "Ticker count didn't increase enough"); - } - [Fact] public async Task DispatchedEventsWillKeepBeingProcessed_ButUpdatedWillBeDelayedUntilARenderIsAcknowledged() { @@ -223,7 +70,7 @@ public async Task DispatchedEventsWillKeepBeingProcessed_ButUpdatedWillBeDelayed // Act await Client.ClickAsync("increment", expectRenderBatch: false); - Assert.Single(Logs, l => (LogLevel.Debug, "The queue of unacknowledged render batches is full.") == (l.LogLevel, l.Message)); + Assert.Contains(Logs, l => (LogLevel.Debug, "The queue of unacknowledged render batches is full.") == (l.LogLevel, l.Message)); Assert.Equal(fullCount, Batches.Count); Client.ConfirmRenderBatch = true; diff --git a/yarn.lock b/yarn.lock deleted file mode 100644 index fb57ccd13afb..000000000000 --- a/yarn.lock +++ /dev/null @@ -1,4 +0,0 @@ -# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. -# yarn lockfile v1 - - From 9ea28031ae035753faa42ce3804dcbd28dc0d5e9 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Mon, 5 Aug 2019 09:09:42 -0700 Subject: [PATCH 09/11] Cleanups --- .../BasicTestApp/HighFrequencyComponent.razor | 29 ------------------- .../test/testassets/BasicTestApp/Index.razor | 1 - 2 files changed, 30 deletions(-) delete mode 100644 src/Components/test/testassets/BasicTestApp/HighFrequencyComponent.razor diff --git a/src/Components/test/testassets/BasicTestApp/HighFrequencyComponent.razor b/src/Components/test/testassets/BasicTestApp/HighFrequencyComponent.razor deleted file mode 100644 index 50daa848bd1a..000000000000 --- a/src/Components/test/testassets/BasicTestApp/HighFrequencyComponent.razor +++ /dev/null @@ -1,29 +0,0 @@ -@page "/highfrequency" -@implements IDisposable -@using System.Threading - -

High frequency render component

-

This component renders updates at a really high frequency. We use it to test the server-side limits on the amount - of pending batches we keep buffered in the server while we wait for a client to acknowledge them.

-

@tick

- -@code { - int tick; - private bool _isDisposed; - - protected override void OnInitialized() - { - Task.Run(() => InvokeAsync(async () => - { - while (!_isDisposed) - { - await Task.Delay(TimeSpan.FromMilliseconds(1)); - tick++; - StateHasChanged(); - } - })); - } - - - public void Dispose() => _isDisposed = true; -} diff --git a/src/Components/test/testassets/BasicTestApp/Index.razor b/src/Components/test/testassets/BasicTestApp/Index.razor index 79edea70fac4..e1d08a3d81b5 100644 --- a/src/Components/test/testassets/BasicTestApp/Index.razor +++ b/src/Components/test/testassets/BasicTestApp/Index.razor @@ -31,7 +31,6 @@ - From 3dc991232ebc22832a630c6b1d831f26366db4af Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Mon, 5 Aug 2019 09:12:14 -0700 Subject: [PATCH 10/11] Cleanups --- src/Components/Components/src/Rendering/Renderer.Log.cs | 8 -------- src/Components/Server/src/Circuits/RemoteRenderer.cs | 3 ++- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Components/Components/src/Rendering/Renderer.Log.cs b/src/Components/Components/src/Rendering/Renderer.Log.cs index f822b06f8b45..bd6580963267 100644 --- a/src/Components/Components/src/Rendering/Renderer.Log.cs +++ b/src/Components/Components/src/Rendering/Renderer.Log.cs @@ -25,9 +25,6 @@ internal static class Log private static readonly Action _handlingEvent = LoggerMessage.Define(LogLevel.Debug, new EventId(5, "HandlingEvent"), "Handling event {EventId} of type '{EventType}'"); - private static readonly Action _noPendingComponentRenderRequests = - LoggerMessage.Define(LogLevel.Debug, new EventId(6, "NoPendingComponentRenderRequests"), "No pending component render requests."); - public static void InitializingComponent(ILogger logger, ComponentState componentState, ComponentState parentComponentState) { if (logger.IsEnabled(LogLevel.Debug)) // This is almost always false, so skip the evaluations @@ -63,11 +60,6 @@ public static void HandlingEvent(ILogger logger, ulong eventHandlerId, { _handlingEvent(logger, eventHandlerId, eventArgs?.GetType().Name ?? "null", null); } - - public static void NoPendingComponentRenderRequests(ILogger logger) - { - _noPendingComponentRenderRequests(logger, null); - } } } } diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index 46a003766cf9..096ee33ff009 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -293,7 +293,8 @@ public Task OnRenderCompleted(long incomingBatchId, string errorMessageOrNull) // available buffered renders and new renders got queued. // Invoke ProcessBufferedRenderRequests so that we might produce any additional batch that is // missing. - // Its also safe to use the discard as ProcessRenderQueue won't throw. + + // We return the task in here, but the caller doesn't await it. return Dispatcher.InvokeAsync(() => ProcessPendingRender()); } } From e00bd3dd0828ac5ff36a0b53f9a174051f8ecba7 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Mon, 5 Aug 2019 09:46:25 -0700 Subject: [PATCH 11/11] Update ref assemblies --- .../ref/Microsoft.AspNetCore.Components.netstandard2.0.cs | 2 +- .../ref/Microsoft.AspNetCore.Components.Server.netcoreapp3.0.cs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs index 98b2644ab8bd..0d1890dfacb3 100644 --- a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs +++ b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs @@ -535,13 +535,13 @@ public abstract partial class Renderer : System.IDisposable public Renderer(System.IServiceProvider serviceProvider, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) { } public abstract Microsoft.AspNetCore.Components.Dispatcher Dispatcher { get; } public event System.UnhandledExceptionEventHandler UnhandledSynchronizationException { add { } remove { } } - protected internal virtual void AddToRenderQueue(int componentId, Microsoft.AspNetCore.Components.RenderFragment renderFragment) { } protected internal int AssignRootComponentId(Microsoft.AspNetCore.Components.IComponent component) { throw null; } public virtual System.Threading.Tasks.Task DispatchEventAsync(ulong eventHandlerId, Microsoft.AspNetCore.Components.Rendering.EventFieldInfo fieldInfo, System.EventArgs eventArgs) { throw null; } public void Dispose() { } protected virtual void Dispose(bool disposing) { } protected abstract void HandleException(System.Exception exception); protected Microsoft.AspNetCore.Components.IComponent InstantiateComponent(System.Type componentType) { throw null; } + protected virtual void ProcessPendingRender() { } protected System.Threading.Tasks.Task RenderRootComponentAsync(int componentId) { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] protected System.Threading.Tasks.Task RenderRootComponentAsync(int componentId, Microsoft.AspNetCore.Components.ParameterView initialParameters) { throw null; } diff --git a/src/Components/Server/ref/Microsoft.AspNetCore.Components.Server.netcoreapp3.0.cs b/src/Components/Server/ref/Microsoft.AspNetCore.Components.Server.netcoreapp3.0.cs index 7a89318f43e3..ce7b6a1d26a3 100644 --- a/src/Components/Server/ref/Microsoft.AspNetCore.Components.Server.netcoreapp3.0.cs +++ b/src/Components/Server/ref/Microsoft.AspNetCore.Components.Server.netcoreapp3.0.cs @@ -33,6 +33,7 @@ public CircuitOptions() { } public int DisconnectedCircuitMaxRetained { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public System.TimeSpan DisconnectedCircuitRetentionPeriod { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public System.TimeSpan JSInteropDefaultCallTimeout { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public int MaxBufferedUnacknowledgedRenderBatches { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } } } namespace Microsoft.AspNetCore.Components.Server.Circuits