Meteor-Community-Packages / meteor-collection-hooks

Meteor Collection Hooks
https://atmospherejs.com/matb33/collection-hooks
MIT License
657 stars 92 forks source link

Entire document is fetched even when no hooks are registered #203

Closed ephemer closed 7 years ago

ephemer commented 7 years ago

Short explanation: The line here https://github.com/matb33/meteor-collection-hooks/blob/master/update.js#L27 checks whether aspects.before or aspects.after exist. This check is incorrect though, because aspects.before and .after are arrays, which are always 'truey'.

What this means in practice is that every time any update occurs on any document in any collection (even without any hooks being registered), that entire document is fetched and then thrown away again by the collection-hooks package.

This came up in our Kadira performance monitoring, because some of our documents are over 1MB in size, meaning updates to that collection took up to 1s in production for just an indexed query + simple increment operation. We narrowed the issue down to this package, which we have subsequently removed (we didn't need its functionality). But I thought I'd post an issue here in case others are wondering, and to kindly suggest a fix for package users (I'd check the array lengths).

After removing the package our updates take <1 ms as they should.

namirsab commented 7 years ago

Thanks for posting the issue here. If you like you can write a pull request. If not ill take a look as soon as i can.

ephemer commented 7 years ago

I'm going to leave this one to you. Have a good weekend!

zimme commented 7 years ago

This should be solved in the next version.