-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Streaming of request and response payloads #695
Comments
We are having the same issue with our API providing access to both uploading and downloading files. The request/response is buffered entirely into memory, causing heavy issues in our cloud environment with limited resources. |
Our team having the same issue too, when trying to upload large files (e.g.: video). |
Same problem here with content_type="text/event-stream". |
Yeh I agree this needs to be changed and has been annoying me ever since I made Ocelot! The problem is that Ocelot expects data to be there to do transformations etc anyone got any ideas how we get around this e.g. certain use cases use a stream, and only read it if you need to mess with the data? |
Today we hit this issue, any update on how will this be handled. Uploading a huge file (2GB +) to an API through Gateway throws an exception as |
Yes we should have a way of doing a stream if no transforms are required. If someone submits a PR for this we can accept it. |
@TomPallister when you say "if no transforms are required", could you guide me as to where the "transforms" are being handled within Ocelot? I'd like to submit a PR for this fix. |
Ocelot does a bunch of processing on the incoming HTTP request and HTTP response from the server such as working with headers, changing the path & content. All of this is optional based on configuration. Ocelot just assumes this functionality is being used so always reads the HTTP request / response stream into memory even if it isn't doing anything interesting with it. If the user doesn't want these features than it should be possible to not read the stream into memory. I think this might actually be a much more complicated refactor than I initially thought after you have asked me that question. I would need to look into this more to understand if it is possible with the current architecture and how you would do it with dotnet core. I guess its possible to stream the incoming HTTP request straight out to the downstream server and stream that response straight back to the client. However I don't know what the code needs to look like. Once this is worked out I think there are many places in the code that would need to be refactored you might even say it needs an architecture refactor :) |
I could only find two places that read the request body. In the
Which looks like a fix for #464. I have a fix for that in this unrelated Issue/MR #1432 And in the
The caching would only be enabled with Looks like response handling uses |
Is there any progress on this issue? It has been a long time. Streaming data is crucial part of our project. Is there at least a work around for this problem? |
I think you have to build it yourself and submit a PR. |
I didn't pay attention to dates and tags. I am sorry. I have made two changes to solve our problem. I don't think it is a generic solution so I haven't submited PR. Ocelot/src/Ocelot/Request/Mapper/RequestMapper.cs Lines 48 to 49 in 3ef6abd
changes to HttpContent content = request.ContentType == "application/octet-stream"
? new StreamContent(request.Body)
: new ByteArrayContent(await ToByteArray(request.Body)); I know that Ocelot/src/Ocelot/Requester/HttpClientWrapper.cs Lines 19 to 22 in 3ef6abd
changes to public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default)
{
return Client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
} With |
I've found that even with updates to remove full in-memory reads of the upstream response, with very large requests (like file downloads) the memory balloons up to 5x in some quick testing. So a 20Mb response adds 100Mb at runtime. I've had much better luck with https://github.com/microsoft/reverse-proxy. It does not use the HttpClient directly, uses HttpMessageInvoker with an auto-flushing stream copy that keeps memory low, even for very large responses. |
Is changing the Content in RequestMapper.cs from I've tried this to my route after changing the above but it still works without issue:
|
@TomPallister do you see any obvious issues with the solution proposed by @meliky? I thought of just making a copy of |
@meliky Have you had to do anything else to get it working on your end? For me, the connection seems to close automatically a few seconds after, and the streaming stops. |
@raman-m @CobusKruger |
Hi @ggnaegi ! After your design review of this issue, we will decide what to do next: attach this to #1724 and close this issue, or keep it open and prepare some quick PR with a fix... |
Sorry, guys! I've totally lost the thing... As a team, we will double check this issue in current release once again.
Tom lives in a "far-far away" Kingdom! 🤣 I'm Tom's representative. 😉
Updates are above! ☝️ Please make Ocelot DLLs temporarily compiling develop branch code, and let us know your test results please! |
Dear Simon, |
@RaynaldM Welcome to official testing stage! P.S.Technically we could develop manual tests in Ocelot.ManualTest project, run console session and pump large files. But you know I'm overloaded with current release activilities & tasks... I will think more, should we update manual tests or not... |
* first version * first working version of the new http client pool * Some cleanup, removing classes that aren't used * some more cleanup * forgot passing PooledConnectionLifetime * adding todo for connection pool and request timeouts * some code cleanup * ongoing process refactoring tests * a little mistake with big effects * Several refactorings, disposing http response message to ensure that the connection is freed. Moving errors from Polly provider to Errors\QoS. * providing some comments * adding response body benchmark * some minor changes in MessageInvokerPool. * using context.RequestAborted in responder middleware (copying the response body from downstream service to the http context) * Fix style warnings * code review * moving response.Content.ReadAsStreamAsync nearer to CopyToAsync with using. Making sure, that the content stream is disposed * HttpResponse.Content never returns null (from .net 5 onwards) * adding more unit tests (validating, log warning if passthrough certificate, cookies are returned, message invoker timeout) * adding a tolerance margin * adding new acceptance test, checking memory usage. Needs to be compared with current implementation first. * Review tests by @raman-m * Update src/Ocelot/Configuration/HttpHandlerOptions.cs * Update src/Ocelot/Middleware/DownstreamResponse.cs * Update src/Ocelot/Requester/MessageInvokerPool.cs * Update src/Ocelot/Requester/HttpExceptionToErrorMapper.cs * Update src/Ocelot/Responder/HttpContextResponder.cs * Update src/Ocelot/Middleware/DownstreamResponse.cs * Use null-coalescing operator for `Nullable<int>` obj * some modifications in the acceptance test, adding a tolerance of 1 Mb * finalizing content tests. Making sure, that the downstream response body is not copied by the api gateway. * adapting tolerances * Disable StyleCop rule SA1010 which is in conflict with collection initialization block vs whitespace. More: https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1010.md * Refactor Content tests --------- Co-authored-by: Raman Maksimchuk <[email protected]>
@raman-m |
@IamMartinPeek Actually say Thanks to @ggnaegi who is the author of new design of system core. 😉 He is Top 1 Contributor of the current 23rd release. But what's your project? I didn't find something related to Ocelot in your repos... Private project? |
Hi,
we tested ocelot in front of a dotnet core API for uploading files. The setup is WebApp (Razor Page) -> Ocelot -> API, each part running in its own linux docker container on Windows. Apparently, uploaded files are buffered entirely in memory in Ocelot before they are passed on to the backend. I recon responses are treated the same way, although we didn't test that yet. I couldn't find any configuration setting to enable request or response streaming.
Is there a setting i missed or might we be using Ocelot in a wrong way? If not, are there any plans to add this feature?
Thanks for any advice or help :)
The text was updated successfully, but these errors were encountered: