Subscription priming/ordinals

[bernardd]

This is the absinthe part of my attempt to address the issue discussed over in https://elixirforum.com/t/subscription-catchup-state-delta/47459. There will be a corresponding absinthe_phoenix PR shortly.

I still need to add documentation, so consider this a WIP, but I'd appreciate feedback on whether this is going to be an acceptable way forward before I spend time writing it :)

There are 3 main elements on which the solution is built:

Continuations

This is a trimmed down version of the continuation system I wrote to implement the @defer directive in #559. I used this generalised system because it seems like it could be a useful addition for other features in the future (@defer being the obvious example) and I didn't want to just write code that was specific to subscription priming. Plus @benwilson512 mentioned at the time that he liked the idea :)

Subscription priming

This is a rework/renaming of the subscription-catchup system I wrote a while back. Subscription configurations are given an optional prime parameter which is a function called when a user first subscribes. The result of that function is sent only to the subscribing user, allowing them to "catch up" to where the subscription state is for all other subscribers.

Subscription ordinals

(Feel free to come up with a better name - "ordinals" was the best I could think of).
This feature addresses the issue of the asynchronous handling of subscription updates which have a strict semantic ordering, such as state updates. This is the specific issue that is explained in the Elixir Forum post above. Subscription configs can now be given an ordinal function which is passed the root_value of the subscription update and returns a term which will be used for ordering of updates. The actual functional bit of this (dropping out-of-date updates) is performed over in the corresponding absinthe_phoenix changes (which I'll reference here as soon as I've opened that PR). The key functionality here, though, is to have the user able to specify a way to determine ordering on updates. This could be a timestamp, simple integer or any other value that will be < when compared between older and newer updates. (I did consider allowing a customisable comparator function too but that seemed like overkill to me - happy to do so though).

For all these changes I've tried to leave everything backwards compatible, so not specifying a prime or ordinal function will behave exactly as it does now.

[benwilson512]

Hey @bernardd thanks for this. Hoping to review later today.

[eliasdarruda]

If i understood correctly, this prime mechanism can be also used to just send data to the connected client as a callback after the topic subscription right?

Having this in mind WDYT of this name alternative suggestions?

prime

• data
• data_callback
• client_callback
• callback

ordinal

• data_callback_filter_by
• client_callback_filter_by
• data_filter_by
• callback_filter_by

[bernardd]

It could be used for that, certainly, though only at the initialisation of the subscription, not at arbitrary later points.
I'm reluctant to endorse something like data because it's about the most generic possible word in programming and doesn't really convey the particular intent or behaviour of this feature.

[derekbrown]

Pinging @benwilson512 again here on a review; priming in particular would be very helpful for our use case.

[derekbrown]

Cursory review; I want to dig in a bit more here too, particularly on the ordinals/continuation logic.

[derekbrown]

Thoughts on a more descriptive name like after_connect or initial_response or initial or something similar?

[derekbrown]

FWIW, I'm fine with prime, just there may be something more apt/intuitive here.

[bernardd]

prime was suggested by @benwilson512 and I kind of like it since it's both terse and descriptive. after_connect doesn't really fit because the "connection" has nothing to do with this. It's after a successful subscription that it's sent. The other two options I don't really have any problem with but nor do I think they really have anything to recommend them over prime.

[derekbrown]

Sure. after_subscribe also viable. But either way, if it LGTBen, then it LGTM. LOL.

[benwilson512]

@bernardd As a quick sanity check, does this handle the race condition highlighted here where the item pushed would happen prior to the client actually subscribing to the phoenix pubsub topic?

[bernardd]

Well this is awkward. I was all set to explain how it did and...it doesn't. I'd entirely missed the subtlety that PubSub.subscribe isn't called until after the subscription itself has returned. I think this should be fixable, though. I'll get back to you :)

[bernardd]

Okay, it totally does handle that condition now :)

The prime callback function is now passed through in the continuation and not actually called until the continuation is executed. This means that the initial subscription request has returned back up to absinthe_phoenix, which has in turn had its chance to call PubSub.subscribe. It then calls Absinthe.continue which executes the prime function. Since the subscription is already in place, the state produced by the prime call must be calculated strictly after that point, leaving no window in which for state updates to get lost.

Note that this did require one extra tweak: continuations can now return a special :no_more_results result, which indicates to the caller that there's actually nothing more to send back to the client even though there was a continuation returned by the previous run or continue call.

[benwilson512]

OK this is cool.

[bernardd]

Hey @benwilson512 - are you happy with the general approach here? If so I'll start adding some docs to get this closer to being mergeable. Cheers.

[benwilson512]

@bernardd a couple of quick updates:

1. I am quite happy with the approach here.
2. I am on paternity leave for the next two months. As I am able in this time I hope to catch up on some of my open source projects, and this PR is the top of the list.

