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

'the hub terminates the subscription' #119

Closed
marten-de-vries opened this issue Aug 23, 2017 · 22 comments
Closed

'the hub terminates the subscription' #119

marten-de-vries opened this issue Aug 23, 2017 · 22 comments

Comments

@marten-de-vries
Copy link
Contributor

@marten-de-vries marten-de-vries commented Aug 23, 2017

  1. Content Distribution - last sentences:

Hubs SHOULD retry notifications up to self-imposed limits on the number of times and the overall time period to retry. When the failing delivery exceeds the hub's limits, the hub terminates the subscription.

This suggests to me that the hub should remove the corresponding (topic url, callback url) object from its database. It would mean that from that moment on, the hub would act like the subscription never existed. But that conflicts with this comment:

#16 (comment)

Could anyone clarify the intention of the spec here? Thanks!

@julien51
Copy link
Collaborator

@julien51 julien51 commented Aug 23, 2017

Huh... indeed this is a mistake in my mind. Hubs should not terminate subscriptions arbitrarily because in this specific case a subscriber would not know about it :/

@aaronpk
Copy link
Member

@aaronpk aaronpk commented Aug 23, 2017

but it also isn't reasonable for a hub to keep retrying delivery indefinitely

@julien51
Copy link
Collaborator

@julien51 julien51 commented Aug 23, 2017

You are right the hub can drop the notification... not the subscription. The subscription MUST have an expiration date so it will not retry indefinetly!

@aaronpk
Copy link
Member

@aaronpk aaronpk commented Aug 23, 2017

I don't think I understand what you mean by dropping the notification vs the subscription.

Do you mean that the hub can give up on delivering a particular notification, but if a new item is published to the feed then it should retry delivery to that previously-failed subscriber again for the new notification?

@julien51
Copy link
Collaborator

@julien51 julien51 commented Aug 23, 2017

Exactly! After reasonably trying to send a given notification, the hub SHOULD be able to drop it... but since the hub has no way of knowing that the subscriber will permanently fail, it should try other upcoming notifications as well (and drop them if they fail too), until the subscription expires.

@aaronpk
Copy link
Member

@aaronpk aaronpk commented Aug 23, 2017

So in that sense, the lease_duration the hub returns to the subscriber is a commitment to continue attempting delivery of notifications for the full duration of the lease.

It sounds like we might need to adjust the quoted paragraph to say the hub can stop delivering that particular notification rather than terminating the whole subscription.

@julien51
Copy link
Collaborator

@julien51 julien51 commented Aug 23, 2017

👍

@aaronpk
Copy link
Member

@aaronpk aaronpk commented Aug 23, 2017

Previous text:

Hubs SHOULD retry notifications up to self-imposed limits on the number of times and the overall time period to retry. When the failing delivery exceeds the hub's limits, the hub terminates the subscription.

Proposed text:

Hubs SHOULD retry notifications up to self-imposed limits on the number of times and the overall time period to retry. When the failing delivery exceeds the hub's limits, the hub stops attempting to deliver that nofication. The hub MUST keep the subscription active until the end of the lease duration, and if a new update is published to the topic, MUST continue to retry delivery to the previously-failed subscriber.

@julien51
Copy link
Collaborator

@julien51 julien51 commented Aug 23, 2017

LGTM! Minor thing "previously-failed subscriber" => "previously-fail__ing__ subscriber"

@aaronpk
Copy link
Member

@aaronpk aaronpk commented Aug 23, 2017

I've added this to the agenda for the next call: https://www.w3.org/wiki/Socialwg/2017-08-29#Topics

btw @julien51 I might not be able to make that call so I'd appreciate if you could plan on being there to take this issue if needed!

@julien51
Copy link
Collaborator

@julien51 julien51 commented Aug 23, 2017

Ho yes! Sorry I missed the previous ones :/

@marten-de-vries
Copy link
Contributor Author

@marten-de-vries marten-de-vries commented Aug 24, 2017

