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 usage grows to extremely large values for queries that return big JSONs #456

Closed
Crhaj opened this issue Nov 5, 2019 · 5 comments
Closed
Labels
Community 👨‍👧 Something initiated by a community Duplicate 🔑 This issue or pull request already exists

Comments

@Crhaj
Copy link

Crhaj commented Nov 5, 2019

Describe the bug
Recently, we introduced this library to our GraphQL service which was using string schema representation until now. After this change task running this service on aws ecs started crashing regularly due to running out of memory. While debugging locally I noticed that the related node process goes from ~20mb when idle, to values close to 500mb (or even more) when processing query that returns large json file (2 to 3 mb). When I run the same query with version of the app before introducing type-graphql memory still rises more than I would like but instead of hundreds of megabytes it rises approximately by 20mb. The issue happens every time I run the query not just the first time.

To Reproduce
I created a minimal example repository https://github.com/Crhaj/type-graphql-memory where you can play around and see the issue is real. I've included most of the libraries we use so that you can see if it isn't issue of some interaction. This example app has only one query resolver which fetches large array of jsons (random data from some test site). In master there is version with type-graphql library but I've also created branch which is not using type-graphql to demonstrate the difference. Each of the branches contains reports (flamegraph) created by the 0x tool. When I run this app locally and run the query using type-graphql version the memory usage of the node process goes over 1gb which is simply not usable. Without the library it goes to ~60mb. Once the app starts, you can run the query easily from graphql playground like this:
query exampleData { exampleData { country indicator value year } }

Expected behavior
I expect the memory usage should be close to values produced by app which is not using the library when there are no other differences.

Enviorment (please complete the following information):

  • OS: macOS Mojave 10.14.6 (18G103), also happens in our docker containers
  • Node: 11.12.0 (tried also on 12.11.1 - same issue)
  • Package version: 0.17.5
  • TypeScript version: 3.1.6

Additional context
Any help appreciated, thank you.

@MichalLytek
Copy link
Owner

Looks like a duplicate of #254 - TypeGraphQL builds a reach field resolver for each field, so when you return big JSON, it creates tons of class type instances to call the getters with all midlewares pipeline.

@MichalLytek MichalLytek added Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community Discussion 💬 Brainstorm about the idea Need More Info 🤷‍♂️ Further information is requested labels Nov 5, 2019
@theodorDiaconu
Copy link

@MichalLytek nice. Should we add a way to opt-out of the middlewares ? Have a sort of context variable/symbol, that states the request is middlewareless?

@MichalLytek
Copy link
Owner

Should we add a way to opt-out of the middlewares ?

What do you mean? @Field({ simpleResolver: true }) or @ObjectType({ simpleResolvers: true }) will results in directly calling root[fieldName] without calling any global middlewares.

@theodorDiaconu
Copy link

@MichalLytek what you mentioned simpleResolver I fail to see any documentation of it. If it's pending, then it would be alright. However I would inverse control at a higher level. Like @Crhaj mentioned, there can be situations where I need a big load of data, I want to have almost 0 overhead from type-graphql.

It can be marked at Query/Mutation level, maybe via context injection?

ctx.skipMiddlewares();

@MichalLytek
Copy link
Owner

@theodorDiaconu
Closing in favor of #254 - let's discuss that only in one place 🔒

@MichalLytek MichalLytek added Duplicate 🔑 This issue or pull request already exists and removed Bug 🐛 Something isn't working Discussion 💬 Brainstorm about the idea Need More Info 🤷‍♂️ Further information is requested labels Nov 11, 2019
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 Duplicate 🔑 This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants