Skip to content

Commit

Permalink
#827 #1679 Improve performance of Request Mapper (#1724)
Browse files Browse the repository at this point in the history
* using ArrayPool<byte> instead of memorystream CopyToAsync

* Maybe this could be a solution. It should be checked. Adding StreamHttpContent (yarp)

* little bit of cleanup here

* Avoiding ToLower() in IsSupportedHeader, using StringComparer.OrdinalIgnoreCase

* for smaller payloads, avoid allocating a buffer that is larger than the announced content length

* adding some unit tests for stream http content tests

* typo

* GivenThereIsAPossiblyBrokenServiceRunningOn

* Some code refactorings after code review. There are still some todos, but providing some more improvements, removing the exception handling from RequestMapper. It's handled in the middleware now, we will need to analyze this in detail.

* Some code cleanup

* removing some commented code bits

* adding more information in InvalidOperationException

* changing some methods signatures in request mapper, making them static.

* code review

* Update gotchas.rst

* Update notsupported.rst

* Update gotchas.rst

Add "Maximum request body size" notes

* With this fix, the system is able to handle content with size > 2 Gb

* adding new benchmarks, generating the payloads on the fly, from 1KB to 1024MB

* Review PayloadBenchmarks

* valid JSON

* should make sure the payloads directory exists

* Payloads folder name should match test method name

---------

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>
  • Loading branch information
ggnaegi and raman-m authored Dec 13, 2023
1 parent 6b1f332 commit a17242d
Show file tree
Hide file tree
Showing 15 changed files with 1,220 additions and 770 deletions.
42 changes: 39 additions & 3 deletions docs/introduction/gotchas.rst
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
Gotchas
=============


Many errors and incidents (gotchas) are related to web server hosting scenarios.
Please review deployment and web hosting common user scenarios below depending on your web server.

IIS
-----
---

Microsoft Learn: `Host ASP.NET Core on Windows with IIS <https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/iis/>`_

We do not recommend to deploy Ocelot app to IIS environments, but if you do, keep in mind the gotchas below.
We **do not** recommend to deploy Ocelot app to IIS environments, but if you do, keep in mind the gotchas below.

* When using ASP.NET Core 2.2+ and you want to use In-Process hosting, replace ``UseIISIntegration()`` with ``UseIIS()``, otherwise you will get startup errors.

Expand All @@ -23,3 +26,36 @@ Probably you will find a ready solution by Ocelot community members.
Finally, we have special label |IIS| for all IIS related objects. Feel free to put this label onto issues, PRs, discussions, etc.

.. |IIS| image:: https://img.shields.io/badge/-IIS-c5def5.svg

Kestrel
-------

Microsoft Learn: `Kestrel web server in ASP.NET Core <https://learn.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel>`_

We **do** recommend to deploy Ocelot app to self-hosting environments, aka Kestrel vs Docker.
We try to optimize Ocelot web app for Kestrel & Docker hosting scenarios, but keep in mind the following gotchas.

* **Upload and download large files**, proxying the content through the gateway. It is strange when you pump large (static) files using the gateway.
We believe that your client apps should have direct integration to (static) files persistent storages and services: remote & destributed file systems, CDNs, static files & blob storages, etc.
We **do not** recommend to pump large files (100Mb+ or even larger 1GB+) using gateway because of performance reasons: consuming memory and CPU, long delay times, producing network errors for downstream streaming, impact on other routes.

| The community constanly reports issues related to `large files <https://github.com/search?q=repo%3AThreeMammals%2FOcelot+%22large+file%22&type=issues>`_ (``application/octet-stream`` content type, :ref:`chunked-encoding`, etc.), see issues `749 <https://github.com/ThreeMammals/Ocelot/issues/749>`_, `1472 <https://github.com/ThreeMammals/Ocelot/issues/1472>`_.
If you still want to pump large files through an Ocelot gateway instance, we believe our PRs (`1724 <https://github.com/ThreeMammals/Ocelot/pull/1724>`_, `1769 <https://github.com/ThreeMammals/Ocelot/pull/1769>`_) will help resolve the issues and stabilize large content proxying.
In case of some errors, see the next point.

