Resolve memory leak when using cursor operations like $count in mongodb@4-6 #152
Conversation
ericyhwang
force-pushed
the
fix-mongo-count-leak
branch
2 times, most recently
from
0b893cc
to
d73d14d
Compare
|
@ericyhwang is this a bug with |
|
Here's the relevant spot in |
There was a problem hiding this comment.
Do we also need to actively close the cursor if no operation is defined?
There was a problem hiding this comment.
No need, the toArray() will exhaust the cursor, so no leak in that case.
As discussed, I'll remove the cursor.close() in the $map case since I added a toArray() there.
…db@4-6 Starting in Mongo Node driver `mongodb@4`, up through the latest version `mongodb@6`, using `cursor.count()` results in a memory leak. `mongodb@3` doesn't leak in the same situation. The driver creates an implicit client session, but it doesn't automatically close the session when getting the results back from the server. Those unclosed sessions build up over time, causing a memory leak. sharedb-mongo exposes document counting via a user passing the `$count: true` property on query objects, and it currently uses `cursor.count()`. There are a couple ways sharedb-mongo could address the leak: - Switch to `Collection#countDocuments()`, which is the recommended replacement for the deprecated `FindCursor#count()`. This is better long-term, but it's more work since we have to map things like `$limit` from using chained cursor calls over to the equivalent property in CountOptions, where appropriate. - Explicitly close the cursor. Easy and safe, since the cursor is created inside sharedb-mongo and not exposed externally. To resolve the leak more quickly, this change opts for the latter, explicitly closing the cursor for the "cursor operations" `$count`, `$explain`, and `$map`.
ericyhwang
force-pushed
the
fix-mongo-count-leak
branch
from
d73d14d
to
06b5c78
Compare
…he close() unnecessary
I'll try and set up a repro for |
|
BTW, after fighting Jira search for a while, I found issues already filed against the Node driver: Have edited them into the PR description for posterity. |
Starting in Mongo Node driver
mongodb@4, up through the latest versionmongodb@6, usingcursor.count()results in a memory leak.mongodb@3doesn't leak in the same situation.The driver creates an implicit client session, but it doesn't automatically close the session when getting the results back from the server. Those unclosed sessions build up over time, causing a memory leak.
Relevant issues filed against the Mongo Node driver:
sharedb-mongo exposes document counting via a user passing the
$count: trueproperty on query objects, and it currently usescursor.count().There are a couple ways sharedb-mongo could address the leak:
Collection#countDocuments(), which is the recommended replacement for the deprecatedFindCursor#count(). This is better long-term, but it's more work since we have to map things like$limitfrom using chained cursor calls over to the equivalent property in CountOptions, where appropriate.To resolve the leak more quickly, this change opts for the latter, explicitly closing the cursor for the "cursor operations"
$count,$explain, and$map.Thanks to @deongroenewald for the private report and repro case!
This also includes the changes in #153 and #154 - assuming those get merged first, I'll rebase this for a smaller diff.