cult-of-coders / grapher

Grapher: Meteor Collection Joins + Reactive GraphQL like queries
https://atmospherejs.com/cultofcoders/grapher
MIT License
275 stars 53 forks source link

Observers are not all cleaned up on named queries (?) #416

Closed joerou closed 3 years ago

joerou commented 4 years ago

Hello,

Great Package!

I am not 100% sure this is a problem with this package or the code I wrote around it, but was hoping someone here might be able to spot the problem. I'm using the logs in Apm.meteor.com and see that I have quite a lot of observers that aren't getting cleaned up, which would suggest a memory leak and they seem to only be happening with the named queries, which results in documents continuing to be fetched even after all active subscriptions have gone away.

I don't have any hidden business logic, so I just use a simple json structure like so to create my queries and then wire them up and include that on both server and client (using the expose function in a Meteor.isServer block) (see also below)

As you can see in the attached screen shots, There seems to be some small leak on some publications (ex. 21 observers created, 17 removed and consistently less removed over a 24 hours period) and on others there just is none being removed ever. and documents continue to be fetched with no active subs.

Screen Shot 2020-03-12 at 3 18 51 PM Screen Shot 2020-03-12 at 3 18 33 PM Screen Shot 2020-03-12 at 3 18 24 PM
const queries = [
  {
    name: 'ownProjects',
    returns: '[Project]',
    publishedFields: publishedFieldsListView,
    auth: true,
    isAdmin: false,
    defaultSort: { title: -1 },
    filter: function(where, userId) {
      const query = where !== 'undefined' ? { ...where } : {};
      query.userId = userId;
      return query;
    },
  },
];
export const wireQuery = query => {
  const queryClone = { ...query };
  const { publishedFields, defaultSort } = queryClone;

  queryClone.query = queryClone.collection.createQuery(queryClone.name, {
    $filter({ filters, options, params }) {
      _.each(params, function (param, key) {
        if (key === 'sort') {
          options.sort = param;
        } else {
          filters[key] = param;
        }
      });
    },
    $options: { defaultSort },
    $paginate: true,
    ...publishedFields,
  }, {
    scoped: true,
  });

  // Expose grapher query on the server only create graphql query and related schema
  if (Meteor.isServer) {
    queryClone.query.expose({
      firewall(userId, params) {
        if (queryClone.auth === true && !userId) {
          throw new Meteor.Error(403, 'Unauthorized');
        }

        if (queryClone.isAdmin === true && !Roles.userIsInRole(userId, 'administrator')) {
          throw new Meteor.Error(403, 'Unauthorized');
        }

        let addedParams = params !== 'undefined' ? { ...params } : {};
        if (_.isFunction(queryClone.filter)) {
          addedParams = queryClone.filter(params, userId);
        }

        _.each(addedParams, function (param, key) {
          params[key] = param;
        });
      },
      publication: true,
    });

    graphQLGrapherQueries[queryClone.name] = makeGraphQLQuerySchema(queryClone);

    // Write the graphQL resolver for this publication
    const graphQLResolver = async (root, { where, page, limit, sort }, context, ast) => {
      // Insert the userId so Meteor publish functions work as normal
      // Return null if user doesn't exist
      if (queryClone.auth === true && !context.user) {
        return null;
      }

      // Always set the context userid, if it doesn't exist make empty string
      if (context.user) {
        queryClone.userId = context.user._id;
      } else {
        queryClone.userId = '';
      }

      if (queryClone.isAdmin === true && !Roles.userIsInRole(queryClone.userId, 'administrator')) {
        return null;
      }

      // Run filter function again with where from graphql query
      let params = where !== 'undefined' ? { ...where } : {};
      if (_.isFunction(queryClone.filter)) {
        params = queryClone.filter(where, queryClone.userId);
      }

      const skip = page ? (page - 1) * limit : 0;
      const graphqlSort = sort || defaultSort;

      // Check the return type
      const returnType =
        typeof queryClone.returns === 'string'
          ? queryClone.returns
          : queryClone.returns.name;

      let returnValue = queryClone.collection.astToQuery(ast, {
          $filters: params, // Mongo Filters/Selector
          $options: {
            sort: graphqlSort,
            skip,
            limit,
          }, // Mongo Options
          intersect: publishedFields,
          // Automatically enforces a maximum number of results
          maxLimit: 100, // Integer
      }).fetch();

      // If return type is not an array, and the return is an array, return first item
      if (returnType[0] !== '[' && Array.isArray(returnValue)) {
        returnValue = returnValue[0];
      }

      return returnValue;
    };

    graphQLGrapherResolvers[queryClone.name] = graphQLResolver;
  }

  // If is on client then take care of wiring up the subscriptions as well
  if (Meteor.isClient) {
    queryClone.subscription = wireSubscription({
      name: queryClone.name,
      query: queryClone.query,
      returnsSingle: queryClone.returns[0] !== '[',
    });
  }

  return queryClone;
};
theodorDiaconu commented 3 years ago

It seems you are using graphql subscription? Maybe you aren't doing the proper clean-up.

joerou commented 3 years ago

The proper cleanup? on react unmount I am stopping the subscriptions, is there something that I'm missing?

theodorDiaconu commented 3 years ago

@joerou help me clarify are you subscribing via GraphQL subscriptions? If so, do you have a mechanism to switch off Meteor subscriptiosn from GraphQL (server-level)?

joerou commented 3 years ago

Well, I'm using the subscription method from your package, same as the api docs query.subscribe() In this case I'm wiring everything up through your package. I didn't see the need in the docs to switch off Meteor subscriptions from GraphQL on the server.

To be completely honest, I had to switch back to plain Meteor subscriptions and not use your package, but I think there's some other's following this thread, so I'm happy to trouble shoot and document for the future. If this is really not worth your time, I'm happy to leave it until someone else reopens the thread.. Thanks for taking the time!