* **Maximum request body size**. ASP.NET ``HttpRequest`` behaves erroneously for application instances that do not have their Kestrel `MaxRequestBodySize <https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.server.kestrel.core.kestrelserverlimits.maxrequestbodysize>`_ option configured correctly and having pumped large files of unpredictable size which exceeds the limit.

| Please review these docs: `Maximum request body size | Configure options for the ASP.NET Core Kestrel web server <https://learn.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel/options#maximum-request-body-size>`_.
| As a quick fix, use this configuration recipe:
.. code-block:: csharp
builder.WebHost.ConfigureKestrel((context, serverOptions) =>
{
int myVideoFileMaxSize = 1_073_741_824; // assume your file storage has max file size as 1 GB (1_073_741_824)
int totalSize = myVideoFileMaxSize + 26_258_176; // and add some extra size
serverOptions.Limits.MaxRequestBodySize = totalSize; // 1_100_000_000 thus 1 GB file should not exceed the limit
});
Hope it helps.
4 changes: 3 additions & 1 deletion docs/introduction/notsupported.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ Not Supported
=============

Ocelot does not support...


.. _chunked-encoding:

Chunked Encoding
----------------

Expand Down
14 changes: 6 additions & 8 deletions src/Ocelot/Request/Mapper/IRequestMapper.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
using Microsoft.AspNetCore.Http;
using Ocelot.Configuration;
using Ocelot.Responses;

