dotnet / dotnet/aspnetcore

Response Caching [ASP.NET Core 6 & 7]: cache populated for each concurrent connection rather than once

Open
#44,696 16 comments 0 reactions 1 assignee Claimed by @sebastienros View on GitHub
area-networking feature-response-caching Perf
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

### Is there an existing issue for this?

- [X] I have searched the existing issues

### Describe the bug

Response caching goes bust when multiple concurrent inbound connections arrive.

It appears that cache population begins with the first concurrent inbound request hitting the endpoint and not finishing until all 100 of them arrive. So each of them invokes the service, and GC can't cope with the load. The number 100 is chosen here almost arbitrarily. It can't serve C10k requests with response caching---it just crashes.

If, however, I run a preliminary request against the cached endpoint as described below in "Reproduction," then the cache gets populated and all 100 concurrent requests are ordinarily served from the cache and only 1 service invocation occurs. If instead I let that cache expire, then 101 service invocations take place.

When properly served from cache and the runtime VM is warmed up, my MacBook Air 2020 gives 3-4 GB/s throughput on that ASP.NET Core endpoint with connection reuse according to `bombardier`, whereas my Akka HTTP implementation with in-memory Caffeine cache gives 7-9 GB/s, all other things being equal, with up to tenfold smaller 99th percentile latency. I'm not sure where to look for the root cause of this performance mismatch.

More importantly, the runtime throws a `System.OutOfMemory` exception when the `Duration` is not long enough. I suppose multiple cache populations and evictions are taking place at the same time, they overlap, and RSS goes beyond 1 GiB, before shrinking while still serving requests from the then populated cache.

### Expected Behavior

I expect it to populate the cache on the first approaching request, regardless of how many identical inbound requests arrive at the same time or prior to cache population. All but one requests should be served from the cache. Now they all induce cache population. Perhaps simply one of them should be picked at random to populate the cache.

### Steps To Reproduce

I define a very simple controller,
```csharp
[ApiController]
[ApiVersion("1.2")]
[Route("v{version:apiVersion}/{controller}")]
[Produces(MediaTypeNames.Text.Html)]
public class WebsiteController : Controller
{
private readonly ILogger _logger;
private readonly HtmlSanitizerService _service; // must be registered, as a singleton
private uint _requestNumber = 0;

public WebsiteController (ILogger logger, HtmlSanitizerService service)
{
_logger = logger;
_service = service;
}
```

with just the following endpoint

```csharp
[HttpGet]
[Route("")]
[ProducesResponseType(StatusCodes.Status200OK)]
[ResponseCache(
Duration = 30
, Location = ResponseCacheLocation.Any
, NoStore = false
, VaryByQueryKeys = new[] { "address" }
)]
public async Task> GetSanitizedHtml ([FromQuery] string? address)
{ ... }
```

which calls a service. Nothing fancy.

Here's the service that gets invoked

```csharp
public class HtmlSanitizerService
{
private readonly IHttpClientFactory _httpClientFactory;
private readonly HttpClient _httpClient;
private readonly IHtmlSanitizer _sanitizer;
private readonly ILogger _logger;

private uint _requestNumber = 0;
private uint _responseNumber = 0;
public HtmlSanitizerService (IHttpClientFactory httpClientFactory, ILogger logger)
{
_logger = logger;
_logger.LogDebug("HtmlSanitizerService being initialized...");

_httpClientFactory = httpClientFactory;
_httpClient = _httpClientFactory.CreateClient("name");

_sanitizer = new HtmlSanitizer();

_logger.LogDebug("HtmlSanitizerService initialized...");
}

public async Task SanitizeAsync (Uri uri)
{
_requestNumber++;

_logger.LogDebug("> #{RequestNumber} request / HtmlSanitizerService.SanitizeAsync being executed...", _requestNumber);
var html = await _httpClient.GetStringAsync(uri);
var htmlSanitized = _sanitizer.SanitizeDocument(html);

_responseNumber++;
_logger.LogDebug("< #{ResponseNumber} response / HtmlSanitizerService.SanitizeAsync complete...", _responseNumber);
return htmlSanitized;
}
}
```

Now if I run a load test on this endpoint on Kestrel,
```sh
bombardier -c 100 -d 10s "http://localhost:5104/website?address=localhost:8080"
```

I observe exactly 100 service invocations (each of which is an expensive operation). To prevent this from happening all I need to do is run a single preliminary request against this endpoint, e.g., something like
```sh
curl -v "http://localhost:5104/website?address=localhost:8080" 1>/dev/null
```
which populates the cache, so that the same 100 concurrent connections immediately afterwards are served from cache and no additional invocations are taking place.

### Exceptions (if any)

`System.OutOfMemory` from `Ganss.Xss.HtmlSanitizer` (inside a singleton service) due to multiple concurrent invocations

### .NET Version

6 and 7-rc2

### Anything else?

ASP.NET Core 6.0 and 7.0-rc2

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.