firebase / firebase-functions

Firebase SDK for Cloud Functions
https://firebase.google.com/docs/functions/
MIT License
1.02k stars 201 forks source link

getting Possible EventEmitter memory leak detected on firebase functions that use google-cloud/ packages #1578

Closed jdziek closed 2 months ago

jdziek commented 3 months ago

Hi. I started getting a memory leak warning a few days ago for no apparent reason. I did update packages a few weeks ago but didn't see anything till recently. I updated the packages to the most recent ones again and still got that message. This warning pops up whenever a function is initiated. This is a copy of an issue I originally raised in nodejs-firestore repo but realised that this might be more related to functions, after reading the logs

image

1) Is this a client library issue or a product issue? I believe this is a google-cloud package issue. I get this error, particularly in functions that must process files to storage.

2) Did someone already solve this? I've seen it has been an issue in the past but nothing has been raised since then on a current release

Environment details

It happens on gen 2 firebase functions using firebase-tools@13.11.2 using nodejs 20 OtherGCPp packages that I'm using are "@google-cloud/documentai": "^8.8.0", "@google-cloud/firestore": "^7.8.0", "@google-cloud/pubsub": "^4.5.0", "@google-cloud/storage": "^7.11.2", "firebase-admin": "^12.1.1", "firebase-functions": "^5.0.1",

Here is a snippet of my code. The issue happens in every function that references google-cloud packages, so its not an isolated issue to this particular code, but I thought that it cant hurt including it.


import firestore from "@google-cloud/firestore";
import { InternalServerError } from "@moveready/request-errors/lib/index.js";
import envConfig from "../shared_utilities/envConfig.js";

const client = new firestore.v1.FirestoreAdminClient();

export default {
  async createDbBackup() {
    try {
      const databaseName = client.databasePath(
        envConfig.gcp.projectId,
        "(default)",
      );

      return client
        .exportDocuments({
          name: databaseName,
          outputUriPrefix: `gs://${envConfig.gcp.dbBackupBucket}`,
          collectionIds: [],
        })
        .then(([response]) => {
          logger.info(`Backup creation was successful: ${response.name}`);
          return response;
        })
        .catch((err) => {
          logger.error(err);
          throw new InternalServerError();
        });
    } catch (err) {
      logger.error(err);
      throw err;
    }
  },
};
google-oss-bot commented 3 months ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

exaby73 commented 2 months ago

Hey @jdziek. Could you provide a code sample and logs in text so that I can investigate this further?

jdziek commented 2 months ago

Hi,. thanks for replying. The code is above. I think you don't even need to run it, I get this warning whenever a function/container starts. The above is a simple backup, scheduler function, for our database. The message pops up in any function that references firebase/gcp packages, functions related in particular in particular.

Chose this function as a sample as it had the least amount of code and thought it was the easiest to reproduce. Also, been trying the root of this problem in other repos so I thought it might be worth sharing this link https://github.com/googleapis/nodejs-firestore/issues/2072

DEFAULT 2024-06-26T12:57:20.148846Z (Use `node --trace-warnings ...` to show where the warning was created)
DEFAULT 2024-06-26T12:57:20.151936Z [2024-06-26T12:57:20.152] [INFO] MOVERLY - MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 uncaughtException listeners added to [process]. Use emitter.setMaxListeners() to increase limit
DEFAULT 2024-06-26T12:57:20.151948Z at genericNodeError (node:internal/errors:984:15)
DEFAULT 2024-06-26T12:57:20.151952Z at wrappedFn (node:internal/errors:538:14)
DEFAULT 2024-06-26T12:57:20.151956Z at _addListener (node:events:593:17)
DEFAULT 2024-06-26T12:57:20.151960Z at process.addListener (node:events:611:10)
DEFAULT 2024-06-26T12:57:20.151966Z at ErrorHandler.register (/layers/google.nodejs.functions-framework/functions-framework/node_modules/@google-cloud/functions-framework/build/src/invoker.js:82:17)
DEFAULT 2024-06-26T12:57:20.151970Z at Server.<anonymous> (/layers/google.nodejs.functions-framework/functions-framework/node_modules/@google-cloud/functions-framework/build/src/main.js:46:26)
DEFAULT 2024-06-26T12:57:20.151974Z at Object.onceWrapper (node:events:633:28)
DEFAULT 2024-06-26T12:57:20.151977Z at Server.emit (node:events:531:35)
DEFAULT 2024-06-26T12:57:20.151980Z at Server.emit (node:domain:488:12)
DEFAULT 2024-06-26T12:57:20.151984Z at emitListeningNT (node:net:1932:10)

EDIT. Sorry, just realised that originally I posted a log of Honeybadgers report regarding the issue instead of the actual log relevant to the issue. Any help would be greatly appreciated. The thing keeps popping up still

google-oss-bot commented 2 months ago

Hey @jdziek. We need more information to resolve this issue but there hasn't been an update in 7 weekdays. I'm marking the issue as stale and if there are no new updates in the next 3 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!

jdziek commented 2 months ago

Writing is as i dont want this issue to close as I'm still experiencing the problem. I updated my previous post with a correct log this time. If my code sample is not sufficient, tell me how to improve it. It will be useful for the future issues. It was just the smallest/the easiest function to recreate I could find at the time.

exaby73 commented 2 months ago

This error appears due to a hard coded default value for event listeners. See https://stackoverflow.com/questions/9768444/possible-eventemitter-memory-leak-detected for more details. This value can be increased but it's not recommended.

Just to confirm, this error happens on large Firestore operations? It's what I assume from your code sample you had provided

jdziek commented 2 months ago

I saw that option and yeah, im quite reluctant to do it. Happens even in a small scale operation . The warning pops up as soon as the container starts that has code referencing gcp or firebase packages that reference @google-cloud/functions-framework in any way. The code doesn't even have to run.

exaby73 commented 2 months ago

Could you give me a sample code I could run directly? I cannot seem to reproduce this issue but maybe I'm not doing the correct things to reproduce. A code sample I can copy and paste would really help :)

jdziek commented 2 months ago

Ah, sorry now i understand what you meant by the proper code sample. Sadly I'm struggling to recreate it and while I was doing so I started debugging it again. Thing I realised is that it has to do with how code between onRequest functions is interacting. Struggling to identify which ones those are as they seem to alternate I think depending on combination of which are enabled or not, making it incredibly frustrating. Also, just realised, the only reason I'm noticing that warning is because I use

process
  .on("uncaughtException", function (err) {
    console.log(`${new Date().toUTCString()} uncaughtException:`, err.message);
    console.log(err.stack);
    throw err;
  })
  .on("warning", (e) => console.log(e.stack));

in oRequest functions that also use express and express router. If I get any more info ill post an update.

EDIT. Crap, I know why this is happening. Its the number of these process.on listeners. I had exactly 9 functions, only one of them had two on listeners (example above), so when I added another function with just

process
  .on("uncaughtException", function (err) {
    console.log(`${new Date().toUTCString()} uncaughtException:`, err.message);
    console.log(err.stack);
    throw err;
  });

that made it eleven. Damn. Im sorry if I wasted your time. I guess that's another example that functions are not as independent as they seem at first, even within their own container. Also, now I know what you mean by a code sample. Mine was too tied to our codebase. Will bear that in mind in the future.

exaby73 commented 2 months ago

No worries :) Don't let this discourage you from opening issues in the future. I'll close this issue now. Thanks for raising it anyways