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 (Help wanted) #9259

Closed
ktjd123 opened this issue Jul 19, 2020 · 11 comments
Closed

Memory leak (Help wanted) #9259

ktjd123 opened this issue Jul 19, 2020 · 11 comments
Milestone

Comments

@ktjd123
Copy link

ktjd123 commented Jul 19, 2020

Do you want to request a feature or report a bug?
bug (help wanted)

What is the current behavior?
memory leak

If the current behavior is a bug, please provide the steps to reproduce.
memory grows over time. have no idea why
스크린샷 2020-07-19 오후 12 42 33
This happened after updating some code. the code seems to be totally fine.

Did to solve issue.

  1. reduce mongoose connect poolsize 1000 to default.
  2. changed GridFS instance to singleton pattern.
  3. added lean to some codes since heap snapshot showed me JSArrayBuffer.

Didn't work.

  // MongoDB
  // mongoose.set('debug', true);
  mongoose.Promise = global.Promise;
  await mongoose
    .connect(MONGODB_URI, {
      useUnifiedTopology: true,
      useNewUrlParser: true,
      useCreateIndex: true,
      autoIndex: true,
      // poolSize: 1000,
    })
    .catch(() => {
      console.error('DB NOT CONNECTED');
      process.exit();
    });
  // Session
  const MongoStore = connectMongo(session);
  server.use(
    session({
      name: 'artgravia',
      secret: SESSION_SECRET,
      resave: false,
      saveUninitialized: false,
      rolling: true,
      cookie: {
        maxAge: 365 * (24 * 60 * 60 * 1000),
        // domain: dev ? undefined : SESSION_DOMAIN,
        sameSite: true,
      },
      store: new MongoStore({
        mongooseConnection: mongoose.connection,
        ttl: 365 * (24 * 60 * 60 * 1000),
      }),
    })
  );

Mongoose connection code.

import mongoose from 'mongoose';
import mongo from 'mongodb';

let gridFsBucket: mongo.GridFSBucket;

export default () => {
  if (gridFsBucket) {
    return gridFsBucket;
  }
  gridFsBucket = new mongoose.mongo.GridFSBucket(mongoose.connection.db);
  return gridFsBucket;
};

gridfs bucket instance code.

import express from 'express';
import joi from 'joi';
import mongoose from 'mongoose';
import { File, Files } from '../models';
import { imgPro, toBase64 } from '../lib/imgPro';

const router = express();

router.get('/video/:id', async (req, res) => {
  const schema = joi.object().keys({
    id: joi.string().trim().required(),
  });

  const result = joi.validate(req.params, schema);

  if (result.error) return res.json({ code: 1 });

  const { id } = result.value;

  if (mongoose.Types.ObjectId.isValid(id) !== true) return res.json({ code: 1 });

  const videoFile = await Files.findOne({ _id: id });
  if (!videoFile) return res.json({ code: 2 });

  const headerRange = req.headers.range;

  if (headerRange) {
    const parts = headerRange.replace('bytes=', '').split('-');
    const partialstart = parts[0];
    const partialend = parts[1];

    const start = parseInt(partialstart, 10);
    const end = partialend ? parseInt(partialend, 10) : videoFile.length - 1;
    const chunksize = end - start + 1;

    res.writeHead(206, {
      'Content-Type': 'video/mp4',
      'Content-Range': `bytes ${start}-${end}/${videoFile.length}`,
      'Accept-Ranges': 'bytes',
      'Content-Length': chunksize,
    });

    return File()
      .openDownloadStream(mongoose.Types.ObjectId(id), { start, end: end + 1 })
      .pipe(res);
  }
  res.writeHead(200, { 'Content-Type': 'video/mp4' });
  return File().openDownloadStream(mongoose.Types.ObjectId(id)).pipe(res);
});

router.get('/image/:id', async (req, res) => {
  const schema = joi.object().keys({
    id: joi.string().trim().required(),
  });

  const result = joi.validate(req.params, schema);

  if (result.error) return res.json({ code: 1 });

  const { id } = result.value;

  if (mongoose.Types.ObjectId.isValid(id) !== true) return res.json({ code: 1 });

  res.set('Cache-Control', 'public, max-age=31557600');
  res.set('Content-Type', 'image/jpeg');
  res.set('Access-Control-Allow-Origin', '*');
  res.set('Content-disposition', `attachment; filename=${id}.jpeg`);
  return File().openDownloadStream(mongoose.Types.ObjectId(id)).pipe(res);
});