As a general note, I am extremely grateful for your patience with this project and myself. You've submitted really thoughtful work over the years and I have done a very poor job of keeping up with reviews or helping guide them towards a merge.

Thanks for sticking with it!

[benwilson512]

OK so as far as the path to getting this merged goes we have:

1. Ben needs to finish detailed review
2. Bike shed about the name "ordinal".
3. Bike shed return values a bit
4. Documentation.

On the (4) front, this PR is gonna require some updates to the subscriptions.md guide. Obviously let's get the bike shedding about terms done first so that we don't have to make to many changes to the docs once written.

[benwilson512]

I'm still working through the details here but the switch from plural to singular has me confused.

[bernardd]

Me too, apparently :) Initially I think I had it only supporting a single continuation, but I realised potentially there may be multiple ones, so changed it but didn't do a very thorough job of it. I've fixed it up now.

[benwilson512]

I would prefer that we did not add the ordinal key to the output if no ordinal function is configured.

[bernardd]

Yep, makes sense. I've made that change.

[benwilson512]

I take it the purpose of the {:ok, } tuple wrapping for now is to leave room for other return tuples in the future? Is it conceivable that prime would return some non :ok value?

[derekbrown]

Personally, I think we should have the prime function return the tuple, similar to resolutions. Otherwise you could conceivably have a {:ok, {:error, :my_error_state}} returned here or something equally wonky.

[bernardd]

My thinking was mostly just to keep it consistent with return types in the other resolution callback functions. You're right that it doesn't serve much purpose at the moment, but I think at the time I was thinking we may want to allow error conditions to be propagated up like in resolutions. I'm not quite sure how that would be presented at a graphQL level though.

[derekbrown]

Bike shed about the name "ordinal".

I like ordinal in general, but it's not super clear at first. Once you understand what it does, it does make sense, but it doesn't help devs intuitively understand what it's for.

FWIW, I feel the same way about prime. Not opposed, but it's not intuitive. Something like after_subscribe or on_subscribe or equivalent is probably more clear, though a bit more verbose.

Shout out to @bernardd for sneaking in mathematical terms tho. ;)

[bernardd]

2. I am on paternity leave for the next two months. As I am able in this time I hope to catch up on some of my open source projects, and this PR is the top of the list.

Congratulations! Don't push yourself on my account - take care of yourself and your family first :) I know how much work a new kid can be even under the best of circumstances.

As a general note, I am extremely grateful for your patience with this project and myself. You've submitted really thoughtful work over the years and I have done a very poor job of keeping up with reviews or helping guide them towards a merge.

Not a worry - in case I didn't already know it, taking over ExAws has reminded me just how easy it is to let PRs slide when it's not top priority at your day job :)

[bernardd]

I like ordinal in general, but it's not super clear at first. Once you understand what it does, it does make sense, but it doesn't help devs intuitively understand what it's for.

That's what docs are for :P More seriously, I'm all for a more intuitive name if we can come up with one.

Shout out to @bernardd for sneaking in mathematical terms tho. ;)

"Prime" was entirely Ben's idea, and "ordinal" was simply because I literally couldn't think of a better word :)

[AdSkipper1337]

Hey, I am working on a project where I more or less desperately need this feature at this point.
So I was looking at the code, and realized that there is no subscription callback and no way I could think of to hack it in any halfway sane way to make it work.
Then I came here to offer to make this feature for you from scratch, but now I am delighted to see that @bernardd has already taken to it. Is there anything I can help with to get this done faster? This has not been touched for some time now and its pretty important for my project. For now I will use the feature branch. Also if you want a good name for it, just call it after_subscription_callback, or just subscription_callback, because that is literally what it is. prime and ordinal are confusing imo

[AdSkipper1337]

Also just consider the following observation from me testing this: It would have already been enough to just have a callback where I know that the subscription has already happened. Everything else can be done more or less manually by just calling the publish functions, in my case having the prime functionality where I also send the result of this function to the client is a welcome addition as it solves another problem that I have - but it does not HAVE to be the case. There is a possibility that you do book keeping manually because you have a complex system of which subscriptions are actually triggered, and so you might or might not trigger the prime subscription, you do not know for sure yet, but you still need to register and unregistered the subscription in your bookkeeping. The feature that is missing the most and which makes it impossible to do something manually about it, is that you do not know when you have finished subscribing and unsubscribing... which this feature definitely handles, but it also does more then that which might again limit the possibilities for some people as it is a constrain on what you can do.

[oortlieb]

Are there any plans to continue this (or similar) work? I would love a way to send an initial value when a user establishes to a subscription. I'd be happy to try contributing if there's a bandwidth issue!

[benwilson512]

Hey @bernardd do you mind emailing me when you have a moment? My email address is my github username at gmail.com

[bernardd]

Hey @benwilson512 - I did send an email - let me know if it didn't reach you.