Juniper: Interop with async? (Tokio/Futures)

[jsgf]

Hi -

Have you put any thought into how this could be used with an async IO model, such as with Tokio? Right now it seems like it's all implicitly synchronous, but perhaps it's just a matter of making the various functions take/return Futures?

Thanks

[theduke]

The big thing here is that this would also enable an implementation of something like dataloader, which Facebook has deemed the best practice for handling remote data fetching for a GraphQL query.

Based on my (limited) experience with GraphQL it's the only sane way to support large/complex backends and multiple queries per request in a sane way.

[mhallin]

I've made a few attempts locally at integrating futures-rs into Juniper, but there are a lot of stumbling blocks that need to be overcome. There are also some nice properties the current synchronous implementation has that we'll probably lose, particularly around memory usage and heap allocations. This might not be a big issue, but I like the "you don't pay for what you don't use" mentality of Rust and C++ in general.

So, these are some issues that have prevented me from building this. There are probably more :)

• Many AST nodes, particularly identifiers, use references to the original query string to avoid copying. Rust statically guarantees that no AST nodes will outlive the query string, and it's easy to reason about when execute just parses and executes the query synchronously.
• GraphQLType::resolve* would need to return a Box<Future>, which means that _all_ fields will cause extra heap allocations, even if it's just field like_count() -> i32 { self.like_count }. Now that I think about it, maybe you could change FieldResult to something like enum { Immediate(Value), Err(String), Deferred(Box<Future<Item=Value, Error=String>>) }.
• The core execution logic would need to be transformed into something that joins multiple futures together and emits a result. No fundamental problem here, it was just a very difficult programming task :)
• The futures-rs crate changing around a bit while working on this. This shouldn't be the case anymore, but I would be weary releasing Juniper 1.0 while depending on a pre-1.0 release of futures-rs.

I've been dabbling with futures-rs and Tokio in another project that I'm working on, and the general feeling I get is that's it's pretty unergonomic to use. I've stumbled upon cases where I couldn't break out a big and_then callback to a separate function because of various reasons: either the type was "untypeable" - i.e. containing closures, BoxFuture requires Send despite the name, Box<Future> did not work because of lifetime issues, and even when switching to nightly and returning impl Future there have been cases where returning lifetimes have been an issue.

Despite all of this, I still think this a feature we want to have! :) However, there are some constraints on the implementation:

• Minimal impact on the non-async path. _Ideally_, Juniper should only create a new heap-allocated future for each object to resolve, _not_ each field. Scalars should not cause any overhead at all.
• No reliance on unstable compiler features. Juniper should not require a nightly compiler.

This became a long response with little amount of actual content :) I might open up a branch to do some work on this, but it's been hard trying work in incremental pieces since it's such a cross-cutting change.

[Mange]

Juniper should only create a new heap-allocated future for each object to
resolve, not each field.

What about fields that require an expensive calculation that you'd rather
define with an async method? After all, fields are how you get objects in
the first place. :-)

Den ons 12 juli 2017 11:37Magnus Hallin [e-mail] skrev:

I've made a few attempts locally at integrating futures-rs into Juniper,
but there are a lot of stumbling blocks that need to be overcome. There are
also some nice properties the current synchronous implementation has that
we'll probably lose, particularly around memory usage and heap allocations.
This might not be a big issue, but I like the "you don't pay for what you
don't use" mentality of Rust and C++ in general.

So, these are some issues that have prevented me from building this. There
are probably more :)

• Many AST nodes, particularly identifiers, use references to the
  original query string to avoid copying. Rust statically guarantees that no
  AST nodes will outlive the query string, and it's easy to reason about when
  execute just parses and executes the query synchronously.
• GraphQLType::resolve* would need to return a Box, which
  means that all fields will cause extra heap allocations, even if
  it's just field like_count() -> i32 { self.like_count }. Now that I
  think about it, maybe you could change FieldResult to something like enum
  { Immediate(Value), Err(String), Deferred(Box Error=String>>) }.
• The core execution logic
  https://github.com/mhallin/juniper/blob/master/src/types/base.rs#L291
  would need to be transformed into something that joins multiple futures
  together and emits a result. No fundamental problem here, it was just a
  very difficult programming task :)
• The futures-rs crate changing around a bit while working on this.
  This shouldn't be the case anymore, but I would be weary releasing Juniper
  1.0 while depending on a pre-1.0 release of futures-rs.

I've been dabbling with futures-rs and Tokio in another project that I'm
working on, and the general feeling I get is that's it's pretty unergonomic
to use. I've stumbled upon cases where I couldn't break out a big and_then
callback to a separate function because of various reasons: either the type
was "untypeable" - i.e. containing closures, BoxFuture requires Send
despite the name, Box did not work because of lifetime issues,
and even when switching to nightly and returning impl Future there have
been cases where returning lifetimes have been an issue.

Despite all of this, I still think this a feature we want to have! :)
However, there are some constraints on the implementation:

• Minimal impact on the non-async path. Ideally, Juniper should only
  create a new heap-allocated future for each object to resolve, not
  each field. Scalars should not cause any overhead at all.
• No reliance on unstable compiler features. Juniper should not
  require a nightly compiler.

This became a long response with little amount of actual content :) I
might open up a branch to do some work on this, but it's been hard trying
work in incremental pieces since it's such a cross-cutting change.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/mhallin/juniper/issues/2#issuecomment-314710027, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAGP7F4ZyFLEmO6FWhUxK2m_aCAzKzLks5sNJPqgaJpZM4K0V7G
.

[mhallin]

I maybe expressed myself a bit ambiguous there - if a user defines an async field, than it should obviously allocate a future for that field. I meant that no futures should be allocated for synchronous fields.

[srijs]

@mhallin @theduke In your opinion, how feasible is it to ship this?

Lack of async support is currently _the_ thing that prevents me from using Juniper for more projects. I appreciate that all in all it would be a huge refactor, but maybe this is something that could be shipped in increments?

I've been gathering a lot of futures/tokio experience lately; would you be motivated to help me get the PRs reviewed and merged if I got started on this? Or do you feel like it's not the right time for this feature?

[mhallin]

@srijs Just out of curiosity, do you already have a futures-based Rust codebase that you want to expose with Juniper, or are you looking at GraphQL servers with async support in general?

To be honest, the more i work with Promises/futures-rs _and_ other concurrency systems such as Erlang/Go, the less interested I am working on this. Just compare the work with integrating futures-rs with say Rayon: it would be trivial making the execution logic parallel without running into any of the problems I listed in my first reply here. If the Rust community's async efforts were directed to something with that kind of API, I'd be more interested.

That said, I will of course help you out if you decide to tackle this! I'm not sure even where to begin, but keeping AST nodes alive for the duration of the entire execution without using references with lifetimes is something that needs to be solved first. That might require putting all nodes under Arc, which is kind of unfortunate...

[srijs]

@mhallin I do have existing Rust codebases (that use tokio extensively) where I would love to be able to use Juniper.

It seems with generator functions/coroutines we're moving into the direction you're describing, but of course that will not be usable in stable Rust for quite a while. So while I agree that it would be simpler by leveraging coroutines, I don't think it's practical to wait. At least personally I'd like to see Juniper support async before coroutines land in Rust stable.