The proposed text looks good to me, it would simplify the implementation I'm working on and also not cause subscriptions to be suddenly dropped from the subscriber perspective. Thanks for picking this up so quickly!

@sandhawke
Copy link
Collaborator

@sandhawke sandhawke commented Aug 29, 2017

@aaronpk is there a test for this? I'm guessing not. Sounds like a new hub test, right? Ask the hub to deliver something to a failing receiver, then ask again? Can't be sure about the test without knowing how long it takes for the hub to give up, though.

In any case, it sounds like this could make some hubs no-longer-conforming, which would make this a normative change, requiring a new CR. :-(

Unless maybe it's clear this is what everyone has always done, or what the spec has always implied one should do? Or maybe we could make this a SHOULD not a must.

Looks like neither of you will be at today's meeting, so I guess best to just continue this here for now.

@julien51
Copy link
Collaborator

@julien51 julien51 commented Aug 29, 2017

I don't think this is a change in "approach" (at least this is how Superfeedr has been operating the whole time...).

Also it is hard to test because it is based on the assumption that the subscriber "fails" intermittently.

@sandhawke
Copy link
Collaborator

@sandhawke sandhawke commented Aug 29, 2017

The question (in my favorite practical framing) is whether anyone who's implemented some part of WebSub would have to change it to remain in conformance, if this change is made. If so, they need to be given due notice.

@tantek
Copy link
Member

@tantek tantek commented Aug 29, 2017

Questions from the WG: Is this a normative change? Either way, does it affect current implementation conformance or interop?

@julien51
Copy link
Collaborator

@julien51 julien51 commented Sep 4, 2017

I don't think it affects interop at all.

@sandhawke
Copy link
Collaborator

@sandhawke sandhawke commented Sep 4, 2017

To be a little more clear in the question:

Is it possible there is a websub-conformant system which acts as a websub consumer or publisher which will unexpectedly misbehave because a hub it's dealing with has decided to make this change?

To say it doesn't affect interop is, I think, to say "No, it is not possible for there to be such a system."

What happens in the worst cast, when a system interacts with a hub with doesn't do this change when it's expected to, or which does this change when it's not expected to?

@julien51
Copy link
Collaborator

@julien51 julien51 commented Sep 4, 2017

No I don't think any existing implementation would break here.

Basically, if an implementation existed like that, when a subscriber actually failed to receive the hubs' notifications, that subscriber would have had to 1) know that it failed and 2) know that the hub actually tried to deliver something then... which, to my knowledge is impossible.

Since this was a "MAY", even if 1) and 2) were possible, the subscribe still would have no way to know what the hub actually did then.

This means that the change that we propose is actually making things more rebust, because the subscriber now knows for sure that the subscription was not dropped (unless it expired).

We're going from a situation where the state is unclear (is the subscriber still subscribed?) to a situation where it is known (if the subscription (time)expired, the subscription does not exist and if it did not, the subscription still exists)

@sandhawke
Copy link
Collaborator

@sandhawke sandhawke commented Sep 4, 2017

Okay, then the issue is whether some hub maker would have a problem with this. Can we enumerate the hub makers and whether they do this already, and if they're happy with the change?

@sandhawke
Copy link
Collaborator

@sandhawke sandhawke commented Sep 5, 2017

The WG discussed it today, although with a small group, and resolved this shouldn't be considered substantive, even though it is normative. See http://www.w3.org/2017/09/05-social-irc#T17-22-41

I'll bring it up with the Director and see if we can get a ruling.

Answers to my previous question would still be useful, though

julien51 added a commit that referenced this issue Sep 10, 2017
aaronpk added a commit that referenced this issue Sep 13, 2017
added to source.html and regenerated index.html with respec
aaronpk added a commit that referenced this issue Sep 18, 2017
@aaronpk
Copy link
Member

@aaronpk aaronpk commented Sep 19, 2017

This has been added in the editor's draft

@aaronpk aaronpk closed this Sep 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.