Support multiple gateways in the same process #261
Conversation
| #[wasm_bindgen_test] | ||
| fn multiple_query_planners() { | ||
| let schema_multiple_keys = include_str!( | ||
| "../../stargate/crates/query-planner/tests/features/multiple-keys/csdl.graphql" |
There was a problem hiding this comment.
is there a reason we're stringly typing paths
There was a problem hiding this comment.
(this seems to pass on windows but i am surprised)
| let planner_basic = get_query_planner(JsString::from(schema_basic)); | ||
| let query_basic = "query { me { name } }"; | ||
|
|
||
| let options = QueryPlanningOptionsBuilder::default().build().unwrap(); |
There was a problem hiding this comment.
we should at least make these unwraps expects.. if these were to fail it would be very unpleasant to debug
|
@jaredly You should be able to test this using the npm tarball artifacts that were generated in the CI build process. Specifically, you probably need to
|
|
@abernix thank you for the clear steps on how to test this out locally! FYI for anyone else testing, I also found I needed to add the following to my package.json to have the correct library picked up by the |
There was a problem hiding this comment.
I don't know much about best practices for interactions between JavaScript and WebAssembly, but I'm wondering if there's a way to avoid manually managing pointers on the Rust side.
Looking at the was-bindgen documentation, it seems annotating functions should create JavaScript bridge classes for structs. That class also has a free() method that can be used to deallocate memory on the Rust side.
Relying on the wasm-bindgen generated code may also make it easier to switch to weak references or the new externref type if/when that makes sense.
The difficulty here is that query-planner-wasm has the wasm-bindgen dependency, not the query-planner crate it depends on. And we may not want to have query-planner depend on wasm-bindgen. Not sure if there's a way to still have wasm-bindgen generate code for a function defined in another crate.
|
This should no longer be necessary with the landing of #622 (specifically, the changing of the query planner back to TypeScript). Put another way, the code this changes is no longer present and the query planners are now scoped within the Gateways that generate and execute them. |
This PR makes it so that getting a query planner creates a new planner every time and never overrides the global instance.
However, because ApolloGateway can roll over its schema on the fly, we've added a new function named
updatePlannerSchemathat overrides a specific planner.I opted for the simplest solution that solves the problem described in #210 , without having to manage memory from JS and implementing complex freeing/reference counting.
This hopefully fixes #210
This implementation is suggested instead of #226 to keep things simple.
This PR also moves away from using raw pointers, to using an index to an array. That was a good suggestion in #226 as well.
@jaredly Can you verify this works for you? As far as I could tell, this solves the problem, and I've used the test you wrote in #226 (thanks for that!). I'd like to get confirmation from you before I merge. Thanks!