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

memory leak in firestore().doc().set() #2372

Closed
zeevl opened this issue Nov 19, 2019 · 10 comments
Closed

memory leak in firestore().doc().set() #2372

zeevl opened this issue Nov 19, 2019 · 10 comments

Comments

@zeevl
Copy link

zeevl commented Nov 19, 2019

[REQUIRED] Describe your environment

  • Operating System version: macOS 10.15.1
  • Browser version: n/a (node 10.15 w/ jest 24)
  • Firebase SDK version: 7.4.0
  • Firebase Product: database

[REQUIRED] Describe the problem

jest --detect-leaks reports memory leaks with firestore().doc().set()

Steps to reproduce:

git clone https://github.com/zeevl/firestore-set-memleak
cd firestore-set-memleak
vim test.test.js
# add firebase credentials to initalizeApp()
yarn install
npx jest --detect-leaks

Expected: success
Result: memory leaks detected

@rommelpe
Copy link

Thank you for taking time filling this issue. This may have seem to be related to issue googleapis/nodejs-firestore#768. I'll file a report and confirm this internally.

@mikelehen
Copy link
Contributor

Thanks for the report. This isn't too surprising. Firebase uses various singleton instances, etc. to track state. I can actually reproduce the leak reported by jest with just:

const firebase = require('firebase/app');
require('firebase/firestore');

describe('test', () => {
  it('doesnt leak memory', async () => {
  })
})

If you're running into a specific issue with unbounded memory usage we can look into it, but we don't provide any guarantees about cleaning up all traces of the SDK when it comes to memory usage.

Can I ask how important this is to you and what your overall goal is? We would likely accept (noninvasive) PRs to clean up memory usage but beyond that I'm not sure if/when we'd be able to invest much time into this. Thanks!

@MaximeHeckel
Copy link

@mikelehen @zeevl my team actually is running into this issue

Started looking at why our Jest tests was running out of memory even for very easy test (I saw a high heap usage like 900MB for a simple hello world)

Are there or has there been any plans to address this? It's not blocking or anything but definitely worrisome as it's making some of our Jest test suite unstable

@melliu
Copy link

melliu commented Oct 19, 2020

we experience memory leaking on production when using firestore too,

   admin.initializeApp();
   admin.app().firestore().doc().set();
   admin.app().firestore().doc().update();

image

@dconeybe
Copy link
Contributor

@melliu My sincere apologies for sitting on your issue report for so long. Based on the code snippet you provided, it looks like you are using the Admin SDK (not the Web SDK). If that is the case then the correct repository for this issue is https://github.com/googleapis/nodejs-firestore. I'd be happy to log this issue there on your behalf if you'd like. In order to do that it would be helpful (but not required) to get some additional information from you:

  1. What OS are you running on (e.g. Windows, Mac, Linux)?
  2. What Node.js version are you using?
  3. What npm version are you using?
  4. Can you provide a minimal app to reproduce (e.g. a GitHub repository)?
  5. What tool did you use to produce that graph that you included?

Thank you for your time and sorry again for not responding sooner.

@dconeybe
Copy link
Contributor

@MaximeHeckel Please accept my sincere apologies for taking so long to reply to your comment. As mentioned in an earlier comment, we do not promise to clean up every single object when calling app.dispose(); however, unbounded memory growth or extreme memory consumption (900MB in your case) is concerning. Are you able to provide some more details about the memory issues that you are experiencing? A sample app and/or steps to reproduce, if available, would be helpful. Thank you.

@MaximeHeckel
Copy link

hey @dconeybe , no worries, I went around the issue and decided to manually mock any firestore/firebase dependent call or functions. This reduced our memory leakage enough to make our unit tests running again

@dconeybe
Copy link
Contributor

Thanks for the response, @MaximeHeckel. From what I can tell, the only action remaining for the Firestore dev team is to potentially open up a similar ticket in the nodejs-firestore repository on behalf of @melliu. As a result, I've added the "needs-info" tag, which will automatically close this issue if there are no more responses.

@google-oss-bot
Copy link
Contributor

Hey @zeevl. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

@google-oss-bot
Copy link
Contributor

Since there haven't been any recent updates here, I am going to close this issue.

@zeevl if you're still experiencing this problem and want to continue the discussion just leave a comment here and we are happy to re-open this.

@firebase firebase locked and limited conversation to collaborators Jan 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants