graphql-go / graphql

An implementation of GraphQL for Go / Golang
MIT License
9.94k stars 841 forks source link

concurrent-resolvers example is not correct #657

Open aneeskA opened 2 years ago

aneeskA commented 2 years ago

As per https://github.com/graphql-go/graphql/issues/592 if the resolver is made as a thunk, they will be concurrent. To confirm I took the example given at https://github.com/graphql-go/graphql/blob/master/examples/concurrent-resolvers/main.go and changed QueryType to this

// QueryType fields: `concurrentFieldFoo` and `concurrentFieldBar` are resolved
// concurrently because they belong to the same field-level and their `Resolve`
// function returns a function (thunk).
var QueryType = graphql.NewObject(graphql.ObjectConfig{
    Name: "Query",
    Fields: graphql.Fields{
        "concurrentFieldFoo": &graphql.Field{
            Type: FieldFooType,
            Resolve: func(p graphql.ResolveParams) (interface{}, error) {
                var foo = Foo{Name: "Foo's name"}
                fmt.Println("concurrentFieldFoo: sleeping for 5 secs...")
                time.Sleep(5 * time.Second)
                fmt.Println("concurrentFieldFoo: sleeping for 5 secs... done")
                return func() (interface{}, error) {
                    return &foo, nil
                }, nil
            },
        },
        "concurrentFieldBar": &graphql.Field{
            Type: FieldBarType,
            Resolve: func(p graphql.ResolveParams) (interface{}, error) {
                var foo = Bar{Name: "Bar's name"}
                fmt.Println("concurrentFieldBar: sleeping for 5 secs...")
                time.Sleep(5 * time.Second)
                fmt.Println("concurrentFieldFoo: sleeping for 5 secs... done")
                return func() (interface{}, error) {
                    return &foo, nil
                }, nil
            },
        },
    },
})

concurrentFieldFoo and concurrentFieldBar have the same logic - deviating from the sample code where there was a go routine that was created to send the data in later - where both the resolvers sleep for 5 seconds.

In my output, I was expecting to see, the following:

concurrentFieldFoo: sleeping for 5 secs...
concurrentFieldBar: sleeping for 5 secs...
concurrentFieldFoo: sleeping for 5 secs... done
concurrentFieldFoo: sleeping for 5 secs... done

But I am seeing this:

concurrent-resolvers git:(master) ✗ go run main.go
concurrentFieldFoo: sleeping for 5 secs...
concurrentFieldFoo: sleeping for 5 secs... done
concurrentFieldBar: sleeping for 5 secs...
concurrentFieldFoo: sleeping for 5 secs... done
{"data":{"concurrentFieldBar":{"name":"Bar's name"},"concurrentFieldFoo":{"name":"Foo's name"}}}%      

This is not what I understood from https://github.com/graphql-go/graphql/pull/388 .

What am I missing here?

Full source attached here

package main

import (
    "encoding/json"
    "fmt"
    "log"
    "time"

    "github.com/graphql-go/graphql"
)

type Foo struct {
    Name string
}

var FieldFooType = graphql.NewObject(graphql.ObjectConfig{
    Name: "Foo",
    Fields: graphql.Fields{
        "name": &graphql.Field{Type: graphql.String},
    },
})

type Bar struct {
    Name string
}

var FieldBarType = graphql.NewObject(graphql.ObjectConfig{
    Name: "Bar",
    Fields: graphql.Fields{
        "name": &graphql.Field{Type: graphql.String},
    },
})

// QueryType fields: `concurrentFieldFoo` and `concurrentFieldBar` are resolved
// concurrently because they belong to the same field-level and their `Resolve`
// function returns a function (thunk).
var QueryType = graphql.NewObject(graphql.ObjectConfig{
    Name: "Query",
    Fields: graphql.Fields{
        "concurrentFieldFoo": &graphql.Field{
            Type: FieldFooType,
            Resolve: func(p graphql.ResolveParams) (interface{}, error) {
                var foo = Foo{Name: "Foo's name"}
                fmt.Println("concurrentFieldFoo: sleeping for 5 secs...")
                time.Sleep(5 * time.Second)
                fmt.Println("concurrentFieldFoo: sleeping for 5 secs... done")
                return func() (interface{}, error) {
                    return &foo, nil
                }, nil
            },
        },
        "concurrentFieldBar": &graphql.Field{
            Type: FieldBarType,
            Resolve: func(p graphql.ResolveParams) (interface{}, error) {
                var foo = Bar{Name: "Bar's name"}
                fmt.Println("concurrentFieldBar: sleeping for 5 secs...")
                time.Sleep(5 * time.Second)
                fmt.Println("concurrentFieldFoo: sleeping for 5 secs... done")
                return func() (interface{}, error) {
                    return &foo, nil
                }, nil
            },
        },
    },
})

func main() {
    schema, err := graphql.NewSchema(graphql.SchemaConfig{
        Query: QueryType,
    })
    if err != nil {
        log.Fatal(err)
    }
    query := `
        query {
            concurrentFieldFoo {
                name
            }
            concurrentFieldBar {
                name
            }
        }
    `
    result := graphql.Do(graphql.Params{
        RequestString: query,
        Schema:        schema,
    })
    b, err := json.Marshal(result)
    if err != nil {
        log.Fatal(err)
    }
    fmt.Printf("%s", b)
    /*
        {
          "data": {
            "concurrentFieldBar": {
              "name": "Bar's name"
            },
            "concurrentFieldFoo": {
              "name": "Foo's name"
            }
          }
        }
    */
}
aneeskA commented 2 years ago

@chris-ramon Can you please take a look at this?

LLLLancelot commented 2 years ago

i guess 'concurrent-resolvers' means resolve logic runs concurrently with the graphql schedule, and the schedule will wait and handle all promising resolve functions. Which Not means resolve func is run concurrently by the schedule, you should setup concurrently tasks by yourself

LLLLancelot commented 2 years ago

here is my example:

"concurrentFieldFoo": &graphql.Field{
    Type: FieldFooType,
    Resolve: func(p graphql.ResolveParams) (interface{}, error) {
        var foo = Foo{Name: "Foo's name"}
        fmt.Println("concurrentFieldFoo: sleeping for 5 secs...")
        tick := time.NewTimer(time.Second * 5)
        return func() (interface{}, error) {
            <-tick.C
            fmt.Println("concurrentFieldFoo: sleeping for 5 secs... done")
            return &foo, nil
        }, nil
    },
},
"concurrentFieldBar": &graphql.Field{
    Type: FieldBarType,
    Resolve: func(p graphql.ResolveParams) (interface{}, error) {
        var bar = Bar{Name: "Bar's name"}
        fmt.Println("concurrentFieldBar: sleeping for 5 secs...")
        tick := time.NewTimer(time.Second * 5)
        return func() (interface{}, error) {
            <-tick.C
            fmt.Println("concurrentFieldBar: sleeping for 5 secs... done")
            return &bar, nil
        }, nil
    },
},
aneeskA commented 1 year ago

This is the conclusion I came to based on my experiments. :)

i guess 'concurrent-resolvers' means resolve logic runs concurrently with the graphql schedule, and the schedule will wait and handle all promising resolve functions. Which Not means resolve func is run concurrently by the schedule, you should setup concurrently tasks by yourself