namespace Ocelot.Request.Mapper
{
public interface IRequestMapper
{
Task<Response<HttpRequestMessage>> Map(HttpRequest request, DownstreamRoute downstreamRoute);
}

namespace Ocelot.Request.Mapper;

public interface IRequestMapper
{
HttpRequestMessage Map(HttpRequest request, DownstreamRoute downstreamRoute);
}
187 changes: 82 additions & 105 deletions src/Ocelot/Request/Mapper/RequestMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,109 +2,86 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.Extensions.Primitives;
using Ocelot.Configuration;
using Ocelot.Responses;

namespace Ocelot.Request.Mapper
{
public class RequestMapper : IRequestMapper
{
private readonly string[] _unsupportedHeaders = { "host" };

public async Task<Response<HttpRequestMessage>> Map(HttpRequest request, DownstreamRoute downstreamRoute)
{
try
{
var requestMessage = new HttpRequestMessage
{
Content = await MapContent(request),
Method = MapMethod(request, downstreamRoute),
RequestUri = MapUri(request),
Version = downstreamRoute.DownstreamHttpVersion,
};

MapHeaders(request, requestMessage);

return new OkResponse<HttpRequestMessage>(requestMessage);
}
catch (Exception ex)
{
return new ErrorResponse<HttpRequestMessage>(new UnmappableRequestError(ex));
}
}

private static async Task<HttpContent> MapContent(HttpRequest request)
{
if (request.Body == null || (request.Body.CanSeek && request.Body.Length <= 0))
{
return null;
}

// Never change this to StreamContent again, I forgot it doesnt work in #464.
var content = new ByteArrayContent(await ToByteArray(request.Body));

if (!string.IsNullOrEmpty(request.ContentType))
{
content.Headers
.TryAddWithoutValidation("Content-Type", new[] { request.ContentType });
}

AddHeaderIfExistsOnRequest("Content-Language", content, request);
AddHeaderIfExistsOnRequest("Content-Location", content, request);
AddHeaderIfExistsOnRequest("Content-Range", content, request);
AddHeaderIfExistsOnRequest("Content-MD5", content, request);
AddHeaderIfExistsOnRequest("Content-Disposition", content, request);
AddHeaderIfExistsOnRequest("Content-Encoding", content, request);

return content;
}

private static void AddHeaderIfExistsOnRequest(string key, HttpContent content, HttpRequest request)
{
if (request.Headers.ContainsKey(key))
{
content.Headers
.TryAddWithoutValidation(key, request.Headers[key].ToArray());
}
}

private static HttpMethod MapMethod(HttpRequest request, DownstreamRoute downstreamRoute)
{
if (!string.IsNullOrEmpty(downstreamRoute?.DownstreamHttpMethod))
{
return new HttpMethod(downstreamRoute.DownstreamHttpMethod);
}

return new HttpMethod(request.Method);
}

private static Uri MapUri(HttpRequest request) => new(request.GetEncodedUrl());

private void MapHeaders(HttpRequest request, HttpRequestMessage requestMessage)
{
foreach (var header in request.Headers)
{
if (IsSupportedHeader(header))
{
requestMessage.Headers.TryAddWithoutValidation(header.Key, header.Value.ToArray());
}
}
}

private bool IsSupportedHeader(KeyValuePair<string, StringValues> header)
{
return !_unsupportedHeaders.Contains(header.Key.ToLower());
}

private static async Task<byte[]> ToByteArray(Stream stream)
{
await using (stream)
{
using (var memStream = new MemoryStream())
{
await stream.CopyToAsync(memStream);
return memStream.ToArray();
}
}
}
}

namespace Ocelot.Request.Mapper;

public class RequestMapper : IRequestMapper
{
private static readonly HashSet<string> UnsupportedHeaders = new(StringComparer.OrdinalIgnoreCase) { "host" };
private static readonly string[] ContentHeaders = { "Content-Length", "Content-Language", "Content-Location", "Content-Range", "Content-MD5", "Content-Disposition", "Content-Encoding" };

public HttpRequestMessage Map(HttpRequest request, DownstreamRoute downstreamRoute)
{
var requestMessage = new HttpRequestMessage
{
Content = MapContent(request),
Method = MapMethod(request, downstreamRoute),
RequestUri = MapUri(request),
Version = downstreamRoute.DownstreamHttpVersion,
};

MapHeaders(request, requestMessage);

return requestMessage;
}

private static HttpContent MapContent(HttpRequest request)
{
// TODO We should check if we really need to call HttpRequest.Body.Length
// But we assume that if CanSeek is true, the length is calculated without an important overhead
if (request.Body is null or { CanSeek: true, Length: <= 0 })
{
return null;
}

var content = new StreamHttpContent(request.HttpContext);

AddContentHeaders(request, content);

return content;
}

private static void AddContentHeaders(HttpRequest request, HttpContent content)
{
if (!string.IsNullOrEmpty(request.ContentType))
{
content.Headers
.TryAddWithoutValidation("Content-Type", new[] { request.ContentType });
}

// The performance might be improved by retrieving the matching headers from the request
// instead of calling request.Headers.TryGetValue for each used content header
var matchingHeaders = ContentHeaders.Where(header => request.Headers.ContainsKey(header));

foreach (var key in matchingHeaders)
{
if (!request.Headers.TryGetValue(key, out var value))
{
continue;
}

content.Headers.TryAddWithoutValidation(key, value.ToArray());
}
}

private static HttpMethod MapMethod(HttpRequest request, DownstreamRoute downstreamRoute) =>
!string.IsNullOrEmpty(downstreamRoute?.DownstreamHttpMethod) ?
new HttpMethod(downstreamRoute.DownstreamHttpMethod) : new HttpMethod(request.Method);

// TODO Review this method, request.GetEncodedUrl() could throw a NullReferenceException
private static Uri MapUri(HttpRequest request) => new(request.GetEncodedUrl());

private static void MapHeaders(HttpRequest request, HttpRequestMessage requestMessage)
{
foreach (var header in request.Headers)
{
if (IsSupportedHeader(header))
{
requestMessage.Headers.TryAddWithoutValidation(header.Key, header.Value.ToArray());
}
}
}

private static bool IsSupportedHeader(KeyValuePair<string, StringValues> header) =>
!UnsupportedHeaders.Contains(header.Key);
}
113 changes: 113 additions & 0 deletions src/Ocelot/Request/Mapper/StreamHttpContent.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
using Microsoft.AspNetCore.Http;
using System.Buffers;

namespace Ocelot.Request.Mapper;

public class StreamHttpContent : HttpContent
{
private const int DefaultBufferSize = 65536;
public const long UnknownLength = -1;
private readonly HttpContext _context;

public StreamHttpContent(HttpContext context)
{
_context = context ?? throw new ArgumentNullException(nameof(context));
}

protected override async Task SerializeToStreamAsync(Stream stream, TransportContext context,
CancellationToken cancellationToken)
=> await CopyAsync(_context.Request.Body, stream, Headers.ContentLength ?? UnknownLength, false,
cancellationToken);

protected override async Task SerializeToStreamAsync(Stream stream, TransportContext context)
=> await CopyAsync(_context.Request.Body, stream, Headers.ContentLength ?? UnknownLength, false,
CancellationToken.None);

protected override bool TryComputeLength(out long length)
{
length = -1;
return false;
}

// This is used internally by HttpContent.ReadAsStreamAsync(...)
protected override Task<Stream> CreateContentReadStreamAsync()
{
// Nobody should be calling this...
throw new NotImplementedException();
}

private static async Task CopyAsync(Stream input, Stream output, long announcedContentLength,
bool autoFlush, CancellationToken cancellation)
{
// For smaller payloads, avoid allocating a buffer that is larger than the announced content length
var minBufferSize = announcedContentLength != UnknownLength && announcedContentLength < DefaultBufferSize
? (int)announcedContentLength
: DefaultBufferSize;

var buffer = ArrayPool<byte>.Shared.Rent(minBufferSize);
long contentLength = 0;
try
{
while (true)
{
// Issue a zero-byte read to the input stream to defer buffer allocation until data is available.
// Note that if the underlying stream does not supporting blocking on zero byte reads, then this will
// complete immediately and won't save any memory, but will still function correctly.
var zeroByteReadTask = input.ReadAsync(Memory<byte>.Empty, cancellation);
if (zeroByteReadTask.IsCompletedSuccessfully)
{
// Consume the ValueTask's result in case it is backed by an IValueTaskSource
_ = zeroByteReadTask.Result;
}
else
{
// Take care not to return the same buffer to the pool twice in case zeroByteReadTask throws
var bufferToReturn = buffer;
buffer = null;
ArrayPool<byte>.Shared.Return(bufferToReturn);

await zeroByteReadTask;

buffer = ArrayPool<byte>.Shared.Rent(minBufferSize);
}

var read = await input.ReadAsync(buffer.AsMemory(), cancellation);
contentLength += read;

// Normally this is enforced by the server, but it could get out of sync if something in the proxy modified the body.
if (announcedContentLength != UnknownLength && contentLength > announcedContentLength)
{
throw new InvalidOperationException($"More data ({contentLength} bytes) received than the specified Content-Length of {announcedContentLength} bytes.");
}

// End of the source stream.
if (read == 0)
{
if (announcedContentLength == UnknownLength || contentLength == announcedContentLength)
{
return;
}
else
{
throw new InvalidOperationException($"Sent {contentLength} request content bytes, but Content-Length promised {announcedContentLength}.");
}
}

await output.WriteAsync(buffer.AsMemory(0, read), cancellation);
if (autoFlush)
{
// HttpClient doesn't always flush outgoing data unless the buffer is full or the caller asks.
// This is a problem for streaming protocols like WebSockets and gRPC.
await output.FlushAsync(cancellation);
}
}
}
finally
{
if (buffer != null)
{
ArrayPool<byte>.Shared.Return(buffer);
}
}
}
}
Loading

0 comments on commit a17242d

Please sign in to comment.