Having multiple polling queries on the same QueryManager causes setInterval leak

[timbotnik]

I discovered this one via a React component (with Apollo connect container) that had multiple polling queries. Each time I unmounted & mounted the component, I would get a polling leak, as evidenced by increasingly frequent HTTP requests to my /graphql endpoint.

I put some console logging in this zone https://github.com/apollostack/apollo-client/blob/master/src/QueryManager.ts#L426-L455, like this:

  private startQuery(options: WatchQueryOptions, listener: QueryListener) {
    const queryId = this.generateQueryId();
    this.queryListeners[queryId] = listener;
    this.fetchQuery(queryId, options);
    if (options.pollInterval) {
      this.pollingTimer = setInterval(() => {
        const pollingOptions = assign({}, options) as WatchQueryOptions;
        // subsequent fetches from polling always reqeust new data
        pollingOptions.forceFetch = true;
        this.fetchQuery(queryId, pollingOptions);
      }, options.pollInterval);
+++   console.log('starting timer', this.pollingTimer)
    }
    return queryId;
  }

  private stopQuery(queryId: string) {
    // XXX in the future if we should cancel the request
    // so that it never tries to return data
    delete this.queryListeners[queryId];

    // if we have a polling interval running, stop it
    if (this.pollingTimer) {
+++   console.log('stopping timer', this.pollingTimer)
      clearInterval(this.pollingTimer);
    }

    this.store.dispatch({
      type: 'APOLLO_QUERY_STOP',
      queryId,
    });
  }

I mounted & unmounted the component twice, and the component sets up 3 polling queries on every mount. Here's the result:

As you can see it looks like we have lost (overwritten?) our references to timers 94, 128, 146, & 147.

[timbotnik]

@jbaxleyiii - I think the polling state needs to move to the individual query instead of on the query manager? Would it be better stored on the query handle in some way, perhaps as described here #194?

[helfer]

@jbaxleyiii Any update on this issue? It's currently blocking us from using Apollo client in production. If you want, @Poincare could also look into it.

[jbaxleyiii]

@helfer I haven't actually gotten any time to look into this. I'm working on rebuilding our production server currently. I'd love @Poincare to take a look at it and especially at #194. There will need to be some changes to react-apollo after #194 is done too fwiw

[timbotnik]

I tried a work-around for this by making my own timer and using refetch instead of pollInterval. Then I ran into apollographql/react-apollo#61, which is a differently bad behavior.

[Poincare]

Sure, I'll look into this in detail tomorrow and hopefully make some headway on some kind of fix.

[timbotnik]

I think there is another subtlety to be aware of here. If your query takes LONGER than the polling interval, it seems the query will re-fire before you receive any results, and even when the original request completes, the data never gets propagated because another (identical) query is already in-flight. I don't know if the fix for this would be in apollo-client or react-apollo, just something to consider if refactoring the polling interval code.

[Poincare]

I think that issue is currently mentioned in the code as something to fix. It seems the polling structure requires some pretty significant changes so that (1) polling doesn't leak (2) we can keep track of the queries that have been sent to the server (probably by query id) and don't re-fire the same query again.

I think fixing this issue is actually pretty intimately connected to the batching over HTTP since I've been building a tick-based scheduler-like system for that anyway. Hopefully, that will make it easier to resolve this issue and others that involve the current polling system.

[jbaxleyiii]

@timbotnik while @Poincare fixes the polling setup, I think I fixed apollographql/react-apollo#61 with 0.3.7. Could you see if that works?

[helfer]

I think for now it's fine to keep polling even if some query is still in flight, but we should fix the bug where the UI doesn't update when a query returns if there's still some polling query in flight.

[timbotnik]

@Poincare @jbaxleyiii Now that apollographql/react-apollo#61 is fixed, I have a viable work-around for the polling issue, which is to manage my own timer per component and call refetch myself. This also enables me to check if it's still in the “loading” state, and not refire the query if it's still in-flight. I think that means I'm unblocked on this issue, but we still need to address this.

[Poincare]

Great to hear. I have a fix working for the polling timer issue but I'm still working on the unit test that should be able to reproduce this issue.

I think it will be valuable to have this unit test working since the complexity of the polling mechanism will only increase and having a template to test the polling queries will be helpful.

[bjeanes]

I am on apollo-client 1.4.0 and I think I'm experiencing this issue as described above. I am introducing React into an existing codebase so have multiple top-level components. One of the components I am mounting fetches data via two separately graphl()-wrapped queries, only one of which is polling. When I re-mount the top-level component, I seem to have poll leaks. I tested this by purposefully un-mounting and re-mounting my component several times. Despite my 5000ms pollInterval resulted in queries, I'm seeing the same query firing every ~1 second.

I'm not sure how to debug this really as I'm only a day into my experience with Apollo and I may just be doing something obviously wrong.

[wailorman]

I've encountered the same issue on react-apollo@1.4.15. I am using old ejected create-react-app (with webpack@1) and making requests through webpack proxy.
All was great before I set pollInterval to all @graphql()-wrapped components. After that, my bundle started randomly crashing or infinitely reloading when I made some changes in files. In Chrome task manager my react app becomes using up to 1,6G. Some times after reaching of 1,6G app starts loading normally for the first time. The fastest escape of this issue was to close a frozen tab and open a new one.

I solve this problem only by this way: Do not use webpack proxy if you have some polling components, just send graphql requests directly to the server. In my case, the time of page reloading increased by multiple times after that.

Maybe this issue also can be solved through apollo-link-retry in Apollo v2.

I've recorded a snapshot just in case.