search

ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.28k stars 1.63k forks source link

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

Closed alahane-techtel closed 7 months ago

alahane-techtel commented 7 months ago

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

  1. Get the code from the repo mentioned below
  2. Open using VS2022
  3. Setup both the projects as startup.
  4. Debug them (F5)
  5. 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 commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

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 commented 7 months ago

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

alahane-techtel commented 7 months ago

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 commented 7 months ago

@alahane-techtel we are on it, i'm working on the unit and acceptance tests at the minute. https://github.com/ThreeMammals/Ocelot/pull/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 commented 7 months ago

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

raman-m commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

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 commented 7 months ago

@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 commented 6 months ago

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

raman-m commented 6 months ago

@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 commented 6 months ago

@alahane-techtel commented on Jan 29

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

alahane-techtel commented 6 months ago

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 https://github.com/ThreeMammals/Ocelot/issues/1924#issuecomment-1913764231

It was released https://github.com/ThreeMammals/Ocelot/pull/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 https://github.com/ThreeMammals/Ocelot/issues/1924#issuecomment-1961567535, or unsubscribe https://github.com/notifications/unsubscribe-auth/BFL2NR5WHIZIZFEYXRDYIALYVC25XAVCNFSM6AAAAABB4GORASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRRGU3DONJTGU . You are receiving this because you were mentioned.Message ID: @.***>