router.post('/transform/image', async (req, res) => {
  if (!req.session || !req.session.info) return res.json({ code: 1 });
  if (req.session.info.role !== 'Admin') return res.json({ code: 2 });

  const { image } = req.body;

  const result = await imgPro(image);
  const base64Result = toBase64(result);

  return res.json(base64Result);
});

export default router;

file loading api.

스크린샷 2020-07-19 오후 2 00 21
heap snapshot comparison between start and now.

스크린샷 2020-07-19 오후 2 02 08
first one is about bson

and second ~~ most are about this and have no idea what this is about
스크린샷 2020-07-19 오후 2 02 17

server is Next.js based custom server with express.js.

Below is serverside code of the commit. It seems to be memory leak is happend after this commit.
스크린샷 2020-07-19 오후 2 05 10
스크린샷 2020-07-19 오후 2 05 21
스크린샷 2020-07-19 오후 2 05 31
스크린샷 2020-07-19 오후 2 05 47
스크린샷 2020-07-19 오후 2 06 08

addToPointArray is global variable and is

const addPointsToArray = (array: Array<Ranking>, _id: string, amount: number) => {
  const resultArray = array;
  const index = resultArray.findIndex((item) => item._id === String(_id));

  if (index !== -1) {
    resultArray[index].amount += Math.floor(Math.abs(amount) / 100);
  } else {
    resultArray.push({ _id: String(_id), amount: Math.floor(Math.abs(amount) / 100) });
  }

  return resultArray;
};

about 6k accounts, 1k pointdeposithistories, 6k pointhistories.

I know code is not properly optimized, but have no idea why there is memory leak..

I'm using pm2 clustered(2 clusters) mode for this.
when I see heap size through pm2 monit It only shows about 100mb ~ 200mb but when I see total memory usage through pm2 list, It grows forever (over 2GB).

Thank you and always appreciate your works.

What is the expected behavior?
no memory leak

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

Node.js v12.18.0

mongoose ^5.9.24

mongoDB mongod --version
db version v3.6.8
git version: 8e540c0b6db93ce994cc548f000900bdc740f80a
OpenSSL version: OpenSSL 1.1.1f 31 Mar 2020
allocator: tcmalloc
modules: none
build environment:
distarch: x86_64
target_arch: x86_64

@vkarpov15 vkarpov15 added this to the 5.9.26 milestone Jul 23, 2020
@vkarpov15 vkarpov15 added the needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue label Jul 23, 2020
@ktjd123
Copy link
Author

ktjd123 commented Jul 24, 2020

@vkarpov15 I don't know where the leak is happening so I can't provide repro script , sorry. I can show you the full code of repo. Thanks

@vkarpov15
Copy link
Collaborator

I'll take a look and see what I can find from the information you included. If that isn't enough to repro this, I'll need to take a look at the full repo.

@ktjd123
Copy link
Author

ktjd123 commented Jul 25, 2020

Thank you.

@vkarpov15 vkarpov15 modified the milestones: 5.9.26, 5.9.27 Jul 27, 2020
@vkarpov15
Copy link
Collaborator

@ktjd123 I took a closer look at your code samples and I don't think there's enough info for me to repro this. Can you post the changes you made that appear to fix the leak as well?

Also, if you would be willing to give me access to your repo, that would be helpful. My email is val@karpov.io.

@vkarpov15 vkarpov15 modified the milestones: 5.9.27, 5.9.29 Aug 7, 2020
@vkarpov15
Copy link
Collaborator

I can probably look into this more, but my best explanation so far is this: Node.js doesn't forward errors when doing pipe(), so if openDownloadStream() throws an error (like due to an id that isn't in the database) your request will hang forever. Given that you were creating a separate GridFS stream for every request, that might explain the leak. I'll try to see if I can repro this tomorrow.

@ktjd123
Copy link
Author

ktjd123 commented Aug 8, 2020

@vkarpov15 That might be a problem. I changed to singleton instance after thinking of that problem, and reduced pool size 1000 -> default.

