Meteor-Community-Packages / meteor-collection-hooks

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

Query regarding server bottleneck optimisations... #201

Closed Siyfion closed 7 years ago

Siyfion commented 7 years ago

I'm trying to debug my application to find the cause of a huge bottleneck in the server code and was hoping you might be able to clarify something for me...

First off, the info, here is the Kadira profile: https://ui.kadira.io/mt/86d5a109-6eff-4aa7-bf22-06a5ff556ee0/q56rPZsBwrGWWcykj

Note, I have no hooks at all on Users and no PrintLists hooks for find or findOne.

As such the expected behaviour would be:

  1. fetch on printLists
  2. fetch on user
  3. update on printLists
  4. update on quantityLists

Obviously there is a LOT more going on than just that... So I wondered if the collection-hooks might be responsible for the extra find/fetch's even when there is no hook defined to utilise the result?? Or am I clutching at straws?

And now the method, followed by the one hook that relates to PrintLists.

PrintLists.methods.addPrintItems = new ValidatedMethod({
  name: 'PrintLists.methods.addPrintItems',

  validate: new SimpleSchema({
    printListId: { type: String },
    items: { type: [Object] },
    'items.$.product': { type: String },
    'items.$.template': { type: String },
    'items.$.quantity': { type: Number },
  }).validator(),

  run({ printListId, items }) {
    const printList = PrintLists.findOne(printListId);
    if (!printList) {
      return throwError('not-found', 'The specified print list does not exist');
    }
    userIsOwner(printList);

    // For each item that is to be added to the print list
    _.each(items, (printListItem) => {
      // Check to see if there is already an entry in the
      // print list for this exact product & template combo
      const existingItem = _.findWhere(printList.items, {
        product: printListItem.product,
        template: printListItem.template,
      });
      if (existingItem) {
        // There's an existing entry, so just add the quantity
        existingItem.quantity += printListItem.quantity;
      } else {
        // There's no item for this combo so add one
        printListItem._id = Random.id();
        printListItem.overrides = [];
        printList.items.push(printListItem);
      }
    });

    // Overwrite the old items array with the new one
    PrintLists.update(printList._id, {
      $set: { items: printList.items },
    });
  }
});
PrintLists.after.update(function (userId, doc) {
  // Get a list of valid print list item ids
  var validPrintListIds = _.pluck(doc.items, '_id');

  // Remove all quantity list items that reference non-valid print list items
  QuantityLists.update(
    { printList: doc._id },
    { $pull: { items: { printListItem: { $nin: validPrintListIds } } } },
    { multi: true }
  );
}, { fetchPrevious: false });
zimme commented 7 years ago

Your assumption about collection-hooks doing unnecessary document fetching when no hooks are defined is correct, I'm working on this currently and hopefully we can get a new version out soon-ish.