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

Memory Leak #449

Closed
mahaben opened this issue Oct 21, 2019 · 19 comments
Closed

Memory Leak #449

mahaben opened this issue Oct 21, 2019 · 19 comments
Labels
Community 👨‍👧 Something initiated by a community Wontfix ❌ This will not be worked on

Comments

@mahaben
Copy link

mahaben commented Oct 21, 2019

Describe the bug
We are facing a memory leak issue. We have a graphql query that returns 1200 records and called every 10 seconds. Memory keeps increasing over time and never released when the query is stopped.

To Reproduce
-Create a simple entity
-Create a graphql query that returns 1000 records of this entity
-Create a graphql client that calls this query every 10 secs
-Inspect memory

Expected behavior
-Memory should be released

Logs
Capture d’écran 2019-10-21 à 11 09 31

Environment (please complete the following information):

  • Node (12.11.1)
  • Package version [^0.17.5]
  • TypeScript version (^3.6.4)

Thanks.

@MichalLytek MichalLytek added Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community Need More Info 🤷‍♂️ Further information is requested labels Oct 21, 2019
@MichalLytek
Copy link
Owner

Memory should be released

What memory? Metadata from decorators?

The only memory leak that could happen is that you construct the schema multiple times, so e.g. when you use lambda/serverless that shares the same Node.js process but executes modules evaluation and run the schema build on every request.

If you want me to tackle this issue, please help and provide more detailed info what does the metadata storage contains, e.g. if the fields that contains all collected metadata like resolvers has the same length as the number of your resolvers in app. Maybe you store some additional data in the class instances of static properties so the debugger shows that the metadata storage has 600 MB of data.

@mahaben
Copy link
Author

mahaben commented Oct 21, 2019

@MichalLytek The schema is constructed once at the server start not on every request.

You can use this repo to reproduce https://github.com/mahaben/graphql

Thanks.

@MichalLytek
Copy link
Owner

This repo has very outdated dependencies (type-graphql 0.15.0), no package lock and I can't build the app:

PS F:\#Projekty\#Github\typegraphql-memory-leak> npm run build

> graphql-typescript@1.0.0 build F:\#Projekty\#Github\typegraphql-memory-leak
> tsc

node_modules/subscriptions-transport-ws/dist/server.d.ts:2:28 - error TS7016: Could not find a declaration file for module 'ws'. 'F:/#Projekty/#Github/typegraphql-memory-leak/node_modules/ws/index.js' implicitly has an 'any' type.  
  Try `npm install @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

2 import * as WebSocket from 'ws';
                             ~~~~
Found 1 error.

@MichalLytek
Copy link
Owner

MichalLytek commented Oct 24, 2019

Fixed the errors, run the app, call many queries - cannot reproduce, everything looks ok, memory grows to 35MB and then it's garbage collected to 14MB.

Please create a full reproduction detailed instruction with repo, node version, steps to record memory, etc. Otherwise will close the issue as invalid.

@mahaben
Copy link
Author

mahaben commented Oct 25, 2019

Hello,

Maybe I was not clear about how to reproduce this bug using the repo. Have you followed these steps?

-Start the server in index.ts
-Take a snapshot of memory
-Start multiple clients using client.ts (10 for example)
-Wait about 1 hour
-Stop clients
-Take again a memory snapshot

Thanks,

@Marsup
Copy link

Marsup commented Oct 25, 2019

I've been able to reproduce this leak, but my best guess is that it's the same as nodejs/node#28420. There is no leak on node 10 while there's a massive one on node 12. Unless your library specifically triggers the leak, I'm afraid there's not much you can do.

@MichalLytek
Copy link
Owner

More ...Retained size shows 600MB

I think that there is a real memory leak problem. I can't reproduce this behavior using classic graphql. Any Idea?

Thanks.

Capture d’écran 2019-10-19 à 15 20 50

I tried to reproduce the issue but on Node v12.11.1 after an hour of querying I still have the same fixed size of TypeGraphQL metadata storage:

Code_2019-10-27_10-29-32

After stopping the queries it grew up for a few kBs. The overall memory consumption grows really big up to ~120MB (from initial 12MB) and then after disconnecting the client went to ~200MB 😕

So I think the issue is not related to TypeGraphQL (the amount of registered resolvers or fields is the same, no rebuidling the schema), maybe it's a Node.js version issue or some libs like Apollo Server and websockets, I don't know.

@MichalLytek
Copy link
Owner

Closing as no response from OP, I cannot reproduce and it might be Node.js version related issue 🔒

@MichalLytek
Copy link
Owner

@mahaben
Please check the newest v0.18.0-beta.7 - it has a build done for ES2018 target with async/await so it should be much more memory efficient 😉

@mahaben
Copy link
Author

mahaben commented Jan 9, 2020

@MichalLytek Thanks for the v0.18.0-beta.7, it seems to resolve the memory leak. But I get some error GraphQLError: Unknown type, validation failed when using inheritance. The error is not present using the release 0.17.6. Any idea of what changed between the two versions?

@MichalLytek
Copy link
Owner

MichalLytek commented Jan 9, 2020

Any idea of what changed between the two versions?

@mahaben Now we build only explicitly referenced types - you may need to put the interfaces or other types into the orphanedTypes array.

@mahaben
Copy link
Author

mahaben commented Jan 9, 2020

@MichalLytek Hmm, don't know what i'm doing wrong, here is my code:

Capture d’écran 2020-01-09 à 15 31 29

ActorTaskResolver is abstract and ActorTaskChooseGroupsResolver extends ActorTaskResolver

@MichalLytek
Copy link
Owner

MichalLytek commented Jan 9, 2020

orphanedTypes is not for resolvers!
It's for the case like you have an graphql interface class IPerson that you use as response type of users query and then two object type classes Student and Employee that implements that interface.
In the v0.18 pipeline they are not emitted in schema as not directly referenced, so the returned interface has no implementation found and you receive "Unknown type" I guess.

@mahaben
Copy link
Author

mahaben commented Jan 9, 2020

So what should I do to have these resolvers in the schema to avoid this error? Indeed, the resolvers are present in the schema generated in v0.17 and not in v0.18 🤔

@MichalLytek
Copy link
Owner

Put them in the resolvers array! 😠

@mahaben
Copy link
Author

mahaben commented Jan 9, 2020

😄 this is already the case !! but it doesn't work :( . Didn't change anything between the two versions

@MichalLytek
Copy link
Owner

Is it an abstract repository?

Maybe it will be faster for you to create a repository with a minimal reproducible code example (not a dump from your project codebase) and create a new issue with that.

@MichalLytek MichalLytek removed the Bug 🐛 Something isn't working label Jan 9, 2020
@mahaben
Copy link
Author

mahaben commented Jan 9, 2020

Thanks, I'll create a repo to reproduce.

@mahaben
Copy link
Author

mahaben commented Jan 9, 2020

@MichalLytek it's okay, solve it 🎉 . Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Wontfix ❌ This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants