More explanation of 0.9.0 breaking changes please about meteor-collection-hooks HOT 5 CLOSED

Any comment on this @mizzao? I would really like to make mizzao:partitioner compatible with meteor-collection-hooks 0.9+ because I am also suffering from #203 and in my case the entire user document can be 100s of kBs.

from meteor-collection-hooks.

I've been trying to merge PR #206 into my own fork of 0.8.4 to fix #203.

I've discovered that changing this one line in meteor-collection-hooks/update.js:

      if (aspects.before || aspects.after) {

to this:

      if (!_.isEmpty(aspects.before) || !_.isEmpty(aspects.after)) {

breaks mizzao:partitoner - an update which is only supposed to update users in one partition actually updates ALL users. Looks like this PR might have unintended side-effects...

from meteor-collection-hooks.

In the meantime, I have added a line to collection-hooks.js in v0.8.4:

CollectionHooks.getDocs = function getDocs (collection, selector, options) {
  const findOptions = { transform: null, reactive: false,
    {fields: {_id: 1}}, // I added this line
  }

This works because mizzao:partitioner doesn't actually need the documents in its before hooks, all it does is modify the selector. And nothing else in my app uses collection-hooks so this is safe for me.

I uploaded this to production about 12 hours ago. The result are as follows, showing method response time of my most common method over 24 hours:

You can see that this has halved my overall method response time, and will have drastically reduced the bandwidth from the database. There are still unnecessary fetches going on, but this is much better.

from meteor-collection-hooks.

Sorry @wildhart, I've been too busy with other stuff to look at this at the moment.

Based on your knowledge of this, where should the fix be? In collection-hooks or mizzao:partitioner?

from meteor-collection-hooks.

I'm really not sure to be honest. I suspect that partitioner was using a 'feature' of collection-hooks which meant that update operations were also using the find hooks and so didn't need their own hooks. For some reason I don't understand yet, adding the _.isEmpty(aspects...) breaks that 'feature' so that the update isn't limited to the current user's group.

So I don't know whether the 'feature' in collection-hooks was broken by PR #206 (and may break other people's code outside of partitioner), in which case collection-hooks needs to be fixed; or whether partitioner was making an unsafe assumption about the internal workings of collection-hooks, and that assumption is no-longer valid, in which case partitioner would need to be fixed.

I also have other priorities at the moment so I don't have much time to investigate further - I've implemented a temporary hack (as above) which mitigates the problem of fetching an entire document whenever it's updated so for the moment I'm happy. I may have more time to help identify the root cause in the new year...

from meteor-collection-hooks.