firebase / firebase-admin-node

Firebase Admin Node.js SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
1.62k stars 367 forks source link

Memory leak in v8 messaging #662

Closed 0x80 closed 4 years ago

0x80 commented 4 years ago

[REQUIRED] Step 2: Describe your environment

[REQUIRED] Step 3: Describe the problem

I have a cloud function which used the old FCM API sendToDevice(). Then upgrading firebase-admin from 6 to 8 I started to get into trouble because if what appears to be a memory-leak.

I have updated the function to use the new FCM sendMulticast() method. This didn't change anything to the leak. I have also tried deploying to both node 8 and 10 functions, but this made no difference.

When I downgraded to firebase-admin v7 the problem went away.

In the attached image you can clearly see when I upgraded and downgraded back to 7.

In the second half of the leaking you see a more defined saw tooth pattern, but that is the result of me optimising the code so that the amount of messages sent is limited to 2k users. As a result it leaks less memory and the build-up is slower and more consistent.

When the function hits the maximum of 2GB then it crashes (usually with a vague error like:

Could not load the default credentials

or

Status code: 502.

Steps to reproduce:

Use sendToDevice or sendMulticast to send lots of messages. The more you sent the more pronounced the leak will be. Try 5k and it quickly runs over the 2GB limit.

Screenshot 2019-10-01 10 08 10
hiranya911 commented 4 years ago

I'm not able to reproduce this. I can send very large volumes of messages (tested with up to 10K) with no impact on memory usage. Also the FCM code (especially the legacy code) hasn't changed between v7 and v8, so there' no reason why it would cause a memory leak. Are you using any other APIs (Firestore for example, which has a known memory leak issue in latest versions)?

Here are my test results:

const admin = require('firebase-admin');

function toMB(bytes) {
  return Math.round(bytes / 1024 / 1024 * 100) / 100;
}

function heapStats() {
  const used = process.memoryUsage();
  return `[ HEAP: ${toMB(used.heapUsed)} MB, RSS: ${toMB(used.rss)} MB ]`;
}

function sendMulticast(tokens) {
  const msg = {
    data: {key: 'value'},
    tokens,
  }
  return admin.messaging().sendMulticast(msg, true);
}

const app = admin.initializeApp();
const BATCHES = 100;

console.log('Starting application:', heapStats());

const promises = [];
for (let i = 0; i < BATCHES; i++) {
  const tokens = [];
  for (let j = 0; j < 100; j++) {
    tokens.push(`fake.token.${j}`);
  }

  promises.push(sendMulticast(tokens));
}

console.log(`Started ${BATCHES} batches:`, heapStats());

Promise.all(promises)
  .then((results) => {
    const total = results.map(r => r.responses.length).reduceRight((a, b) => a + b, 0);
    console.log(`Done sending ${total}:`, heapStats());
  })
  .catch((err) => {
    console.log(err);
  })
  .then(() => {
    global.gc();
    console.log('Garbage collected:', heapStats());
    return app.delete();
  })
  .then(() => {
    console.log('\nDONE');
  });

Output for 10K messages:

Starting application: [ HEAP: 7.91 MB, RSS: 29.1 MB ]
Started 100 batches: [ HEAP: 11.28 MB, RSS: 40.91 MB ]
Done sending 10000: [ HEAP: 52.83 MB, RSS: 121.43 MB ]
Garbage collected: [ HEAP: 8.57 MB, RSS: 100.57 MB ]

Notice how the heap grows from 8MB to 53MB, but then shrinks after GC had a chance to run. Can you share your function or see if you can reproduce the issue with a similar, simplified test application?

0x80 commented 4 years ago

@hiranya911 Thanks for the detailed analysis! I never used memory stats and the garbage collector like that. This example will come in very handy if I ever run into such a situation again 👍

I believe you are right. I am also using Firestore in that same cloud function. Because I hadn't noticed any issues in all of my other code using Firestore, and this function was primarily about using the messaging API, I assumed the problem must originate from there.

I can't seem to find the Firestore issue you are referring to. Could you link it here?

hiranya911 commented 4 years ago

Hi 0x80. Thanks for confirming that you're also using Firestore in your implementation. Things make more sense now. The primary differentiator between v7 and v8 SDKs is the use of grpc-js by Firestore. And it is in this dependency we are currently observing memory leaks.

I'm going to point you to https://github.com/googleapis/nodejs-firestore/issues/768 and close this issue. I would advise you to stick to v7 for now. We can revisit this issue if we find more evidence that the issue is related to the code strictly in this repo.

dougg0k commented 1 year ago

Yeah, I have grpc-js in another project, the only way to avoid mem leaks is to cache the client.