dlukez / graphql-dotnet-dataloader

Batch data access using DataLoader + GraphQL in .NET
MIT License
51 stars 6 forks source link

Nested DataLoader Calls Never Return #4

Open asherjvitek opened 6 years ago

asherjvitek commented 6 years ago

We have been working with GraphQL DotNet and we started working with this DataLoader. Seemed like a pretty good tool but we have run into an issue that I believe I can recreate in the sample project.

When you have nested calls that invoke the IDataLoader LoadAsync methods it looks like those calls never return. You can run the below query to see the issue first hand. This will eventually timeout. {humans{name, appearsIn{id, characters{name}}}}

After much digging it appears to happen after going more than 1 level deep. These queries do work: {humans{name, appearsIn{id}}} {episodes{characters{name}}}

At first I was sure that I was doing something wrong but upon being able to recreate using the code here I think there may be a problem with the code.

I did some debugging and it looks like the Fetch Method is never being called for the nested characters query.

Any help would be greatly appreciated.

speeddm commented 6 years ago

Also experiencing the same problem, @asherjvitek did you arrive at any fix for this issue yourself?

asherjvitek commented 6 years ago

I did quite a lot of testing and debugging and was able to determine that I believe the issue is actually in the DataLoader-DotNet solution. Additionally I think this may be the same issue as referenced here Async threading issues which was created by the author over a year ago.

I tried making some changes to that code base to see if I could resolve the issue but I was only able to sorta fix it.

What I found: (Note all code references are to the DataLoader-DotNet) I believe the issue is an order of operation issue with the \src\dataloader\dataloaderbase.cs Trigger(), FlushLoadersOnThread() and QueueLoader(IDataLoader loader) methods.

Using the example query {humans{name, appearsIn{id, characters{name}}}} the field humans is level 0, appearsIn is level 1 and characters is level 2.

When humans is resolved \src\dataloader\dataloadercontext.cs internal void QueueLoader(IDataLoader loader) is called for the appearsIn Field before the call Trigger() method calls Context.FlushLoadersOnThread();. This results in the Trigger() method being called for appearsIn once all the humans have been completely resolved.

For all Complex Fields below level 1 like characters this happens out of order and QueueLoader is called after the call to Context.FlushLoadersOnThread(); in Trigger() and this results in an IDataLoader in the _localQueue but that IDataLoader Task is never Triggered and thus the DataLoader Context thread never ends as there is still 1 task in the Queue.

I attempted fixed this by placing a call in to FlushLoadersOnThread() in the internal void QueueLoader(IDataLoader loader) call. This ensured that anything queued was going to be triggered. This did indeed seem to fixed the issue but upon running a SQL trace to make sure the DataLoader was doing what I expected, I found it was not. Instead of running the characters query in 1 batch it was running them in mini batches.

At this point I gave up as we are on a deadline with the project we are working on and we made the choice to remove the use of the DataLoader and for now live with the N+1 issues.

I really hope that this is helpful info and someone smarter than me can really nail down the issue and fix it.