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

[feature] Prepare each level before fetching #1052

Closed
andremussche opened this issue Feb 20, 2020 · 1 comment
Closed

[feature] Prepare each level before fetching #1052

andremussche opened this issue Feb 20, 2020 · 1 comment

Comments

@andremussche
Copy link

What happened?

When using the "dataloaden" dataloader, we get:

  1. unneeded delays (e.g. waiting 10ms per data fetch)
  2. unpredictable and timing related behavior

Regarding 1: when 2 objects need to be fetched, e.g. using this query:
{
first: todo(id:1){
text
second: todo(id:2){
text
}
then "dataloaden" always waits 10ms (this is our current setting) before fetching both items.
This is unneeded delay for simple queries and quickly sums up when we add more levels in more complex queries.

Regarding 2: when the server is idle, then a 10ms delay is overkill. But when the server gets busy, it is likely to get a context switch of 15ms so an inefficient fetch is done because of pending go routines not added to the queue. So the same graphql query gives different sql queries to the database, so less query plan reuse on the sql server, so less and unpredictable performance, depending on the load (probably less and less performance when the server gets more busy because of different chatty calls). This makes it nearly impossible to reproduce and test a performance issue in production.

Also understanding the behavior of the "dataloaden" component is difficult, because it is dependent on internal go routines of gqlgen and it has it's own go routines and closures.

What did you expect?

  1. no delay for simple queries and no recursive delay for queries with more levels
  2. predictable and reproduce-able behavior, regardless of the load
  3. always combining data fetches (no chatty calls), regardless of the load

Minimal graphql.schema and models to reproduce

See https://github.com/ultraware/gqlgen/pull/1/files#diff-5f72172a2f4e75b463620ea9c3aae88a for extended todo schema.graphql

Proposal

What I tried to do in this PoC (https://github.com/ultraware/gqlgen/pull/1/files#) is to prepare each level before fetching.
This way, you always have all id's that need to be loaded for the next level.
These id's are stored per object, so when more then one object type is queried on the same level, the data is loaded in parallel.
To make it easy to use and less error prone regarding preparing, I embedded the dataloader part into gqlgen, so the user only has to implement one array load function (func([]ids)[]objects).

The only (?) downside is, is that each level is processed multiple times (when a prepare is done, it returns to the root query and descends all levels again till everything is fetched). For that (and other reasons) I use a field context cache.

versions

gqlgen version = v0.10.2-dev
go version = go1.13.1 windows/amd64
github.com/vektah/dataloaden v0.2.1-0.20190515034641-a19b9a6e7c9e

@vektah
Copy link
Collaborator

vektah commented Mar 5, 2020

dupe of #518

@vektah vektah closed this as completed Mar 5, 2020
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