I have started to play around with the AST/parser parts, to use reference-counted string objects as you suggested (although I'm not 100% they would need to be Arc; Rc might be sufficient). Servo is using tendril for this purpose, but it seems rather unstable at the moment. I wonder if there is a crate like bytes just for text, or whether we could actually use bytes for this purpose...

[dcabrejas]

Hi, can someone give me an update on this, I recently started a project using Juniper but the data to fulfil the GraphQL request comes for an external web api so I need to take advantage of futures for performance. Is futures support being worked on, if so by when will it be ready? if not, how do people go about doing what I am trying to do without using async I/O ?

Thanks

[thedodd]

Mostly in response to, or in addition to, @mhallin's comment above, here are some thoughts based on the recent advances on the rust futures & async front, especially drawing off of these sources:

• http://aturon.github.io/2018/04/06/futures2/
• https://boats.gitlab.io/blog/post/2018-04-06-async-await-final/

heap allocations

With conservative impl trait and, less importantly, universal impl trait slated for rust 1.26, we could experiment with field signatures having a return type of impl Future<...> as opposed to Box<Future<...>>, to save on heap allocations (we'll see when it actually lands on stable).

Per some comments above about reference issues even inside of a return impl Future<...>, perhaps the upcoming Pin API will be useful.

AST sharing

With futures-rs 0.3, the Pin API will be leveraged for futures, which will allow reference sharing between futures &c. I haven't looked at the code in Juniper where the AST sharing is taking place, so the Pin API may or may not make any difference. Need to investigate a bit more.

futures stability

futures-rs has just recently gone through a pretty massive revamp with the 0.2 release. Apparently the design is more long term, with a 0.3 coming soon:

we anticipate a 0.3 release relatively soon. That release will set a stable foundation for futures-core, after which we can focus on iterating the rest of the stack to take full advantage of async/await!

So, once 0.3 lands, that may be a good time to start experimenting more aggressively.

Thoughts?

[dvic]

FYI, futures 0.3.0-alpha.1 has been released.

[thedodd]

Yes! It would be so awesome to have async GraphQL in Rust! I’ve built GraphQL servers in Node, Python, Go & Rust; and Rust’s Juniper is by far the most powerful and ergonomic to work with. Adding async support will be the crown jewel, IMHO 🙌

I think now would be a great time for some futures integration, for sure.

[divoxx]

By the way, I'm not sure if this has been proposed/discussed, but other libraries implemented support for dataloader by using thunks and a breadth-first resolution, without the need to introduce async code into the graphql executor.

The golang graphql library PR explains the approach and contains other references as well: https://github.com/graphql-go/graphql/pull/388

Would this be worth pursuing? Should this be added as a separate issue?

[liuchong]

Will this async juniper comming soon? :joy_cat:

[tomhoule]

I looked into how this could be implemented a bit. My understanding is that we would need to return futures instead of results in a few places if we want to keep the changes minimal, for example resolve_field. This is a good example because it is a trait method, so we currently would have to box the returned future, which means one allocation per field. As far as I understand, returning unboxed futures from trait methods is blocked on GAT and existential types/async trait methods.

With async/await _and async methods in traits_, it looks like it would be possible to generate code for async resolution without boxing a future per field, but it blocks this feature until at least a few months after async_await lands (existential types are not working at the moment, even on nightly, as far as I know).

[theduke]

True, we will also need to figure out how to deal with the query AST. We will need to either put it behind an Arc or clone the sub-AST when neccessary (or do a hybrid approach for fan-outs).

The boxing issue may not be that bad if we can come up with an API that's transparently upgradable to a non-boxing solution, which should be possible.

Also, considering that async resolvers will only be necessary for fields that do DB or api requests the boxing should not be significant since the small allocation will be dwarfed by the actual workload.

The bigger challenge may be

• possibly restricting the concurrency for certain fields
• easily support batching with something similar to https://github.com/facebook/dataloader
• tuning the tokio runtime to handle the workload correctly. (EG: always use blocking for non-async resolvers, or having a dedicated thread pool for the sync resolvers, etc.
• prevent users from blocking the runtime by making the docs very clear on when to use async and when not to (and when users might manually use the blocking primitives)

We should also be aiming at basing this on futures 0.3 instead of 0.1.

[thedodd]

@theduke yea, I would imagine that cloning an Arc would be more performant than cloning the query AST or subsections thereof (could be wrong on that though).

Agreed on the boxing issue.

Do you mind expounding a bit on "possibly restricting the concurrency for certain fields"?

As far as tokio runtime tuning, using blocking and such, I do have a few thoughts for discussion:

• I don't know if we should do anything other than strongly encourage folks to understand that if they are not using an async return value (a future) then they must be very careful not to block the runtime.
• For folks that understand this, then the overhead of delegating to a background thread via the blocking API may be overkill and will add some performance hits (as minimal as they may be).
• For folks that do not understand this, they can still easily block the runtime even in async handlers, by using thread blocking locks or making synchronous network calls. So arguably the only real solution is to teach and help folks understand.

So, perhaps a nice balance may be to teach non-blocking in the docs fairly well, include examples of how to use the blocking API for situations where such a thing is needed (eg, dealing with legacy crates), and then just encourage folks not to do any blocking operations when resolving scalar values &c.

Also, +1 on futures 0.3. The compatibility layer is pretty solid as well, so that shouldn't cause any issues.

[theduke]

I was hoping to wait with all this until the whole async ecosystem stabilizes and we see a new tokio release with std Future and async/await support but things are taking a long time sadly.

[jkoudys]

Futures just stabilized in 1.36 - https://github.com/rust-lang/rust/pull/59739 (as 0.3)

Are we confident we now have an API to start building out juniper resolvers as Futures?

[thedodd]

@jkoudys & @theduke looks like with the 1.36 stabilization & the fact that tokio has interop and a strong compatibility layer between futures 01 and std futures, now might be a great time.

For async/await, we will have to be on nightly ... but we could always ship a preview branch which requires nightly, and just not merge it until everything is on stable.

Something of note on async/await, it seems that the async patterns are pretty well settled down, but the await syntax is still in the air. We could just use the nightly await! macro for now and then once the final await syntax lands, we can just switch over.

[theduke]

I don't think await will be all that useful for the code in Juniper itself, since we will mostly be dealing with joins and a few manually written futures, so I think it's fine to go ahead now.

In addition to what I posted above, the big questions we need to answer are:

the FieldResult type

We certainly want to allow a mix of async and non async resolvers. EG it makes no sense to resolve fields that are just a struct field access with a future. In that light, the FieldResult type will have to become an enum that can contain both a sync response and a async one. And both the macro auto-generated and other plumbing will have to deal with resolving sync resolvers directly and async ones via a future, and then wrap everything in a future when appropriate.

Runtime

There is also the question of how and when a threadpool will need to come into play. Preferably we would remain runtime agnostic, and not rely on anything provided by tokio et al. I'm not sure that's possible but it probably should be. But that would also mean no threadpool and letting the user decide what things are expensive and need to be delegated to a threadpool.

With futures we can now add timeouts to resolvers in a sensible way, but that might require a specific runtime.
If we need a runtime, we should wait until tokio is refactored to `0.3.

Concurrency limits

Currently everything happens one after the other, synchronously.
With futures we could have a lot of work items fired off simultaneously.

Just imagine a API request that fires of 10 database requests and 5 network fetches.
Currently, they happen one after the other in a thread.

With futures we might have 30 requests arriving at once, all started at the same time, leading to 300 db and 150 network requests. And new requests arriving all the time. I guess this could overwhelm the runtime pretty quickly and cause a ballooning of workload because every single request takes longer.

So I think some limiting system will be essential. Eg: no more than X actively processed requests at a time. Additional ones get put in a queue and are only resumed once another request finishes.

Also timeouts come into play here.

I think this will be trial and error. Let's whip something up and see how it works out with benchmarking. But it is something to consider.

I'm pretty certain the limit/queuing will need to happen.

Introspection / tracing

I don't know much about the tracing functionality of tokio (https://github.com/tokio-rs/tokio/tree/master/tokio-trace), but this might be a very nice thing to have to provide insight into your workload and point out slow / problematic resolvers. Investigating this would be nice.

Let's take it step by step.

Is anyone interested in working on this?

[tomhoule]

The threadpool part could be hard to abstract, so it would make sense to leave setting up the runtime and handling blocking tasks to users of the library in my opinion. For example the tokio threadpool handles blocking tasks differently from most others (the blocking function).

[thedodd]

But that would also mean no threadpool and letting the user decide what things are expensive and need to be delegated to a threadpool.

@theduke & @tomhoule yea, I definitely agree. As long as their blocking/background threadpool operation returns a future, then we should be good. We shouldn't have to do anything else to handle it.

As far as Concurrency Limits:

• on one hand I'm tempted to say that we should allow users to horizontally scale out their instances as they need to on their own. Allow them to load balance traffic across their instances, and then we just operate on a best effort basis. If we impose limitations on our side, then we might actually inhibit scalability for folks that have other ways of managing this sort of stuff (like using kubernetes & HPA).
• on the other hand, perhaps we could introduce some sort of complexity limitations. Something that is opt-in. A few other GraphQL frameworks in other languages have something like this. We could offer depth complexity limits and maybe offer parallel resolution limits. However, once again, the tokio runtime can handle a pretty massive number of concurrent futures/tasks. Perhaps introducing a limitation is an over-optimization here which might actually dwarf the capabilities of the underlying system. Maybe something optional would be best.

Thoughts?

[dsilvasc]

For concurrency limits, does juniper already batch requests within a query? If not, it might be useful to see how the Sangria graphql server does it with what they call deferred values, deferred resolvers, and fetchers:

https://sangria-graphql.org/learn/#deferred-value-resolution
https://sangria-graphql.org/learn/#high-level-fetch-api

[jkoudys]

I'm all for limiting the number of DB + network queries it could fire off, but is it really juniper's job to worry about this? If I'm making DB queries, then I have a DB connection pool that my resolver functions will query from. I could limit that size, which means that those 300 DB requests will wait on the pool, so while I have 300 requests I could still only have 10 or so active DB connections. This is exactly what I want, since it maximizes how much can run in parallel and could scale up really easily.

And do we really need an enum of FieldResult? We already say impl IntoFuture for Result, because anything sync can be treated as async, and then we can easily pass results back in closures in a chain of and_thens, e.g. .and_then(|i| Ok(i + 5)). If there's aFieldResult, why not a FieldFuture, that impl Future, so we could impl IntoFuture for FieldResult? You could still use any non-blocking FieldResult-returning functions the same as before.

[LegNeato]

Relevant: https://www.reddit.com/r/rust/comments/bi2puk/help_rate_limiting_in_async_rust_and_tokio/

[theduke]

@jkoudys

And do we really need an enum of FieldResult?

We definitely need a efficient way of dealing with sync results like a simple field access.

Most objects will have mixed sync and aynsc resolvers, eg a BlogPost with a bunch of sync fields and a author async field. Now let's say a query gets 10 fields of a post.

A naive approach would mean that for each post, we need to join over the 10 fields, which means allocating a Vec and boxing each of the field result futures. That's a lot of redundant allocations and also a dynamic dispatch for each field result.

Sync values would be represented by an immediately ready future (https://docs.rs/futures/0.1.26/src/futures/future/result.rs.html#13-15) which has little overhead, just an extra Option.

We can check if the future is ready by calling poll right in the resolver and prevent the boxing and joining in the generated code. This would mean relatively little overhead, if the poll call is inlined. It still leaves us with a redundant self.inner.take().expect("cannot poll Result twice").map(Async::Ready), which would hopefully always be inlined, but that is not guaranteed since the expect produces quite a few instructions.

I think a enum might be the better solution.

but is it really juniper's job to worry about this?

Probably not, and hopefully not.
But it's the classic problem in network services: let's say your server can maintain equilibrium with 10 db pool connections and serving 20 queries per second. Now the query load doubles, and you have a lot more code fighting over those DB connections, driving up latency for each query. Which leads to even more active queries as more and more come in, resulting eventually making the server totally unresponsive.
At some point, you really want to reject additional requests instead of leaving them hanging for a minute.

Of course there are plenty of ways to deal with it, load balancers, reserving a single db connection for each query, etc. It's just something we should keep in mind and benchmark how tokio will deal with it under high load.

Wrapping the execution into some kind of rate limiter should be easy, if necessary.

[neoeinstein]

One thing to consider is the path of another project: actix-web. They are preparing for their 1.0 release, and have actually removed their internal Async enum, going with a futures-first interface and using generics to differentiate between the sync and async cases. Going with an enumeration means that _every_ field access incurs the cost of branch during runtime for _every_ request, regardless of whether they use both. I think that this can be better handled with a trait that has different impls for the sync and async cases (and you can probably have things returning impl Future, so no boxing necessary, again, look to actix-web for inspiration). This way you get the zero-cost and only-pay-for-what-you-use benefits.

Worrying about the cost of allocating a Vec in this case seems to me like a premature optimization. If allocating a Vec means that I can handle double the requests per second per instance, that might be a tradeoff I'm happy to make. This cost seems outweighed by other bigger costs, e.g. no way to cache parsed and validated queries means that every request must be re-parsed and re-validated, even for repeated requests with the same query text (albeit with different variables).

On the rate-limiting/balancing, that seems outside of the scope of juniper. There are other tools that do it very well, and tower-qos is a great example, using tokio-sync under the covers. If someone wants to limit the number of concurrent query executions, they can do that at the layer above juniper. If they want to limit the number of concurrent operations on an underlying service, they can do that at the layer below. Timeouts or other policies to apply backpressure feel like cross-cutting concerns, so I see these as possible extensions, but not part of the core concern for this library. Everyone is going to want to configure it their own way, and my belief is that adding an core internal concurrency limit is more likely to lead to suboptimal configuration by users in the real world than the default of letting tokio handle it.

[jkoudys]

@neoeinstein The actix-web is exactly the approach I was imagining. impl Future has really cleaned out a lot of cases where Boxing was necessary, and is doubly good because of the sync case with Results you describe.

I'll always want rate-limiting, but I have a hard time thinking of a case when I'd want my graphql lib to be the one doing it. Often I'll have resolvers that query psql, redis, and/or a couple 3rd-party RESTful services. Seems mad that I'd leave my redis + psql sitting idle, when there are queries that only need those to resolve, because I have other queries waiting on a RESTful service that's slowed to a crawl. Plenty of places where I could limit that, but no idea why juniper would be involved in the decision. In the case where I'd want to reject the query, I'm happy to have my pool's request for a connection throw back the Err for me.

[thedodd]

So, it sounds like folks are pretty on-board with the impl Future pattern, and given that 03 futures don't require an error value to be present, just an Output which could be whatever, we have a few nice patterns that we could try out. Perhaps something like the following would be nice.

First from a Juniper user's perspective:

• Resolvers may return all of the standard scalar values which are available in Juniper today as well as results and options as normal.
• Resolvers may return Futures of some output type which falls under the category defined above.

From the Juniper framework perspective:

• Standard object derivations behave pretty much the same way. Fields will be resolved as scalar values &c.
• Resolver functions will need a little love. Perhaps, in order for proper Futures03 interop, we will need to define a type, say JuniperFuture, with some From impls.
  
  
  
  • If the resolver returns a scalar value, then the From impl will just create a resolved future of the returned value.
  
  
  • If the resolver returns a future, then the From impl will simply wrap that future for seamless interop.
  
  

This has obvious implications for code generation and the executor. As far as the executor is concerned, perhaps we use something like FuturesUnordered to be able to poll all the futures of a top-level query, and then also be able to push additional futures into the pool for resolution as more fields with resolvers are traversed.

The benefit of taking an approach like this is that the end-user doesn't need to worry about wrapping their resolved values in an enum or anything like that. Things will stay pretty consistent.

Any and all feedback welcome. I am not a maintainer of this crate, so there are probably plenty of oversights here on my part. Thoughts?

[theduke]

Just a little update: making the AST owned/shareable requires a lot of changes. Since it needs work anyway, it's a good opportunity to switch the parser to graphql-parser. To make that happen it needs a little work though.

I'm working on one element ( #19 ), but it also needs quite a few improvements to the spanning infrastructure for error reporting. That's not too challenging but quite a bit of work, if anyone has some spare time..

[LegNeato]

Looks like await is on its way to stabilization:

https://boats.gitlab.io/blog/post/await-decision-ii/

[LegNeato]

I'm still learning futures and async await...if we go impl Future route, that means we can't have a dyn trait object without boxing / using futures::future::BoxFuture.

[theduke]

We will need to use BoxFuture.

Traits can neither have async functions nor functions that return -> impl Trait. Enabling those is blocked on existential types in traits, which doesn't have an RFC yet and is quite a ways off.

Even if it was ready, we most likely wouldn't use it across the whole query: It could create a huge generator that blows the stack or just slows us down because it get's so large.

Existential type aliases could already help us, and do have an RFC, which is currently blocked on a few other things though and also not close to being ready though.

What we could do now is change the way we generate code for objects and use async/await inside GraphQLType::resolve by skipping GraphQLType::resolve_field.

This is somewhat difficult due to the dynamic nature of the resolvers though: we'd have to create no-op futures for all fields that are not included, and then use something like futures_preview::try_join! on all futures and collect the values into an object. This could also become problematic for the stack: what if a object has a lot of fields or ones that result in large generators...

It's an interesting area to explore, but for now I think sticking to boxing and helpers like FuturesUnordered is the reasonable choice.

Since most of this is internal plumbing, we can hopefully play around with improvements in the future.

[theduke]

I just noticed: where we definitely can use await though: at the top level of resolving a query.

This should enable us to use a borrowed AST and Context in all nested resolvers, which is great.

[LegNeato]

FWIW this is what GraphQL.js does:

"The resolve function can return a value, a promise, or an array of promises"

[jkoudys]

Any issues that need to be fixed before this work can go in? Are we waiting on anything in the Futures spec, tokio, etc. or is this ready to be built? Is it going to be BoxFutures? Happy to send in some PRs myself, but I need to know the design direction.

[theduke]

I have a working branch. I'll try to get a PR up in the next 1-2 weeks.

[teh-cmc]

Hey @theduke, I couldn't find anything on your fork: is there any chance we could have early access to that work of yours? It'd be really really helpful even if it's not all polished yet.

[jkoudys]

Seems like @theduke may have abandoned us. We should probably consider this unimplemented again.

[theduke]

Not abandoned, but on an unexpected month-long vacation.
I'm back this weekend and I should have enough time to at least post a WIP PR.

[theduke]

Things are also quite a bit better now since multiple reference support has landed.

[theduke]

Almost there!

Expect a working prototype tomorrow.

[theduke]

Alright, here we go.
I finished a very rough prototype.
It has a bunch of issues which are listed below, so don't use this for anything serious yet.
It is published as the async-await branch.

Caveats

• feature support is unfinished: unions and interfaces don't work yet, and I'm sure a few other impls are missing
• only async fn resolvers are supported right now. this will be extended to resolvers that return -> impl Future soon
• the async_closure feature is required in addition to the async_await feature. This will be fixed soon
• completely unoptimized, there is a lot of redundant boxing right now
  Some of it will be reduced soon, but we really need a few accepted (impl trait type aliases) and up in the air (impl trait in trait methods) language features
• right now there is a public dependency on futures 0.3 (aka futures-preview)
  this will be removed
• for a stable release, the overall trait architecture will need quite a bit of refactoring
  this shouldn't impact the public interface much though, if you are not implementing GraphQLType yourself

Example

I plan to fix up hyper integration with hyper master and tokio 0.2 tomorrow.

So far I only updated the warp integration, based on tokio 0.1, so beware of the compatability and integration issues.

There is a complete example that shows how to use it here.
There is an example resolver that uses reqwest to fetch a remote url in a async function.

How to help

• PRs against the async-await branch are very welcome
• if you find issues that are not yet mentioned, please report them with a reproduction repo

Benchmarks:
Sadly we don't have a benchmark suite yet.
For improving and tuning the performance, I will really need some good, realistic benchmarks to work with.
I will add a benchmark harness to the branch tomorrow, and would then be very happy about contributed benchmark code that is extensive and realistic with a large schema.

The benchmarks should not use real external dependencies like a DB/redis/etc, but should mock the behaviour in a realistic fashion. (eg with randomized delays using tokio-timers).

[LegNeato]

Awesome, hoping to take a look at it later this week!

[obmarg]

I've updated juniper_rocket to support async-await & the async branch of rocket here if anyone's interested.

[theduke]

@obmarg awesome, happy to get a PR against the async-await branch.

[obmarg]

Done: https://github.com/graphql-rust/juniper/pull/417

[LegNeato]

It looks like with the introspection query async might be on par with sync:

[theduke]

Yeah I also have a bunch of benchmarks and the overhead is minmal, even with the current suboptimal implementation.

[theduke]

But pleas enote that introspection query is a bad benchmark for this since the introspection lookup can (and does) skip all the the async logic .

[LegNeato]

Yep, whoops!

[LegNeato]

@theduke I plan to work on this later in the week / this weekend. Can you taskify things that you aren't planning to work on so we don't duplicate work?

[theduke]

@LegNeato I'll only have time to continue next week so the most important tasks right now:

• Implement union and interface support.

This is rather awkward at the moment (which is why I haven't done it yet) because they still use macros instead of a proc macro. The best path would probably be to rewrite them as proc macros, which we wanted to do for a while anyway.

It's mostly just duplicating what the object macro does, plus a type resolver method.

• Write tests

[bsmedberg-xometry]

Is there anything I can do to help with this project? I have a personal and professional interest in moving this forward, and potentially some time to spend on it.

[LegNeato]

Hello @bsmedberg-xometry , long time no see! Would love help as we have been a bit swamped lately. This work is happening on the async-await branch.

The ideal short-term help would be to implement union and interface support there. That entails porting logic over from the macros into proc_macros. It's a bit hairy...not because the work is hard but because macros are annoying to work with.

There is a WIP progress PR for async subscription support but it isn't touching the union and interface stuff so you shouldn't really conflict.

[dfrankland]

Is there a need for more examples? I was able to successfully use the async-await branch with tide: https://github.com/dfrankland/tokei-aas

I'd be happy to add an example using that as the basis.

[theduke]

Small update: I refactored the graphql_union! macro to a juniper::union proc macro.
graphql_interface! will follow soon.

No async support yet, that's the next step, but a fairly small one.

[davidpdrsn]

@theduke Will graphql_union! be deprecated? I'm currently using it in juniper-from-schema.

[theduke]

Yes, I'm replacing all the macros with proc macros.
The proc macros have equivalent functionality, but a slightly different syntax.

[theduke]

Just an additional update: I've been traveling a lot for the past few months, but starting this Saturday, I will have 2 weeks downtime.

I will push hard to have a alpha release with async await by next Friday.

[theduke]

Is there a need for more examples? I was able to successfully use the async-await branch with tide: https://github.com/dfrankland/tokei-aas

I'd be happy to add an example using that as the basis.

@dfrankland what would be extremely helpful is building out a full-featured benchmark with a complex schema in https://github.com/graphql-rust/juniper/tree/async-await/juniper_benchmarks . ( this is currently very rudimentary )

[LegNeato]

@theduke Seeing as we are telling folks to base their PRs on the aync-await branch, let's just get CI passing and merge that into master so we can jam on it. I don't think we'll do another sync-only release and if we need to we can re-branch at the pre-async point.

[theduke]

:+1: , that's what I wanted to suggest.

[LegNeato]

This is landed on master! 🎉

There is still some cleanup work to do (and the book tests don't pass) but all other tests seem to be working with both the async feature and without it.

[rivertam]

Should this issue not be closed then?

[kiljacken]

I believe that it wouldn't make sense until async support is in a release on crates.io

[LegNeato]

Interfaces still don't work and we need to rip out the sync code. Keeping this open until those are done and a release is made.

[repomaa]

Hey! Thanks for the awesome work! I managed to integrate master with dataloader-rs! I noticed that the batch loading isn't working on deeper levels of the tree though. The following example will make it clear:

query {
  recipes {
    name
    ingredients {
      ingredient { name }
      amount
    }
  }
}

This will result in the following db queries (all of which are done by dataloader batch loading functions).

SELECT name FROM recipes
SELECT ingredient_id, amount FROM recipe_ingredients WHERE recipe_id IN ($1, $2, $3)
parameters: $1 = '3', $2 = '1', $3 = '2'
SELECT name FROM ingredients WHERE id IN ($1)
parameters: $1 = '1'
SELECT name FROM ingredients WHERE id IN ($1)
parameters: $1 = '2'
SELECT name FROM ingredients WHERE id IN ($1)
parameters: $1 = '4'

So the the second level (recipe -> recipe_ingredients) of resolvers is batched but the third isn't (recipe_ingredient -> ingredient). This could just as well be a bug in dataloader-rs or even more likely just my incompetence, but i thought I'd post it here if someone has come across this and solved it already.

EDIT:

Ok it seems that if i set data_loader.with_yield_count(very_high_number) it will successfully batch the third level as well. But this results in very long running requests (a second or so).

[LegNeato]

Do you have a link to code?

[repomaa]

@LegNeato sure: https://p.jokke.space/5l2FU/

[tyranron]

With #682 landed, we're fully async compatible now!

[wongjiahau]

@tyranron Are there any examples?

[tyranron]

@wongjiahau check the book on master and examples in the repository.

[wongjiahau]

I found the fix by using juniper = { git = "https://github.com/graphql-rust/juniper", rev = "68210f5"} instead of juniper = 0.14.2.

[LegNeato]

crates.io has been updated with juniper's async support, sorry for the delay. Any future bugs or api changes can get their own issues.

Note that we still support synchronous execution via juniper::execute_sync.

_Thank you to all the contributors who made this possible_, especially @nWacky, @tyranron , @theduke , and @davidpdrsn 🍻 🎉 🥇