-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: execution batching #80
Comments
Closing the loop on this one – @nmaquet indicated this is a fine roadmap feature, although not a priority of core development. Open for contributions. I may take a pass as a Go learning challenge, though have no commitments on a timeline. In the meantime, we can probably close this one out. |
For anyone who found this issue. 🙂 It seems like after #267 one could just write a plugin with Haven’t tested it tho. |
Yeah, this is working (but there's a problem): package plugins
import (
"github.com/99designs/gqlgen/graphql/handler"
"github.com/movio/bramble"
"github.com/davewhit3/gqlgen-batching"
)
func init() {
bramble.RegisterPlugin(&BatchingPlugin{})
}
type BatchingPlugin struct {
*bramble.BasePlugin
}
func (p *BatchingPlugin) ID() string {
return "batching"
}
func (p *BatchingPlugin) SetupGatewayHandler(handler *handler.Server) {
handler.AddTransport(batching.POST{})
} curl 'http://localhost:8082/query' \
-H 'content-type: application/json' \
-H "X-Batch: true" \
-d '[{"query":"{randomGizmo{id}}"}, {"query":"{randomGizmo{id}}"}]'
[{"data":{"randomGizmo":{"id":"702"}}},{"data":{"randomGizmo":{"id":"806"}}}] However it required deletion of this line, because gqlgen uses the first supported transport. Line 50 in 8a6da66
|
As I mentioned in #80 (comment), one could not override default transports now. With this small change, one could set up own transport, because first transport get's selected: https://github.com/99designs/gqlgen/blob/fb67b709af4f0c5ba6eff8222c728b076a6eb09b/graphql/handler/server.go#L98 It will allow users, for example, to introduce batching with a bramble plugin, as I also mentioned in comment: #80 (comment).
I'm curious where execution batching could fit within Bramble's roadmap, and if the feature as a whole aligns with the green path for the tool. Bear with me please for a slightly contrived example... I've previously framed this scenario in a video if you'd like to skip the written introduction. We have two simple schemas:
Then we have this query:
This contrived example demonstrates an inefficiency: we've initiated two separate requests to the products service during the same resolver generation for the two
products
fields. This is traditionally avoidable using batch execution, which consolidates multiple operations created during the same execution frame into one outbound request, such as:This combined operation issues a single network request; which guarantees the operations arrive at the destination together and get multiplexed; which in turn maximizes the opportunities for database batching. Correctly implemented, this service would only perform one database lookup for both
products
accessors thanks to batching.All that's to say – is there anything prohibitive in the Bramble architecture that should prevent this from working? The pattern simply wraps each Service client in a dataloader that will merge and unpack the individual requests made to it.
I don't mean this as a feature request so much as a discussion point for an optimization roadmap. Is this a feature that would harmonize with Bramble's goals in the long term? We built this into GraphQL Tools, though we stole it from Gatsby, and it works great. Rah, rah, open source!
The text was updated successfully, but these errors were encountered: