Skip to content
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

[storage-blob][APPENDBLOCK] Memory Leak #4964

Closed
1 task
sdnetwork opened this issue Sep 3, 2019 · 9 comments
Closed
1 task

[storage-blob][APPENDBLOCK] Memory Leak #4964

sdnetwork opened this issue Sep 3, 2019 · 9 comments
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage Storage Service (Queues, Blobs, Files)

Comments

@sdnetwork
Copy link

  • Package Name: @azure/storage-blob
  • Package Version: 12.0.0-preview.2
  • Operating system: ALL
  • nodejs
    • version: 10.15.3

Describe the bug

when you append a block to appendblob, the memory increase at each append to finally reach the limit of process

To Reproduce
Steps to reproduce the behavior:

  1. just append a block to an appendblob
@loarabia loarabia added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage Storage Service (Queues, Blobs, Files) labels Sep 4, 2019
@himadrinath
Copy link

also seeing this with uploadFileToBlockBlob with blockBlobURL memory increase when uploading and still holding their after upload finished

@jeremymeng
Copy link
Member

@sdnetwork @himadrinath Thanks for the feedback! We will look into it.

@jeremymeng
Copy link
Member

@himadrinath Are you seeing the similar behavior with the released v10 version of the library?

@himadrinath
Copy link

@jeremymeng after update to 10.5.0 looks like solved the problem

@jeremymeng
Copy link
Member

Thanks @himadrinath for verifying it. So the issue only happens when using preview version. We are looking into it.

@jeremymeng
Copy link
Member

It looks like we always create new agents in nodeFetchClient.prepareRequest() when there's no proxy settings.

const agent = httpRequest.url.startsWith("https") ? new https.Agent(options) : new http.Agent(options);

This returned requestInit is used to combine with the original request:

https://github.com/azure/azure-sdk-for-js/blob/master/sdk/core/core-http/lib/fetchHttpClient.ts#L112

Maybe we should check httpRequest.agent too before creating new agents.

@jeremymeng
Copy link
Member

Maybe we should check httpRequest.agent too before creating new agents.

Never mind, it could be undefined then a global agent might be used.

@jeremymeng
Copy link
Member

The root cause of this issue is #5398, which is exposed by a fix in WebResource.clone() to pass keepAlive setting to clones.

As we use clones of the original WebResource to make requests, before the fix KeepAlivePolicy would never wor since the setting is not passed. After the fix, KeepAlivePolicy would not work as expected either since we are creating a new agent for each request.

While we are working on a proper fix for #5398, I made a change in PR #5396 to not passing keepAlive setting to clones so we don't have memory leaks.

@ghost
Copy link

ghost commented Oct 4, 2019

Thanks for working with Microsoft on GitHub! Tell us how you feel about your experience using the reactions on this comment.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

6 participants