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

Another asynchronous resolution PR. But using JS-like promises instead of goroutines. #357

Closed
wants to merge 1 commit into from

Conversation

ccbrown
Copy link
Contributor

@ccbrown ccbrown commented Jul 18, 2018

Like everyone else, I'm in need of batching, deduplication, and concurrency in my query execution.

But the other pull requests all look like they utilize goroutine-based approaches. This pull request does not use goroutines. Instead, it works a bit more like the JavaScript reference implementation using a notion of promises. Here's how it works in terms of usage:

Resolvers that can execute asynchronously return a channel. To be explicit, a graphql.ResolvePromise. So you might have a loader that creates and returns a new graphql.ResolvePromise like so:

{
  Resolve: func(p graphql.ResolveParams) (interface{}, error) {
    id, _ := p.Args["id"].(string)
    loader := p.Context.Value("loader").(*Loader)
    return loader.LoadNodeByGlobalId(id)
  },
}
func (l *Loader) LoadNodeByGlobalId(id string) (graphql.ResolvePromise, error) {
  promise := make(graphql.ResolvePromise, 1)
  ...
  l.pendingNodes[id].Promises = append(l.pendingNodes[id].Promises, promise)
  return promise, nil
}

How does the loader fulfill the promise without a goroutine? When the query execution gets "stuck" and there are no more resolvers that it can invoke, it invokes an "idle handler" to let it know that one or more pending promises need to be fulfilled before it can continue. So your params can now specify a scheduler for your loader like so:

p.IdleHandler = loader.Schedule

When the handler is invoked, it expects one or more promises to be fulfilled. Afterwards, it'll resume resolving values until it gets stuck again.

So your loader implementation might at a high level do something like this:

  • If one or more pending node is cached, fulfill its promises, then return.
  • If one or more pending node is for whatever reason not batchable, fulfill its promises, then return.
  • Fulfill promises for a batch of nodes, then return.

And so in this way, you can really control when and how your resolutions happen. And there are no goroutines involved or multithreading concerns. Of course, you can use goroutines if you want to though.

@chris-ramon
Copy link
Member

chris-ramon commented Jul 19, 2018

This is really awesome @ccbrown! thanks a lot for taking the taking and working on it.

  • graphql-go follows the reference implementation graphql-js in a parity strategy.
  • Although following Promises based pattern for handling async resolvers matches the graphql-js parity, I'm thinking that might not be a simple API for graphql-go users, since Go does not have Promises natively, therefore Go users are not familiar with it; instead we have goroutines and the tooling around it.
  • Said that, I'm thinking that something like a previous PR proposed, would match your use cases somehow ?:
Resolve: func(p graphql.ResolveParams) (interface{}, error) {
  return func() (interface{}, error) {
    // async work
  }, nil
},

@ccbrown
Copy link
Contributor Author

ccbrown commented Jul 19, 2018

@chris-ramon

I'm thinking that might not be a simple API for graphql-go users, since Go does not have Promises natively, therefore Go users are not familiar with it

It might feel more at home with Go users if we just used the term "channel" instead of "promise". I don't think this sort of channel usage should feel too foreign to anyone interested in concurrency. The really JS-like promise stuff is hidden away from the public API and not expected to be seen or used by users.

I'm thinking that something like a previous PR proposed, would match your use cases somehow ?

There are two downsides to that approach that I really want to avoid:

  • It requires a new goroutine to be created for each field that returns a function. On a busy server, this is significantly more expensive than creating a channel.
  • It requires batching to be done with either a timeout or a pending node threshold. There's no way for a dataloader to know precisely when no more nodes will be requested of it, so the batching algorithm can never be optimal.

@chris-ramon
Copy link
Member

Thanks a lot @ccbrown! 👍 — Closing this one in favor of #388

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

Successfully merging this pull request may close these issues.

None yet

2 participants