Download file API causes Gateway memory to rise by fileSize #1924

alahane-techtel · 2024-01-16T05:19:30Z

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

Get the code from the repo mentioned below
Open using VS2022
Setup both the projects as startup.
Debug them (F5)
from postman or browser, try this URL to download the file:
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

raman-m · 2024-01-16T13:34:35Z

Dear @alahane-techtel !
What's your full name?
Thanks for issue reporting! We know this problem and already worked on it. But the fix is waiting for the release.

We are not going to open any your archive files.
Please upload code to your Github account instead of zipping!

raman-m · 2024-01-16T13:43:57Z

@ggnaegi Please confirm that our fix for downloads/uploads of large files is ready! #1824 will fix, right?
We could possibly link this issue to current open PRs, so closing the issue officially respecting our dev process.

alahane-techtel · 2024-01-16T22:08:25Z

Here's the repository:
https://github.com/alahane-techtel/DownloadViaGateway
Also, updated the original post to include the repo instead of the zip.
Lemme know if this helps.

Btw, I'm Ashish.

ggnaegi · 2024-01-17T07:11:55Z

@alahane-techtel @raman-m We are currently testing a new approach, avoiding the http client full buffering. It might be the cause.

alahane-techtel · 2024-01-17T09:59:22Z

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.

ggnaegi · 2024-01-17T10:10:18Z

@alahane-techtel we are on it, i'm working on the unit and acceptance tests at the minute. #1824
But you could try it. It seems to me it solves the issue, if you compare the memory usage between the two benchmarks.
It's a question of weeks I would say.

alahane-techtel · 2024-01-17T11:31:48Z

Amazing!! I just checked. It works. No mem issue with this branch. Waiting eagerly for the release now :) Thanks a lot @ggnaegi

raman-m · 2024-01-17T12:08:43Z

Thank you, guys, for testing!
I believe #1824 will be merged to develop today or tomorrow, because I've added the PR to the current release.

raman-m · 2024-01-17T17:18:05Z

@alahane-techtel commented on Jan 17:

Amazing!! I just checked. It works. No mem issue with this branch. Waiting eagerly for the release now :) Thanks a lot @ggnaegi

Release is here as Nov-December'23 milestone.
Watch for this milestone activities and progress plz...

alahane-techtel · 2024-01-18T00:50:51Z

Since the actual release may take few more weeks, is it possible to create a pre-release meanwhile? I'll really appreciate it.

@raman-m

* 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>

raman-m · 2024-01-19T11:42:13Z

@alahane-techtel commented on Jan 18
Since the actual release may take few more weeks

No, it will take 2-3 days to release completing the current release milestone.
Linked PR is merged and this ticket is closed as implemented.

So, if you want to test the version right away, then take develop branch code and compile a DLL by your own.

Note

We 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".

alahane-techtel · 2024-01-28T23:48:22Z

thanks @raman-m. Will the release take more time? more than 10 days now :(

raman-m · 2024-01-29T10:30:46Z

@alahane-techtel Our team was busy with testing in Production environment of our team member, @RaynaldM...
Unfortunately previous week it was not able to deploy, so we've deployed develop version today's morning.

Getting and watching in Prod now...

raman-m · 2024-02-23T15:48:31Z

@alahane-techtel commented on Jan 29

It was released on Feb 13 as a part of version 23.0

alahane-techtel · 2024-02-24T01:12:44Z

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: ***@***.***>

raman-m added the waiting Waiting for answer to question or feedback from issue raiser label Jan 16, 2024

raman-m linked a pull request Jan 17, 2024 that will close this issue

#356 #695 #1924 Custom HttpMessageInvoker pooling #1824

Merged

raman-m added bug Identified as a potential bug Nov'23 November 2023 release and removed waiting Waiting for answer to question or feedback from issue raiser labels Jan 17, 2024

raman-m added this to the Nov-December'23 milestone Jan 17, 2024

raman-m assigned ggnaegi Jan 17, 2024

raman-m mentioned this issue Jan 17, 2024

#356 #695 #1924 Custom HttpMessageInvoker pooling #1824

Merged

raman-m closed this as completed in #1824 Jan 18, 2024

raman-m added the merged Issue has been merged to dev and is waiting for the next release label Jan 18, 2024

raman-m mentioned this issue Feb 13, 2024

Release 23.0 | Artifacts | +semver: breaking #1962

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Download file API causes Gateway memory to rise by fileSize #1924

Download file API causes Gateway memory to rise by fileSize #1924

alahane-techtel commented Jan 16, 2024 •

edited

raman-m commented Jan 16, 2024 •

edited

raman-m commented Jan 16, 2024 •

edited

alahane-techtel commented Jan 16, 2024 •

edited

ggnaegi commented Jan 17, 2024

alahane-techtel commented Jan 17, 2024

ggnaegi commented Jan 17, 2024 •

edited

alahane-techtel commented Jan 17, 2024

raman-m commented Jan 17, 2024 •

edited

raman-m commented Jan 17, 2024

alahane-techtel commented Jan 18, 2024

raman-m commented Jan 19, 2024

alahane-techtel commented Jan 28, 2024

raman-m commented Jan 29, 2024 •

edited

raman-m commented Feb 23, 2024 •

edited

alahane-techtel commented Feb 24, 2024 via email

Download file API causes Gateway memory to rise by fileSize #1924

Download file API causes Gateway memory to rise by fileSize #1924

Comments

alahane-techtel commented Jan 16, 2024 • edited

Expected Behavior

Actual Behavior

Steps to Reproduce the Problem

raman-m commented Jan 16, 2024 • edited

raman-m commented Jan 16, 2024 • edited

alahane-techtel commented Jan 16, 2024 • edited

ggnaegi commented Jan 17, 2024

alahane-techtel commented Jan 17, 2024

ggnaegi commented Jan 17, 2024 • edited

alahane-techtel commented Jan 17, 2024

raman-m commented Jan 17, 2024 • edited

raman-m commented Jan 17, 2024

alahane-techtel commented Jan 18, 2024

raman-m commented Jan 19, 2024

Note

alahane-techtel commented Jan 28, 2024

raman-m commented Jan 29, 2024 • edited

raman-m commented Feb 23, 2024 • edited

alahane-techtel commented Feb 24, 2024 via email

alahane-techtel commented Jan 16, 2024 •

edited

raman-m commented Jan 16, 2024 •

edited

raman-m commented Jan 16, 2024 •

edited

alahane-techtel commented Jan 16, 2024 •

edited

ggnaegi commented Jan 17, 2024 •

edited

raman-m commented Jan 17, 2024 •

edited

raman-m commented Jan 29, 2024 •

edited

raman-m commented Feb 23, 2024 •

edited