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

Slow response times with large documents #144

Closed
Mithras opened this issue Nov 15, 2021 · 9 comments
Closed

Slow response times with large documents #144

Mithras opened this issue Nov 15, 2021 · 9 comments
Assignees

Comments

@Mithras
Copy link

Mithras commented Nov 15, 2021

Describe the bug
Performance on large responses is even worse than with gateway. This makes federation unusable if you need to query large amounts of data.
E.g.
router (it doesn't gzip but it's all running on localhost so size is not the reason it's 10 times slower):
image
gateway:
image
direct:
image

To Reproduce
Steps to reproduce the behavior:

  1. Setup subgraph that returns a few Mb of any objects.
  2. Query it directly
  3. Query it through gateway/router. Gateway adds over 2x overhead. Router adds over 10x overhead.

Expected behavior
Router overhead at least shouldn't be worse than gateway.

Desktop (please complete the following information):

  • OS: node:
  • Version: 16-bullseye

Additional context
See graphql/graphql-js#723

I think the TL;DR of this issue is that GraphQL has some overhead and that reducing that overhead is non-trivial and removing it completely may not be an option. Ultimately GraphQL.js is still responsible for making API boundary guarantees about the shape and type of the returned data and by design does not trust the underlying systems. In other words GraphQL.js does runtime type checking and sub-selection and this has some cost.

It would be amazing to have an option to disable whatever "guarantees" gateway/router is "responsible" for. I can guarantee everything I need at a subgraph level, I don't need gateway/router to double guarantee it at a 10x cost.

@Mithras Mithras added the triage label Nov 15, 2021
@Geal
Copy link
Contributor

Geal commented Nov 16, 2021

hi! Just to check something: do you do a few queries to the router first, or launch the benchmark directly on it? It includes a caching step for the query planning, and that step can take a while, but afterwards queries are much faster.

@Mithras
Copy link
Author

Mithras commented Nov 16, 2021

The difference between the first query and subsequent queries is like 46 seconds vs 45 seconds. And that's while underlying subgraph takes only 3 seconds. Again the problem here is that both gateway and router overhead scales exponentially with response size.
Check out the workaround I described here: graphql/graphql-js#723 (comment)

@Geal
Copy link
Contributor

Geal commented Nov 16, 2021

I see. Another thing to check, this is with the router from the release package, not built from source?

I'll look into those perf issues

@Mithras
Copy link
Author

Mithras commented Nov 16, 2021

Yes, this is what I used for testing:

FROM debian:bullseye

WORKDIR /opt/apollo-router

RUN apt-get update \
    && DEBIAN_FRONTEND=noninteractive apt-get install -y curl \
    && curl -OL https://github.com/apollographql/router/releases/download/v0.1.0-alpha.0/router-0.1.0-alpha.0-x86_64-linux.tar.gz \
    && tar -xf ./router-0.1.0-alpha.0-x86_64-linux.tar.gz

CMD ["/opt/apollo-router/dist/router"]

@Geal
Copy link
Contributor

Geal commented Nov 16, 2021

Alright. What's the shape of the data? A few very large fields, lots of different fields, a deeply nested structure ?

@Mithras
Copy link
Author

Mithras commented Nov 16, 2021

I'm pretty sure it'll repo on any big collection. Here is a good investigation graphql/graphql-js#723 (comment) on this.

@Geal
Copy link
Contributor

Geal commented Nov 22, 2021

looks like it's spending most of the time on deserialization (tested on main at d9b3c43)

Screenshot from 2021-11-22 10-59-09

@Geal
Copy link
Contributor

Geal commented Nov 22, 2021

@Mithras could you test again building the router from the main branch? #172 should have improved things

@Geal Geal self-assigned this Nov 22, 2021
@Geal
Copy link
Contributor

Geal commented Dec 1, 2021

@Mithras I'm closing this now, let me know if the problem appears again

@Geal Geal closed this as completed Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants