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

High memory usage and slow to update large numbers of parent documents #33

Open
mfen opened this issue Jan 9, 2023 · 1 comment
Open

Comments

@mfen
Copy link

mfen commented Jan 9, 2023

The underlying meteor-collection-hooks performs two fetch()s on update() for any collection with an after.update hook defined.

  • the first to get the ids of all docs matching the selector
  • the second to iterate over these docs post-update and fire the after hooks

This can be both slow for large numbers of docs, as well as expensive on memory as it is doing a fetch() of all docs rather than iterating over the cursor.

This denormalize package adds a parentCollection.after.update hook, but also calls parentCollection.update() in any relevant childCollection mutation hook to maintain the caches. The result is that on any child document mutation there is a chain of hooks causing the 2x fetch(), e.g.:

childCollection.update()
  -> childCollection.after.update()
    -> parentCollection.update()
      -> parentCollection.after.update() // 2x fetch()

Related: Meteor-Community-Packages/meteor-collection-hooks#259

Suggestion

Denormalize simply needs to do an parentCollection.updateMany() to update the caches without the extra pre-fetching of ids the hooks do to support arbitrary selectors. Perhaps this package should wrap the Mongo API mutators directly, similar to how the hooks tie in, so as to avoid the chain of hook logic. The one downside I see is exactly that: are there other hooks expected to be chained by the cache update, or even chained denormalization, which this would break? Perhaps this could be an opt-in alternative for maintaining the cache if the user does not require triggering hooks/chaining via the cache update.

@github-actions
Copy link

github-actions bot commented Jan 9, 2023

Thank you for submitting this issue!

We, the Members of Meteor Community Packages take every issue seriously.
Our goal is to provide long-term lifecycles for packages and keep up
with the newest changes in Meteor and the overall NodeJs/JavaScript ecosystem.

However, we contribute to these packages mostly in our free time.
Therefore, we can't guarantee your issues to be solved within certain time.

If you think this issue is trivial to solve, don't hesitate to submit
a pull request, too! We will accompany you in the process with reviews and hints
on how to get development set up.

Please also consider sponsoring the maintainers of the package.
If you don't know who is currently maintaining this package, just leave a comment
and we'll let you know

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

1 participant