Skip to content
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] GraphQL Subscriptions #267

Merged
merged 7 commits into from Mar 7, 2017
Merged

[RFC] GraphQL Subscriptions #267

merged 7 commits into from Mar 7, 2017

Conversation

robzhu
Copy link
Contributor

@robzhu robzhu commented Feb 16, 2017

Let's add GraphQL Subscriptions to the Spec!


**Possible Solutions**

We broadly categorize realtime API solutions into three types:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if is a typo, but there are six types defined below (instead of the three mentioned here).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(If you look at the rendered view, it's a nested list)

@stubailo
Copy link
Contributor

stubailo commented Feb 16, 2017

This is really great! It was awesome that this took into account feedback from a variety of sources.

I think this sketch of the subscriptions lifecycle is compatible with the one currently implemented in our graphql-subscriptions package here, which can be a nice way to test-drive some of the concepts: https://github.com/apollographql/graphql-subscriptions/

We've designed it in such a way that anyone currently using GraphQL.js can just drop it in and start using subscriptions right away, and we have some adapters for Redis, MQTT, etc.

We'll make sure that implementation is always up to date as new conclusions are reached in the discussion. After all, this RFC is just the beginning, not something set in stone.

There's also a transport protocol over websockets (something that shouldn't be covered in a spec, but you need a transport to use it) which we are currently using in production. It isn't coupled to any particular client or web server package, and implements all of the important lifecycle events necessary: https://github.com/apollographql/subscriptions-transport-ws

I encourage people interested in this proposal to try out those packages with their existing server and experiment with GraphQL subscriptions today! I think that's going to be a great way to collect more feedback about this design.

@leebyron
Copy link
Collaborator

Awesome! Here's a rough schedule of what happens next:

This is a really well written RFC, but this should not deter anyone from asking questions - both getting into the weeds and challenging core assumptions and aspects of the proposal. An RFC is the beginning of a discussion. Let's leave this PR open as a place for discussion for the time being.

@OlegIlyenko
Copy link
Contributor

OlegIlyenko commented Feb 16, 2017

Really happy to see progress on subscriptions! Good job, it looks great so far! 🏆

the input query and variables are mapped to a set of events

It would be interesting to see the semantics of this mapping. In particular, do the root subscription fields have any special semantics or the interpretation of the root field semantics can be defined by a GraphQL server (for example, does every root field represents specific type of event like commentCreated or topic comments or something else)?

Also another remark. The RFC assumes and emphasizes that subscription communication is bi-directional. I wonder whether this restriction is necessary or required. I definitely see appeal of having bi-directional communication. For instance, it can multiplex several subscriptions in a single WebSocket connection and provide extended life-cycle phases for further optimizations.

But in the most basic form, uni-directional communication (server → client) can be sufficient to implement semantics described in the RFC. A while back I implemented an example GraphQL subscription service that is based on SSE, which is uni-directional. As far as I can tell, it satisfies the semantics described in this RFC, but simplifies life-cycle a bit:

  • Subscribe (client → server) - represented by server receiving a client request with subscription query
  • Subscription active (server → client) - absent/implicit
  • Unsubscribe (client → server) - client just closes the connection

Would love to hear your thoughts on this aspect of the RFC :)

Also regarding bi-directional communication. As far as I can tell, it defines a small protocol - something similar to apollo WS transport protocol. Does this mean that in future we will have additional specification, alongside of GraphQL, that will define and standardize this bi-directional communication protocol (life-cycle phases, data format, command types like SUBSCRIPTION_START, SUBSCRIPTION_FAIL, UNSUBSCRIBE, etc.)?

@stubailo
Copy link
Contributor

@OlegIlyenko my interpretation is that "bidirectional" means that in your description there are arrows that go from client -> server and also server -> client.

@taion
Copy link

taion commented Feb 16, 2017

Aren't any client -> server legs just mutations?

This looks consistent with what's implemented between relay-subscriptions and graphql-relay-subscription as well.

A few comments:

@stubailo
Copy link
Contributor

We started our implementation without a "subscription active" notification, but later had to add it. Because the first result from your subscription could arrive a long time after the initial request, you don't know if you're actually receiving those updates. In some cases, you want to be able to look at the state of the subscription to decide if you should be polling for data or simply waiting for events to arrive, especially if you don't want to miss some crucial updates to the information you're looking for.

Essentially, it can be very hard to know what happened if you subscribe and get back total silence - did the subscription somehow fail or stall, or did it successfully start and you should expect to get results in the future? @taion I think you're using socket.io right? Maybe they have a system that checks if the subscription is successful under the hood?

@taion
Copy link

taion commented Feb 16, 2017

@stubailo

To clarify, I'm saying that "subscription active" notifications aren't required in all cases.

It really depends on the app. If it's something like chat, then "subscriptions are down" is a big deal and you want to show some user feedback. For something like, say, showing "new comment" prompts on a GitHub issue page, you don't really care.

A general purpose subscription layer probably wants to expose something like those "subscription active" notifications to handle all uses cases, but they're not always necessary in more specialized cases.

@stubailo
Copy link
Contributor

Good call - I agree that it is not necessary in cases where the subscription is not providing critical functionality. I think one other factor to consider is that returning an error about the initial subscription definitely is necessary - for example, if the query is not valid at all you want to receive some validation error in response to your initial subscription. Without a message that there was a successful subscription (essentially it just tells you the query validated etc) there's this time when you're not sure if there will be an initial error or not.

@taion
Copy link

taion commented Feb 16, 2017

Yup – error handling is necessary in general. I guess it depends how in-detail this spec wants to go on what the network layer should do.

@deanrad
Copy link

deanrad commented Feb 16, 2017

Questions like whether a subscription can do events singly or must be multiple, as asked in the blog post, point to the possbible usefulness of an abstraction like Observable, which is not inherently tied to multiplicity of responses. Also, that it is moving through standardization in the language would be helpful too.

I'm also a fan of the DDP 'ready' event that announces the change from serving up existing old data to serving up new responses. While not strictly necessary, it lets you do things like remove loading graphics as you already know quite well @stubailo :)

My 2c on unsubscribe is it should be a message as well (as in DDP), but obviously the server should do the right thing if a connection goes stale and no unsubscribe was received.

PS I'm in the category of someone whose org may adopt GraphQL if it had subscriptions. Right now we're rolling something on Meteor DDP until we see if Apollo could give us a compatible-enough experience.

@taion
Copy link

taion commented Feb 16, 2017

Observable is really a JS-level implementation detail, no? I don't think it's pertinent at the level of this spec.

@rmosolgo
Copy link

Thanks for writing this up, I'm looking forward to seeing how it evolves! I had a couple questions:

  • Somehow, I was previously under the impression that subscription requests also returned an immediate result, as if they were queries. That's not mentioned here, right? So I was mistaken about that?

  • Would it make sense to add one more arrow to this diagram? I thought there might be one like this:

    image

    After the subscription system is notified of an event, it tells the GraphQL server to re-evaluate the query, and the GraphQL server gives a new response (in magenta) to the subscription system. (Then, the subscription system sends it to the client over the particular transport layer.) Did I understand correctly? That's how I made sense of this arrow in the later diagram:

    image

@taion
Copy link

taion commented Feb 16, 2017

I don't think in general subscription queries should return an immediate result. If I have a subscription on e.g. items being added, what would that result be?

@deanrad
Copy link

deanrad commented Feb 17, 2017

@taion - The point of it wasn't to prescribe an implementation, but to convey semantics by referring to an existing spec to broaden the discussion. The contract covers issues like you mentioned above such as whether you get a result upon subscribe. http://reactivex.io/documentation/contract.html

@smolinari
Copy link

smolinari commented Feb 17, 2017

Whatever the solution may be. I 👍 the addition to the spec, because without a subscription system, GraphQL can hardly be considered as "state of the art", when if fact, it really should be.

Edit: Had to also link to Sashko's great blog post.

Scott

@theorygeek
Copy link
Contributor

theorygeek commented Feb 17, 2017

One thing that's not necessarily clear to me (and to be honest, I haven't been down in the trenches with subscriptions, unlike others commenting here) is this:

When the client wants to create a subscription, is it just executing a GraphQL query with a special operation? Or is it sending a package of "something" (including a GraphQL query, variables, etc) to something else, and then that something else is interacting with GraphQL?

I was previously under the impression that if I sent this query to my GraphQL server, using the exact same transport/etc mechanics as any other query, that it was creating a subscription:

subscription {
  someField {
    # sub-selections
  }
}

In this scenario, the GraphQL library (graphql-ruby in my case) is handling the "book-keeping" of the subscription, and somehow I'm plugging in:

  • Events
  • A special network layer

But after reading this spec, it sounds like I'm instead directing this to something other than my GraphQL library, handling the book-keeping myself, and periodically running a not-so-special query and sending the (black box) payload to the client.

Essentially, what I'm getting at, is should we expect authors of GraphQL frameworks to be providing the book-keeping for subscriptions, etc and then exposing some fancy integration points (events and networking)? Or should we expect a new eco-system of subscription frameworks to grow up alongside existing GraphQL frameworks?

@taion
Copy link

taion commented Feb 17, 2017

I imagine implementation-wise there's not much actual change, right? As set up right now, GraphQL.js can execute a subscription query, but the actual semantics of the full request live outside. It's like the split between graphql and express-graphql for the standard query/mutation side.

@stubailo
Copy link
Contributor

Or is it sending a package of "something" (including a GraphQL query, variables, etc) to something else, and then that something else is interacting with GraphQL?

Yes, the idea is that you can add subscriptions on top of the existing GraphQL execution libraries by adding a subscription manager or subscription gateway. And that thing's job is to re-execute the subscription query against the GraphQL server in response to the event.

I imagine implementation-wise there's not much actual change, right? As set up right now, GraphQL.js can execute a subscription query, but the actual semantics of the full request live outside.

I think that's about right - in terms of a library like GraphQL.js I think the one thing that could be added is a way to map the subscriptions fields to events. I think there would be a lot of value in putting that right next to the resolver code.

Oh, and one more thing - the resolver for a subscription field has one more parameter now, which is the "payload", so now that resolver takes in rootValue, args, context, and payload.

That could come in three different places:

  1. The payload could be passed as the root value, that would be consistent with looking at the subscription as a continuation of the mutation - so the mutation can push its result over the message system, and the subscription field picks up where it left off. But that could be confusing given that root values are used for other stuff as well.
  2. The payload is attached to context, but that feels like a bit of a hack to me especially since it means all fields will be able to access it.
  3. There's a new parameter that only subscriptions field resolvers get access to.

I think I'm in favor of (3), which requires a change to GraphQL.js to have the execution function take an extra subscriptionPayload parameter or similar.

@taion
Copy link

taion commented Feb 17, 2017

It'd be great if running a subscription could return some description of the relevant event set as a first-order concern, though the current workarounds aren't too bad.

I'd prefer subscription payload come through as rootValue for consistency.

@robzhu
Copy link
Contributor Author

robzhu commented Feb 28, 2017

Hey all, amazing feedback and discussion so far, but it’s clear the github PR single-comment-thread feedback format is not suited for hosting multiple conversations at once.

I’m going to go through the feedback and organize everything into open and closed topics. Closed topics are either obvious or have clear consensus and will be merged into the proposal in the next edit. Open topics are still being discussed (I will try to tag everyone who’s been involved/interested on each topic). Once I’ve organized this list, I’ll need everyone’s help:

  • Make sure the list is complete and the categorization of open/closed topics is accurate
  • Make sure your name is tagged on the topics you care about.

After that, I’ll merge this PR (clearly marked as "WIP") and open issues for each of the open topics so that we can have a more focused conversation. As each open topic is resolved, we will make the appropriate edits as part of a new PR. My main goals are to make sure all the great conversations here are captured without competing for attention and that we can make incremental progress on getting this into the Spec.

@smolinari
Copy link

@paralin

What's the difference of a live query to the spec? You're still specifying what data is required in the same syntax.

Somehow a query needs to be designated as "live" by the client. Or, as you said, and I think we are on the same wavelength on this, the queries that can be called as "live" need to be defined on the server in some way and be obvious/ findable through introspection.

Scott

@robzhu
Copy link
Contributor Author

robzhu commented Mar 1, 2017

Here's my attempt to organize this thread into topics:

Closed

  • Add clearer notice that the RFC is a work-in-progress.
  • Add backward arrow from GraphQL to Subscriptions box in diagram (@rmosolgo)
  • Make "subscription active" part of the lifecycle optional and recommended for client-side error-handling (@taion, @stubailo)

Open

Please let me know if I've missed your topic or if you would like to be added/removed from anything. I'll leave this post here for a bit so everyone tagged has a chance to review. After that, I'm going to merge this PR and file issues for all open topics.

@taion
Copy link

taion commented Mar 1, 2017

Do we need a "bi-directional" channel between client and server?

The use of SSE v WS is an application-level implementation detail. All that's needed is the server being able to push messages to the client somehow, which distinguishes this from request/response.

Where should subscription state be stored? (inside GraphQL-js or as a separate component?)

I'd prefer to have this state be stored outside of GraphQL.js. I want to e.g. manage my own Redis subscriptions backing my GraphQL subscriptions, and I feel like the control logic is more straightforward if I have something I can call when my subscription fires, rather than passing e.g. an observable into GraphQL.js.

How should we pass event payloads/data into the subscription resolvers?

This is a framework-level implementation question. I'd prefer GraphQL.js use the existing rootValue field here.

The "event stream" should be derived purely from the subscription root field (and ignore selection set).

I agree with this. It's the "natural" way to write the spec.

Fetching initial state when subscribing.

I think this is an application-level thing. "State" is not a concept that is inherent to GraphQL. Subscribing to e.g. streams of newly created items also doesn't necessarily admit an easy representation of current state. Imagine subscription updates that don't reflect idempotent operations – in such cases there isn't really a coherent initial state.

Re-define Live Queries? Can Live Queries make Subscriptions unnecessary?

@stubailo's point is exactly right here. Live queries should be thought of separately from event-based subscriptions. They can be used to solve similar problems. However, scaling event-stream-based subscription systems is relatively well-understood; the same does not apply for live queries, and for anyone looking to ship something scalable now, live queries aren't really an option.

Intermediate Subscription event processing

This is an application-level detail. If someone wants to subscribe to e.g. a "top X" stream but not re-process that stream, then there's nothing at the framework level that prevents e.g. the "trending tweets" subscription from using different events than the "new tweet" subscription.

@paralin
Copy link

paralin commented Mar 2, 2017

@taion Live queries certainly could be scalable, especially with GraphQL's emphasis on separating logic for each field. I'm looking at implementing a scheduler for Magellan that could do that.

But, you're right. Subscriptions are a different topic from live queries. And IMO, live queries don't need a change to the spec to be implemented. Directives are server-specific with introspection, and that should be all you need.

@taion
Copy link

taion commented Mar 3, 2017

Just to clarify, when I say "live queries", I mean something like the @live directive mentioned in talks, where the interface presented to the client is one where the client can request a particular query be live, rather than one where the client explicitly requests specific streams.

For an example of a subscription-based API, see https://github.com/edvinerikson/relay-subscriptions/blob/v1.0.0/examples/todo/js/components/Todo.js#L124-L148.

If this were using live queries, then perhaps it would just look like:

fragment on Todo @live {
  id
  complete
  text
  # ...
}

However, if you're doing various things such as using microservices and splitting up GraphQL objects across multiple backend services, &c., how to efficiently implement live queries can be much less than obvious, while event-based systems are much more straightforward.

@leebyron
Copy link
Collaborator

leebyron commented Mar 7, 2017

Since there's excitement and support for the direction of this RFC, I'm going to commit it as an official tracked RFC. @robzhu just opened up issues for all of the various topics still under discussion - so this is still very much an open design!

@leebyron leebyron merged commit 0681a50 into graphql:master Mar 7, 2017
@stubailo
Copy link
Contributor

stubailo commented Mar 7, 2017

So excited!

@smolinari
Copy link

Me too! 👍

Scott

@thetwosents
Copy link

AHHHHHHHHHHHHH!!!!!!!

@ghost ghost mentioned this pull request Apr 9, 2017
65 tasks
IvanGoncharov pushed a commit to IvanGoncharov/graphql that referenced this pull request Jun 17, 2017
* RFC: GraphQL Subscriptions

* Fix cutoff content from last checkin

* Fix diagram spacing

* Fix diagram spacing for real

* Minor wording fixes and higher-res images

* More wording clarity fixes

* Make overpushing/underpush wording consistent
@acjay acjay mentioned this pull request Nov 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet