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

Meteor v2.8 - Memory leak in mongo driver? #12321

Closed
xet7 opened this issue Nov 25, 2022 · 8 comments
Closed

Meteor v2.8 - Memory leak in mongo driver? #12321

xet7 opened this issue Nov 25, 2022 · 8 comments

Comments

@xet7
Copy link
Contributor

xet7 commented Nov 25, 2022

https://forums.meteor.com/t/meteor-v2-8-memory-leak-in-mongo-driver/59101

@xet7
Copy link
Contributor Author

xet7 commented Nov 28, 2022

From MongoDB Jira - Bailey Pearson:

Hey dominik,
there are two variations of sessions in the Node driver. Every operation that gets sent to the server has a session attached - we represent these sessions with ServerSessions. The driver manages ServerSessions in a ServerSessionPool and will clean up stale server sessions when they expire. ClientSessions are an abstraction over ServerSessions that allow users of the driver to provide a session for operations. The driver does not manage ClientSessions - users are responsible for ending them using the endSession method.

We allow users to create sessions using the MongoClient's startSession method (source here). This method ensures that when endSessions is called, we remove the client session from the ActiveSession set.

The reported bug is a buildup of ClientSessions. My suspicion (without knowing how Meteor works) is that Meteor is creating client sessions under the hood but never ending the sessions, resulting in a buildup of sessions in the driver.

@xet7
Copy link
Contributor Author

xet7 commented Nov 28, 2022

From MongoDB Jira - Patrick Schubert

To Bailey Pearson

Thank you for your reply. What confuses me the most is the following: 99 (maybe even 100) percent of Meteor's mongo calls are handled by this package: https://github.com/meteor/meteor/blob/devel/packages/mongo/collection.js

I just checked its codebase (and all the other Meteor code), but could NOT find any calls of startSession() anywhere! 

My question is: Are there any other methods to create ClientSessions? What am I missing here? How is it possible that we see so many ClientSessions in our heapdump but I can't find the according code anywhere?

EDIT: I am not fully aware of all the mongodb driver internals, but this code seems to me, that sessions are also indeed created implicitly?!

@xet7
Copy link
Contributor Author

xet7 commented Nov 28, 2022

From MongoDB Jira - Bailey Pearson

Yes, we do create sessions internally but they're created and disposed of one-off. Further down in execute_operation, we try-finally all codepaths that leave that function with session cleanup logic:

This problem could be originating in the driver but it's difficult to say without more information. If I understand correctly, this problem surfaced in a recent meteor upgrade? While they did bump the driver version, it's possible they made other changes that could cause issues (they do patch our Collection.find method, the method responsible for returning a FindCursor). It's also hard to say what could be going wrong without knowing how meteor changes / patches / wraps the driver.

I didn't really answer one of your original questions though. The circular reference is expected. Part of the reason I'm hesitant to believe the problem originates in the driver is that while we did change the structure of the circular reference in v4.8.0 of the driver, it was present before v4.8.0. Previously, the reference cycle would have been FindCursor -> MongoClient -> Topology -> Set -> ClientSession -> FindCursor. Now it's just FindCursor -> MongoClient -> Set -> ClientSession -> FindCursor.

@znewsham
Copy link
Contributor

znewsham commented Nov 28, 2022

The problem (at least part of it) is that meteor creates a cursor then calls collection.countDocuments(), the session is assigned at init time and then cleanedup when you finish running through a cursor - since collection.countDocuments() never executes anything on the cursor, it doesn't trigger the cleanup.

The "correct" fix would probably be to defer creating the actual cursor until we call forEach(), map() or fetch() - however, easier would just be to end the session when a count is called.

As an aside - there really is a bug in rawCollection.find().count() (e.g., in the mongo driver itself) - but since that is deprecated, they probably don't care

@afrokick
Copy link
Contributor

afrokick commented Nov 29, 2022

@znewsham I provide full server code and it works for countDocuments

import {Meteor} from 'meteor/meteor';
import {MongoInternals} from 'meteor/mongo';

const wait = (ms) => new Promise(res => setInterval(res, ms))
const times = (n, action) => {
  while (n > 0) {
    n--;
    action();
  }
}


Meteor.startup(async () => {
  const client = MongoInternals.defaultRemoteCollectionDriver().mongo.client;
  const logSize = () => console.log(client.s.activeSessions.size);

  logSize();

  times(1000, () => {
    // first solution(like workaround) - call `fetch` after `count`
    // const c = Meteor.users.find({});
    // c.count();
    // c.fetch(); // to close the session

    // second solution - use `countDocuments`
    Meteor.users.rawCollection().countDocuments().then(() => null);
  });

  await wait(1000); //wait a little bit

  logSize();
});

So, for both solutions it prints 0

@znewsham
Copy link
Contributor

@afrokick I'm not sure either of those solutions are true solutions (though the latter is certainly a workaround).

Forcing a fetch after a count would negate the value of the count (just get a length on the - potentially massive - array) and we shouldn't need to use the raw collection to get a count without a memory leak.

I believe this PR should fix the issue: #12326

@afrokick
Copy link
Contributor

@znewsham Yeah, I know. It is a temporary solution. Your fix is great!

@xet7
Copy link
Contributor Author

xet7 commented Nov 29, 2022

From MongoDB Jira - Bailey Pearson

Yes, we do create sessions internally but they're created and disposed of one-off. Further down in execute_operation, we try-finally all codepaths that leave that function with session cleanup logic:

This problem could be originating in the driver but it's difficult to say without more information. If I understand correctly, this problem surfaced in a recent meteor upgrade? While they did bump the driver version, it's possible they made other changes that could cause issues (they do patch our Collection.find method, the method responsible for returning a FindCursor). It's also hard to say what could be going wrong without knowing how meteor changes / patches / wraps the driver.

I didn't really answer one of your original questions though. The circular reference is expected. Part of the reason I'm hesitant to believe the problem originates in the driver is that while we did change the structure of the circular reference in v4.8.0 of the driver, it was present before v4.8.0. Previously, the reference cycle would have been FindCursor -> MongoClient -> Topology -> Set -> ClientSession -> FindCursor. Now it's just FindCursor -> MongoClient -> Set -> ClientSession -> FindCursor.

@xet7 xet7 closed this as completed Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants