graph-gophers / dataloader

Implementation of Facebook's DataLoader in Golang
MIT License
1.2k stars 75 forks source link

Loader not batching properly #102

Closed arthurvaverko closed 1 year ago

arthurvaverko commented 1 year ago

Im using dataloader V7 with generic and i have a simple test func ..

func TestBatchByIdDataloader(t *testing.T) {
    requests := []string{"100", "200", "200", "300", "100", "200", "200", "200", "200", "200", "200"}

    loader := dl2.NewBatchedLoader(func(ctx context.Context, ids []string) []*dl2.Result[string] {
        t.Logf("%+v, %v", ids, len(ids))
        results := make([]*dl2.Result[string], len(ids))
        for i := 0; i < len(ids); i++ {
            results[i] = &dl2.Result[string]{"v", nil}
        }
        return results
    })

    for _, v := range requests {
        thunk := loader.Load(context.TODO(), v)
        _, _ = thunk()
    }

}

this test output is

=== RUN   TestBatchByIdDataloader
    dataloader_test.go:42: [100], 1
    dataloader_test.go:42: [200], 1
    dataloader_test.go:42: [300], 1
--- PASS: TestBatchByIdDataloader (0.05s)

it removes the duplicate requets for id 200 but why i don't get the batched slice including all 3 ids ?

arthurvaverko commented 1 year ago

Nevermind :/ forgot about gorutines :/

machship-mm commented 1 year ago

@arthurvaverko what was your solution for this? I am seeing the same issue.

arthurvaverko commented 1 year ago

You have to call the load function concurrently otherwise it's regular code execution that waits until the batchFunc is resolved and only the makes another load call

machship-mm commented 1 year ago

For anyone wondering how to use this with graphql-go/graphql, you need to return a function in the resolver rather than the result of the thunk function.

i.e. rather than this:

field.Resolve = func(p graphql.ResolveParams) (any, error) {
    // ... do stuff ...
    thunk := loader.Load(p.Context, fmt.Sprint(id))
    return thunk()
}

do this:

field.Resolve = func(p graphql.ResolveParams) (any, error) {
    // ... do stuff ...
    thunk := loader.Load(p.Context, fmt.Sprint(id))
    return func() (any, error) {
        return thunk()
    }, nil
}