I saw several mongoose errors at server that file is not found. I added logic to check if file exists. -> I think this helps to manage memory but still having growth of memory so I think there is other problem

I used pm2 to restart after nodejs uses memory after 2G and it has 305 restart records for uptime of 10d.

Thank you for your help :)!

@vkarpov15 vkarpov15 modified the milestones: 5.9.29, 5.9.30 Aug 13, 2020
@vkarpov15 vkarpov15 modified the milestones: 5.10.1, 5.10.2, 5.10.3 Aug 26, 2020
@buccfer
Copy link

buccfer commented Aug 31, 2020

Hi @vkarpov15 , I don't know if this is related or not. But we are getting memory leaks quite often. Below are the logs we have when the app crashes.

image

Seems to be an issue with an embedded document.

nodejs.log     0: builtin exit frame: unshift(this=0x0ee6f6c5a411 <JSArray[0]>,0x2baaced42391 <String[9]: filterSet>,0x0ee6f6c5a411 <JSArray[0]>)
nodejs.log     1: $__fullPath [0x1c8964a55f41] [/var/app/current/node_modules/mongoose/lib/types/embedded.js:~391] [pc=0x28dfc4362369](this=0x0ee6f6c59999 <EmbeddedDocument map = 0x130e711a3469>,path=0x2baaced422c1 <String[8]: valueSet>)

We have mongoose version 5.9.27

@buccfer
Copy link

buccfer commented Sep 1, 2020

Seems like an infinite loop here https://github.com/Automattic/mongoose/blob/master/lib/types/embedded.js#L399

@vkarpov15 vkarpov15 modified the milestones: 5.10.3, 5.10.4, 5.10.5 Sep 3, 2020
@vkarpov15 vkarpov15 modified the milestones: 5.10.5, 5.10.6 Sep 11, 2020
@vkarpov15 vkarpov15 modified the milestones: 5.10.6, 5.10.7, 5.10.8 Sep 18, 2020
@vkarpov15 vkarpov15 added this to the 5.11.x milestone Dec 10, 2020
@geiszla
Copy link

geiszla commented Apr 6, 2021

We are also experiencing this with Mongoose in production. We use Google Cloud for deployment and pm2 needs to restart the server about once in a day because it goes over the 2GB limit. The profiler clearly shows that Mongoose is accumulating data on the heap (the more red the bar is, the more it grew from a previous snapshot).
image
image

As far as I can see from these, mainly the internal cache is the reason that the heap grows this fast. Is there a way to limit it or to somehow eliminate this memory growth?

We're using mongoose version 5.12.2 on node.js 12.

@geiszla
Copy link

geiszla commented May 5, 2021

Turned out our problem above was caused by a memory leak issue in @google-cloud/debug-agent. There is an open issue in the repo on GitHub.

@vkarpov15 vkarpov15 modified the milestones: 5.12.x, 5.13.1, 5.13.2, 5.13.3 Jun 30, 2021
@vkarpov15 vkarpov15 modified the milestones: 5.13.3, 5.13.4 Jul 16, 2021
@vkarpov15 vkarpov15 modified the milestones: 5.13.4, 5.13.5, 5.13.6 Jul 28, 2021
@vkarpov15 vkarpov15 modified the milestones: 5.13.6, 5.13.7, 5.13.8 Aug 9, 2021
@IslandRhythms IslandRhythms added seen and removed needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue labels Aug 11, 2021
@vkarpov15 vkarpov15 modified the milestones: 5.13.8, 5.13.9 Aug 23, 2021
@vkarpov15 vkarpov15 modified the milestones: 5.13.9, 6.0.6 Sep 2, 2021
@vkarpov15 vkarpov15 modified the milestones: 6.0.6, 6.0.9 Sep 14, 2021
@vkarpov15
Copy link
Collaborator

I did some more digging, but haven't been able to find any more insights other than the pipe() issue. Realistically, without more info on workload, I won't be able to make any meaningful progress on whether this leak is caused by Mongoose or something else.

Re: @buccfer 's comment here, we'll add an extra check to throw if there's an infinite loop in that case. While I don't see how there could be an infinite loop here, it's best to be defensive and throw an error if that does happen.

vkarpov15 added a commit that referenced this issue Oct 1, 2021
…ment is a parent of itself in `ownerDocument()`

Re: #9259
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants