Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible memory leak when using multiple queries on the same page #3507

Closed
3 tasks done
juusopiikkila opened this issue Feb 17, 2024 · 15 comments · Fixed by #3612 or #3619
Closed
3 tasks done

Possible memory leak when using multiple queries on the same page #3507

juusopiikkila opened this issue Feb 17, 2024 · 15 comments · Fixed by #3612 or #3619

Comments

@juusopiikkila
Copy link

Describe the bug

When using two queries on the same page and the other one is awaited and the other one is not and that one is using data from the awaited query, there is a memory leak.

Using the reproduction memory usage climbs from ~40 Mb up to ~800 Mb (autocannon with 60 sec duration and 10 connections) and it never comes down.

Reproduction

https://github.com/juusopiikkila/nuxt-urql-memory-leak

Urql version

@urql/vue v1.1.2

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct
@kitten
Copy link
Member

kitten commented Feb 18, 2024

Just to clarify before looking into this because that's an important distinction: Does it never come down and stay stuck at 800MB (I assume MB not Mb?) or does it crash?

If your server doesn't actually rise in usage and/or crash it's not a leak, so just clarifying here.

Also just to point this out, there isn't anything special going on here. If you have just two operations (ie two document + variables combos) then it's more of a question how many concurrent queries are actually run as no memory is retained in the core client, bindings, or exchanges 🤔

@juusopiikkila
Copy link
Author

Yeah sorry I meant MB. I tested it again and when the test was completed memory usage was at ~800MB and then it slightly decreased to ~700MB. No crashes, still works fine at that point when manually checking with browser. Memory usage just stays at ~700MB while I waited for 10 minutes for it to come down.

@kitten
Copy link
Member

kitten commented Feb 18, 2024

Hm, ok gotcha 👍 That doesn't necessarily sound like a memory leak, but a Chrome debugger memory snapshot could show you what's being retained and what's using that memory.

Generally, it's possible that a combination of GraphQL result data and Vue app tree data is being retained concurrently. But it's hard to tell how much memory that adds up to. Generally, 800MB could be plausible given that the GC may choose to also not activate an aggressive sweep phase when it still has enough heap space left

@juusopiikkila
Copy link
Author

Ok I've now updated the reproduction repo with clearer tests. It wasn't even about using the data from the first query in the second, it leaks with just two awaited queries. With just one query there isn't any leak. Also I added a mock API endpoint under the api routes so that the external service isn't affecting the results.

I also set max_old_space_size to 512MB to see if it changed anything.

Below are the results from the autocannon tests. I'm not that good at decyphering the snapshots yet so that doesn't help me much. Those memory readings are taken from the Chrome debuggers memory view after each test.

# /no-leak

11k requests in 60.04s, 17 MB read
Memory usage: 23.8 MB

# /leak

9k requests in 60.04s, 13.2 MB read
Memory usage: 1201 MB

@kitten
Copy link
Member

kitten commented Feb 20, 2024

Gotcha! That's awesome. I'll have a look at the memory debugger myself this evening. Typically, I'd assume we've created some kind of promise chain or cycle here 🤔

@juusopiikkila
Copy link
Author

Have you had time to check this one out yet?

@Bcavez
Copy link

Bcavez commented Mar 26, 2024

I can confirm we have the same issue on our project.
Using @urql/vue v1.1.2 with nuxt 3.

Any page/component which has two queries will retain all the setup context of the component in memory and it is never released.
image
image
image

For a while I thought it was not an issue with urql as it is not just urql objects like graphcache, queries and data that are retained but also other objects like router object, unhead object etc... Basically anything in the setup function.

But if I remove the second query, it always fixes the problem, on all of the pages/components.

@kitten Have you had a chance to look at this issue yet ?

@negezor
Copy link
Contributor

negezor commented Jun 17, 2024

It seems that I'm in the same boat. My SSR is failing and getting killed by OOM. I've been profiling for a long time, but the only clues I have point to the Store object from graphcache. I'm not entirely sure how to verify this, as I don't see any obvious references where it could be stored.
image

I had to increase the amount of memory to reduce the crash rate.
image

@JoviDeCroock
Copy link
Collaborator

@negezor are you by any chance initializing your graphCache globally? If so you might want to move that to your boostrapping code so that it can be garbage collected when your SSR completes.

@negezor
Copy link
Contributor

negezor commented Jun 17, 2024

@JoviDeCroock no. I have a new instance of the application initialized for each request. Including graphcache, otherwise the same data would be sent for each user and it would be a data access violation.

@negezor
Copy link
Contributor

negezor commented Jun 17, 2024

After investigating further, I seem to have some ideas about why the memory leak is happening. For some reason, the request object, which is not being mutated on my end, has a large number of ReactiveEffect.
Screenshot 2024-06-18 040625

Apparently, this is happening here.

const args = reactive(_args) as UseQueryArgs<T, V>;

UPD: By applying markRaw(QueryDocument) from vue the memory leak disappeared. But I don't think this should be the solution, we should get rid of reactive() for args.

@yurks
Copy link
Contributor

yurks commented Jun 18, 2024

@negezor, #3612 completely breaks reactivity support for variables, like { variables: { somevar: ref('value') }}, as reactive() unwraps all nested reactive props

@yurks
Copy link
Contributor

yurks commented Jun 18, 2024

@JoviDeCroock @negezor
this issue should be resolved in #3619 without side-effects

@Warxcell
Copy link

I have consistent memory leak, but I don't know where from. Memory raises from 300MB to 4.5GB (which is maximum of the server itself) and then crashes with OOM error. Waiting for release to test if leak was coming from here. :)

@Warxcell
Copy link

Warxcell commented Jul 26, 2024

Confirmed. After upgrading to 1.4.0-canary-068df71f - Now memory stays at ~200MB.

Edit: But when deployed on production - memory again grows. Probably I have another memory leak. lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants