[RFC] Concurrent Resolvers

[chris-ramon]

Concurrent Resolvers

• At the moment we don't have built-in functionality to run resolvers concurrently.

• We want to integrate built-in, opt-in, performant and simple solution to run resolvers concurrently.

Current Implementation

• Resolvers are executed synchronously. Therefore not possible to run Goroutines within resolvers.

Current alternative solutions

• Run Goroutines before graphql.Do:
  • Execute async work based on requested fields using graphql's visitor and pass resolved data as RootObject or via context. This might be helpful solution even after integrating concurrent resolvers support to the lib, a working example explaining this solution can be found in the following gist: https://gist.github.com/chris-ramon/78027c8c0b283bfd1d20d6d989485e1d

Proposed solutions

Note: Thunk is a function that is returned from a function.

1) Resolver returns a Thunk

✔️ PR: #388
✔️ Opt-in — Via thunks
✔️ Performant — No additional goroutines
✔️ Simple — Uses existing public API
✔️ Up-to-date
Example:

"pullRequests": &graphql.Field{
	Type: graphql.NewList(PullRequestType),
	Resolve: func(p graphql.ResolveParams) (interface{}, error) {
		ch := make(chan []PullRequest)
		// Concurrent work via Goroutines.
		go func() {
			// Async work to obtain pullRequests.
			ch <- pullRequests
		}()
		return func() interface{} {
			return <-ch
		}, nil
	},
},

2) Resolver returns a Goroutined Thunk

✔️ PR: #213
✔️ Opt-in — Via thunks
❗ Performant — Goroutine per resolver
✔️ Simple — Uses existing public API
✔️ Up-to-date
Example:

"pullRequests": &graphql.Field{
	Type: graphql.NewList(PullRequestType),
	Resolve: func(p graphql.ResolveParams) (interface{}, error) {
		ch := make(chan []PullRequest)
		// Concurrent work via Goroutines.
		go func() {
			// Async work to obtain pullRequests.
			ch <- pullRequests
		}()
		return func() (interface{}, error) {
			return <-ch, nil
		}, nil
	},
},

3) Resolver returns a Channel

✔️ PR: #357
✔️ Opt-in — via new public APIs
✔️ Performant — No additional goroutines
❗ Simple — New set of JavaScript promises-like public APIs
✔️ Up-to-date
Example:

ch := make(chan []PullRequest)
// Concurrent work via Goroutines.
// Async work to obtain pullRequests.
graphql.Do(graphql.Params{
	Schema:        schema,
	RequestString: tc.RequestString,
	IdleHandler: func() {
		ch <- &graphql.ResolveResult{
			Value: pullRequests,
		}
	},
})

// ...
"pullRequests": &graphql.Field{
	Type: graphql.NewList(PullRequestType),
	Resolve: func(p graphql.ResolveParams) (interface{}, error) {
		return graphql.ResolvePromise(ch), nil
	},
},

4) Resolver's field flag

✔️ PR: sogko#23
✔️ Opt-in — via Field.Parallel
❗ Performant — Goroutine per resolver
✔️ Simple — Uses existing public API
❗ Up-to-date
Example:

"pullRequests": &graphql.Field{
	Type: graphql.NewList(PullRequestType),
	Parallel: true,
	Resolve: func(p graphql.ResolveParams) (interface{}, error) {
		ch := make(chan []PullRequest)
		// Concurrent work via Goroutines.
		go func() {
			// Async work to obtain pullRequests.
			ch <- pullRequests
		}()
		return <-ch, nil
	},
},

5) Built-in Goroutined Resolver

✔️ PR: #132
❗ Opt-in — Built-in support
❗ Performant — Goroutine per resolver
✔️ Simple — Uses existing public API
❗ Up-to-date
Example:

"pullRequests": &graphql.Field{
	Type: graphql.NewList(PullRequestType),
	// Resolve runs async
	Resolve: func(p graphql.ResolveParams) (interface{}, error) {
		var pullRequests []PullRequest
		// Async work to obtain pullRequests
		return pullRequests, nil
	},
},

6) Goroutined Resolver via Dataloader

✔️ PR: #154
✔️ Opt-in — Via new public APIs
✔️ Performant — No additional goroutines
❗ Simple — New public APIs
❗ Up-to-date
Example:

dl := dataloader.DataLoader{
	pullRequestsLoader: dataloader.New(sch, dataloader.Parallel(func(key interface{}) dataloader.Value {
		var pullRequests []PullRequest
		// Async work to obtain pullRequests
		return dataloader.NewValue(pullRequests, nil)
	})),
}

var loaderKey = struct{}{}
ctx := context.WithValue(parent, loaderKey, dl)

graphql.Do(graphql.Params{
	Schema:        schema,
	RequestString: query,
	Context:       ctx,
	Executor:      &executor,
})

[edpark11]

@chris-ramon -- thanks for writing this up! I have tried #388 (which I contributed to) and #213 and I believe they both work for my purposes. One potential drawback of both of them is that because golang doesn't have something like Javascript's event loop, it seems impossible to have more fine-grained control over the timing of when dataloader batches fire because doing so would require knowledge of where we are in the execution tree. However, for all practical purposes, Nick Randall's dataloader time/size-based algorithms appears to work in practice with both #388 and #213. In the future, I think that some of the concepts from #357 and #154 can be applied on top of #388 (i.e., introducing a concept of a scheduler), but I think the big win near-term is simply batching most like requests together in a reasonably thoughtful way. FWIW, I think the reason for #388 over #213 is what @jjeffery pointed out-- it unnecessarily introduces goroutines directly into the executor, possibly with side effects. That is, I think the strongest argument for #388 right now is that it makes the smallest possible changes to allow someone to do reasonable batching, and I think that when/how to add concepts of additional goroutines/schedulers can be topics for a future PR as they are needed to further improve performance. Just my 2c-- thanks to everyone who has been contributing to this important thread!

[jjeffery]

Thanks @chris-ramon and @edpark11. Of the alternatives I think that @edpark11 is correct in choosing #388. I wrote the PR (#367) that it is based on, and I still need to spend the time to look through the breadth-first traversal code in detail (for my own education as much as anything else). However I agree with @edpark11 on breadth-first traversal being the way to go.

One point I would like to make is that #388 (and #367 before it) do create a little problem with handling error conditions, and while I do not think that this should get in the way of accepting the PR, I would like to canvas the possibility of another, future, PR that would address this.

In the current implementation, because thunks are called inline and immediately, it is possible to use the stack to keep track of the parent node for each node in the result. If an error occurs, it can be passed up the stack until a parent node that is allowed to be nil receives it, and that parent node returns nil as its value. When thunks are called later in the execute cycle, traversing the stack no longer works. If a thunk returns an error it really needs to be able to traverse up the tree to find a parent that can return nil. The only solution in the current implementation is to return nil for the entire result tree. See my changes to list_test.go in #367 where I have had to modify the test.

As I mentioned, I don't think it's enough of a reason to reject #388, but I would make the comment that I think executor.go needs a reasonably significant update so that we can get the list tests working again. I'd be quite keen to give it a go if I thought that @chris-ramon was prepared to consider a PR of this nature.

[chris-ramon]

closed via: #388