Investigate potential memory leaks in SSR scenarios

[phryneas]

Over time, we had different reports of potential memory leaks in SSR scenarios.

I'm going to spike out for a while and try to create some benchmarks and test scenarios that allow us to detect such cases.

See this Milestone for a list of related Apollo Client PRs: https://github.com/apollographql/apollo-client/milestone/37

Related "external" PRs for visibility:

• add remove method to Trie benjamn/wryware#553
• Make Cache instance/class user-configurable via options.cache benjamn/optimism#615
• Add configurable WeakCache implementation based on FinalizationRegistry benjamn/optimism#599 (superseded)
• Introducing a new @wry/caches package, exporting StrongCache and WeakCache benjamn/wryware#568
• WeakCache: delayed finalization of nodes benjamn/wryware#569

External discussions: react-native-community/discussions-and-proposals#741

[rflament]

Not sure if this is the right place to report this, but we're investigating a memory issue in our next.js server and one of the cause seems to be SSR + useLazyQuery. It looks like even if the options ssr: false is passed to useLazyQuery, the QueryInfo is added to the queries set of QueryManager in the function watchQuery, but since the query is never executed on the server side, it stays in the queries variable forever.

As we use one single instance of ApolloClient, the queries variable of the QueryManager grows until OOM.

[phryneas]

@rflament Thank you for the report, I'll look into that!

As we use one single instance of ApolloClient, the queries variable of the QueryManager grows until OOM.

Independently of the outcome of that - please never do that! You will end up mixing private data of multiple users. When you're doing SSR, please create a new instance of Apollo Client for every request!

[rflament]

Thanks!

In our case the data fetched on the SSR side do not depend on the user, we don't pass the cookies to the apollo client, so the fact that the cache would be reused in all requests seemed a good thing.

[phryneas]

I'd still recommend the single store per request, since (as you noticed), Apollo Client isn't really tested for potentially thousands of requests per second on one instance, but only for "normal application usage" scenarios.

[phryneas]

@rflament I've moved this out into #11445. This is a bigger conceptual problem and might need quite a lot of reworking of internals, so I can't give you a timeframe for this at this point.

So far, my recommendation is really to create a new AC instance per request.

[phryneas]

The beta is out, and we're now waiting for feedback to dial in the right default cache limits.

For everyone who was following along: please give it a try and report back - you can find all the details in this blog post: https://www.apollographql.com/blog/apollo-3-9-beta-feature-spotlight-the-memory-story

[rflament]

I confirm that creating a new client instance per request fixed the memory leak, thanks!

[phryneas]

Going to close this with the release of 3.9.

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.