Concurrently resolve fields

[sogko]

This PR addresses #106 and probably #119.
This also depends on PR #123 and has updated with changes from that PR.

Relevant commits specifically for this PR:
(The other commits are not related to this PR)

---

09574d6 Ensure rw-safety of ExecutionContext.errors which will be read/written by multiple routines/threads later

Uses sync.RWMutex to ensure safety of ExecutionContext.errors.

If the executor encounters errors while resolving fields (e.g. from recovering from panic or when ResolveFn returns an error), it will collect it and add to a list of errors, which will then be returned in graphql.Result.

Since this will be done concurrently if we use go-routines to resolve fields, this has to be thread-safe.

---

ab3b083 Experimental implementation of concurrently resolving fields

This change is contained in executeFields(...).

For each field to be resolved, create a go-routine to resolve a field. sync.WaitGroup is used to wait for all fields to complete.
This will happen for each field that has sub-fields as well.

Given the following query:

query Bar {
  article {
    id
    author {
      id
      name
    }
  }
  feed {
    title
    body
    author {
      id
      name
    }
  }
}

  Bar ---> article ---> id
      |           |
      |            --> author ---> id
      |                      |
      |                       ---> name
      ---> feed ---> title
                |
                ---> body
                |
                ---> author ---> id
                            |
                            ---> name

1. article and feed will be resolved concurrently.
2. Within article: id and author will be resolved concurrently.
3. Within article.author: id and name will be resolved concurrently.
4. Within feed: title, body and author will be resolved concurrently.
5. Within feed.author: id and name will be resolved concurrently.

Cheers!

[andreas]

If we can make opt-in concurrency very easy, I think it could be a good solution. Considering the approach of returning a channel from a resolve function, a helper function could make it very easy and get rid of the boilerplate of listening to channels, re-panicking in the executor goroutine, etc.

Resolve: graphql.Parallel(func(params graphql.ResolveParams) (interface{}, error) {
  // ...
})

I think it's less obvious how to get easy/clean opt-in concurrency support for the elements of a list, since the resolve function is tied to the list itself, not the elements. Maybe add a field to List which indicates whether to evaluate the list elements concurrently or not? It doesn't feel as clean though.

It would be interesting to look into the overhead of the current approach, e.g. via the new benchmarks (#22). A pool of goroutines could potentially be used to decrease the overhead and limit the number of goroutines.

[bigdrum]

Even if the go routine overhead isn't a concern, I see another potential problem with this PR. The best feature I like for graphql is the possibility and simplicity of batching/deduping requests to the backend service (e.g. plugging it with facebook/dataloader), which brings substantial performance benefit. Doing parallel in this way makes it very hard (afaict) to implement such features.

If we allow the resolve function to return a channel as @Matthias247 suggested, or a func() (interface{}, error) that I personally prefer, the batching mechanism would be possible to implement (which allow some hook to collect all such promise-like results and dispatch batched requests).

The implementation of supporting such promise-like feature might be more complicated though, and I'm still trying to figure it out a way to do it.

[rantav]

My vote goes to opt-in concurrency.
Would love to see this feature a reality.
My use case: Resolving fields from many database queries. Obviously this is something that can enjoy parallelism.

[andreas]

Following this discussion, I've made a PR for opt-in parallel resolving of fields and lists: sogko#23

[alonrolnik]

I really would like to see this going forward, my use case is the same as @rantav, resolving fields from multiple DB queries which can be executed in parallel.
Any time estimation for it? is there any temporary solution that I can use in the meantime?
Thanks!

[coveralls]

Coverage increased (+0.1%) to 83.604% when pulling 0ba9e8f on sogko:sogko/experiment-parallel-resolve into 22050ee on graphql-go:master.

[Kliton]

Any update?

[sandeepone]

Any Update?

[eduardogalbiati]

Any update?

[Kliton]

Up

[jekiapp]

I've been experimenting to give opt-in concurency for resolving fields myself. Since I really need it. Is there any concern about why we discontinue this feature like as @andreas did in sogko#23?

[dvic]

I resolved the conflicts for this branch and created PR sogko#25, but there are still many data races due to the fact that Schema is not safe for concurrent use.

[divoxx]

Is there a specific reason why this hasn't been finished/merged? What is pending? This makes it nearly impossible to be used in a large application with complex graphql structure and even moderate traffic.

If there is a clear definition of the rationale/direction and what's currently pending, someone else can pick it up. I'm willing to take a stab at it.

[appleboy]

@chris-ramon Could you have time to take look at this?

[chris-ramon]

@chris-ramon Could you have time to take look at this?

• Actually this PR adds experimental support for concurrency and mentioned in the top description.

ab3b083 Experimental implementation of concurrently resolving fields

• Also another experimental support PR: Experimental/Need feedback: Implement pluggable batching/dedup/concurrent fetch like facebook/dataloader #154

• There is a couple of up-to-date pull-requests with different approaches:

1. Returning an async function: Resolve fields asyncronously #213

Resolve: func(p graphql.ResolveParams) (interface{}, error) {
  return func() (interface{}, error) {
    // runs within a goroutine
  }, nil
},

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

ch := make(graphql.ResolvePromise, 1)
// ...
Resolve: func(p graphql.ResolveParams) (interface{}, error) {
  return graphql.ResolvePromise(ch), nil
}

An another approach PR against a fork:

1. Opt-in concurrency: Opt-in parallel resolving of fields and lists sogko/graphql#23

Parallel: true,
Resolve: func(p graphql.ResolveParams) (interface{}, error) {
  // runs within a goroutine
},

Which one would match your use cases @appleboy

[chris-ramon]

Thanks a lot guys! 👍 — Closing this one in favor of #388, you might want to take a look to a real-use case working example that shows the support for concurrent resolvers.