search

nautilus / gateway

A federated api gateway for graphql services. https://gateway.nautilus.dev/
MIT License
396 stars 50 forks source link

Fix race condition in executor #142

Closed sGy1980de closed 3 years ago

sGy1980de commented 3 years ago

This PR addresses and closes #141

The change will collect all the dependent steps and not fire them immediately but deferred after the result of the current iteration has been published. This way it is ensured that the insertion point in the final result actually exists.

AlecAivazis commented 3 years ago

Thanks for hunting this down @sGy1980de. Overall the approach makes sense. Do you by chance have an example that reproduces the race condition, even if unreliably? I know that can be kind of tricky but I don't mind running an example over and over to see if i can verify this fixes it

AlecAivazis commented 3 years ago

Going to merge this is as the tests pass. Thanks for the contributing @sGy1980de!

sGy1980de commented 3 years ago

Sorry mate, I was super busy to get things finished before vacation and kind of forgot about that one. Will see if I can extract some form of test for this.

Just that you get an idea how we actually ran into that issue, because the setup might not be the most common one out there. We have a bunch of quite similar but not identical services. For the common parts we created a common schema and some interfaces, which reflect the resolver needed. Then there is a second graphql service for the parts, that do not match for those services. And this one is federated using the this tool with the common service. These two subservices run in two go routines and share the database connection and the caches, using the graph-gophers/dataloader package. So basically when the common part has been resolved and the query plan issues the second one, most of the time no extra database query is issued but only mapping the model to the graphql schema.

This might help to understand why we experienced this issue.