-
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
Download file API causes Gateway memory to rise by fileSize #1924
Comments
Dear @alahane-techtel ! We are not going to open any your archive files. |
Here's the repository: Btw, I'm Ashish. |
@alahane-techtel @raman-m We are currently testing a new approach, avoiding the http client full buffering. It might be the cause. |
ok. lets hope that it will fix it. At least we got one test scenario in my repo to check if the new approach works on that :) However, when you think the fix will be ready? just a rough estimate. like, couple of days? or weeks, or months? So that I can at least get some workaround ready. Because I need to fix the issue urgently. |
@alahane-techtel we are on it, i'm working on the unit and acceptance tests at the minute. #1824 |
Amazing!! I just checked. It works. No mem issue with this branch. Waiting eagerly for the release now :) Thanks a lot @ggnaegi |
Thank you, guys, for testing! |
@alahane-techtel commented on Jan 17:
Release is here as Nov-December'23 milestone. |
Since the actual release may take few more weeks, is it possible to create a pre-release meanwhile? I'll really appreciate it. |
* 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 <dotnet044@gmail.com>
No, it will take 2-3 days to release completing the current release milestone. So, if you want to test the version right away, then take develop branch code and compile a DLL by your own. NoteWe don't publish alpha, beta, pre-release versions. Because if feature is ready and delivered to develop branch then engineer can build a DLL from develop branch. There is no technical restrictions on such a "self-releasing". |
thanks @raman-m. Will the release take more time? more than 10 days now :( |
@alahane-techtel Our team was busy with testing in Production environment of our team member, @RaynaldM... Getting and watching in Prod now... |
|
Yeah. We upgraded already. It's been working fine since then. Thanks a
lot for your help. Honestly, I feel lucky that we encountered the issue,
and it was already fixed and ready to release. Kudos to your team. Thanks
again.
…On Sat, Feb 24, 2024, 02:48 Raman Maksimchuk ***@***.***> wrote:
@alahane-techtel <https://github.com/alahane-techtel> commented on Jan 29
<#1924 (comment)>
It was released <#1961> on Feb
13 as version 23.0
<https://github.com/ThreeMammals/Ocelot/releases/tag/23.0.0>
—
Reply to this email directly, view it on GitHub
<#1924 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFL2NR5WHIZIZFEYXRDYIALYVC25XAVCNFSM6AAAAABB4GORASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRRGU3DONJTGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Expected Behavior
When we download a file, the gateway size should remain mostly unchanged
Actual Behavior
when a file is downloaded, even the same file multiple times, each time, the Gateway process' size increases by roughly the downloaded file's size.
Steps to Reproduce the Problem
https://localhost:7279/Download/smallfile
Observe in the Task manager that everytime you download the file, the Gateway process's memory increases by around 25MB, which is the size of the downloaded file.
repository
https://github.com/alahane-techtel/DownloadViaGateway
The text was updated successfully, but these errors were encountered: