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

Add support for brokered subscriptions #776

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jscheid
Copy link

@jscheid jscheid commented Aug 31, 2019

This is work in progress, for now I'm putting it up here only to start a conversation.

The aim of this change is to enable use of message brokers for delivering subscription updates. I'm planning to use it with AWT IoT, but it should pave the way for using any broker, including other MQTT/AMQP/etc brokers and commercial offerings such as Pusher or Ably.

This requires two changes outlined below.

Subscription Documents Storage Abstraction

The Registry works well when the lifetime of a subscription is tied to an underlying WebSocket, but doesn't for the case where subscriptions are "out of band" in the sense that there is no direct connection between client and server.

Specifically, when a node goes down it takes all the documents in its Registry with it, which isn't a problem for WebSockets as the client will be forced to reconnect anyway. It is a problem when a broker is used since there's no easy way to make the client aware of the need to set up the subscription anew.

There's also a wrinkle in that the Registry implementation is tied to the HTTP request pid, which works well only when both the subscription setup and update delivery are handled in the same request.

All of this is why the major change here is to move storage behind an abstraction so that it can be swapped out. I've included an alternate storage layer based on Redix for illustrative purposes; it wouldn't necessarily have to stay for the final PR, although I'd be happy to donate it. (It's incomplete though, I'm planning to add expiry/keep-alive in order to preempt misbehaved clients. This shouldn't affect the interface though.)

Context During Publishing

The other change is passing the Absinthe context to publish_subscription. This is useful because publish_subscription can use it to construct an (external) topic that includes some kind of user indentification, which in turn enables access control. Without being able to scope topics by user, authorizing access is much less convenient and efficient.

absinthe_plug changes

There's also a change necessary to absinthe_plug, which currently assumes that subscriptions are updated through a WebSocket. I'll open a separate PR for that.


What do you think about these changes? If you agree with them in general I'm happy to spend some time on polish and adding a few tests. As far as naming goes I'm not attached to the names I've picked, feel free to suggest alternatives.

One open question is how to handle backward compatibility, as you can see I've put in a transparent fallback for the first change but not for the second. Happy to tweak that for consistency, one way or the other.

Move storage of subscriptions and documents behind an abstraction
layer.

Pass Absinthe context to `publish_subscription`.
@benwilson512
Copy link
Contributor

Hey @jscheid I'm really excited to see this PR, I've wanted to do this exact thing for a while. Are you able to link to what an alternative store would look like? I think that will help me evaluate the context related changes better.

We have a similar a sort of hacked together internal setup for doing subscriptions over SQS, stored in postgres, and so on my end I'll throw together a postgres version of the store so that we can make sure it all works in several scenarios.

@jscheid
Copy link
Author

jscheid commented Sep 2, 2019

Hi @benwilson512, there's a Redix implementation in this PR, is that what you mean?

@@ -0,0 +1,67 @@
defmodule Absinthe.Subscription.RedixStore do
Copy link
Contributor

Choose a reason for hiding this comment

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

As a minor note, while I definitely appreciate this as an example, I definitely don't want to have Absinthe ship with this built in. It couples Absinthe too closely to a specific redis library. I'd be more than happy however to promote an external package that contained this code however.

Copy link
Author

Choose a reason for hiding this comment

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

That's fine, as I said, it's only an illustration.

@benwilson512
Copy link
Contributor

@jscheid I'm not seeing how that PR uses the new context argument passed to publish_subscription.

@jscheid
Copy link
Author

jscheid commented Sep 3, 2019

@benwilson512 this PR doesn't use the new context argument. It's meant for use by alternate pubsub implementations that want to scope the topics by some user information. No such alternate implementations are included -- the purpose of the PR is to make those possible, not add them. Does that make sense, and can you see the need for providing publish_subscription with the context?

@benwilson512
Copy link
Contributor

Does that make sense, and can you see the need for providing publish_subscription with the context?

Not yet I guess. The topic when generated by the subscription setup function has access to the context, and can embed any data it wants at that point in time. Without a concrete example to work off I'm hesitant to change this public behaviour function.

@jscheid
Copy link
Author

jscheid commented Sep 3, 2019

Ok, maybe I'm missing something. Essentially the problem I'm trying to solve is this:

If you take SQS as an example, the client wouldn't subscribe to one queue per subscription, but instead use a single queue for all subscriptions (because that is both more efficient in terms of HTTP traffic and cheaper in terms of AWS pricing.) With the context information, publish_subscription can derive the name of that queue from some information in the context -- maybe a client ID.

You make it sound like there may be a different way to do this but I can't see it. What exactly are you referring to by "subscription setup function"?

@jscheid
Copy link
Author

jscheid commented Sep 3, 2019

Oh, I think I see. You mean subscription > field > config/2.

But wouldn't that mean that topics couldn't be shared by subscriptions anymore? For instance, if you have some global, public state, would you want to use a single topic for it rather than generating a different topic per subscriber?

@jscheid
Copy link
Author

jscheid commented Sep 12, 2019

Hey @benwilson512 , no rush on this one but just wanted to check in to see if you were waiting for anything from me? As I said, I'm happy to make changes but not entirely sure yet what you had in mind.

@benwilson512
Copy link
Contributor

benwilson512 commented Sep 19, 2019

Hey @jscheid I am not waiting on you. I completely understand now why you're passing in the context. The delay is most around my wish to re-evaluate slightly how document execution and publication happens so as to make it easier to set some of those values. Your solution may well be the best option, but now that I'm aware of the problem I'm thinking through some other available options for solving it.

@jscheid
Copy link
Author

jscheid commented Sep 20, 2019

Thanks for the update @benwilson512. Sounds good, take your time!

@binaryseed
Copy link
Contributor

Am I right in understanding that this is an attempt to enable a subscription to "switch" transports? ie: it starts as an HTTP request and then is delivered via message queue?

I have a few questions:

  1. Do any other GraphQL frameworks do this?
  2. Can this be accomplished by modeling it into your Schema? ie: return a subscriptionId (or whatever identifier needed) alongside your subscription return type?

@jscheid
Copy link
Author

jscheid commented Jan 7, 2020

That's correct, the point of this PR is to enable updates that are delivered through a different transport mechanism than other GraphQL requests and responses.

  1. Yes, ruby-graphql provides a similar feature. You subscribe with a normal GraphQL request through HTTP, then the updates are delivered out-of-band, for example via Ably. It looks like other frameworks support out-of-band updates too, such as Apollo.
  2. Sorry, I'm not entirely sure I understand the question. The subscription ID is metadata, as such I don't think it belongs in the schema.

@jscheid
Copy link
Author

jscheid commented Jan 7, 2020

@binaryseed there's also some deliberation on the subscription ID delivery in the sibling PR description.

@derekbrown
Copy link

Wondering the state of this PR? Those of us looking to use Apollo and Absinthe in production would love to use it (and an example if someone has the time!)

@benwilson512
Copy link
Contributor

Hi @derekbrown to be clear, you can use apollo and Absinthe in production today via https://github.com/absinthe-graphql/absinthe-socket/tree/master/packages/socket-apollo-link

This PR is about enable subscriptions over something like SQS

@derekbrown
Copy link

derekbrown commented Mar 3, 2020

We're using Apollo on iOS.
https://hexdocs.pm/absinthe/apollo.html

@benwilson512
Copy link
Contributor

What would be the transport mechanism for subscriptions? If you can do websocket / HTTP you also don't need these changes. If you're looking for APNS then this might be more relevant.

@derekbrown
Copy link

What would be the transport mechanism for subscriptions? If you can do websocket / HTTP you also don't need these changes. If you're looking for APNS then this might be more relevant.

We're using websockets, but (unless I'm missing something which may very well be the case!) the Apollo operation message shape requires a transform. Something akin to this acts as a bandaid, but far from ideal.

@benwilson512
Copy link
Contributor

It goes beyond the operation shape. Apollo websocket clients use a websocket frame protocol that is specific to Apollo server. This PR isn't going to help with either the operation shape NOR the websocket frame protocol. Absinthe as a library is already 100% transport agnostic. I'd consider making a forum post about connecting to Absinthe via apollo.

The brokered subscription concept has more to do with storing documents that aren't connected to specific processes. If you're using websockets at all you can use the existing process oriented document storage. Absinthe itself isn't the bottleneck here, and this PR will not help you. I'm not an iOS developer so I'm not 100% sure what options exist out there, but you either need to use the library you linked to make Phoenix talk Apollo's frame format, or you need to find a plugin for the apollo ios library that enables it to talk the phoenix channel frame format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants