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

Is a SUBSCRIBE_UPDATE with Start Group in the past an error? #595

Open
martinduke opened this issue Oct 22, 2024 · 8 comments
Open

Is a SUBSCRIBE_UPDATE with Start Group in the past an error? #595

martinduke opened this issue Oct 22, 2024 · 8 comments
Labels
Editorial The draft is difficult to understand on a specific point, or it is open to multiple interpretations. Needs PR Subscribe Related to SUBSCRIBE message and subscription handling

Comments

@martinduke
Copy link
Contributor

It is for SUBSCRIBE. Maybe SUBSCRIBE_UPDATE shouldn't have a start group at all?

@ianswett
Copy link
Collaborator

You can't widen a SUBSCRIBE with a SUBSCRIBE_UPDATE, so I don't think there's an issue?

@martinduke
Copy link
Contributor Author

SUBSCRIBE (Group 0, Group infinity)
<Groups 0 to 5 arrive>
SUBSCRIBE_UPDATE(Group 3, Group Infinity)

Should be an error.

One way to solve this would be to just get rid of start group, if there's no use case

@ianswett
Copy link
Collaborator

I think there are potential use cases, so I'd prefer not to get rid of start group yet. For example, if you wanted to switch to a higher resolution, but realized you weren't going to get quite enough of the Group to switch up at the next group boundary, you could bump the start group by 1.

I think your example could be racy. Groups 0 to 5 are sent, and when they're in flight, the SUBSCRIBE_UPDATE(Group 3) is sent and possibly lost and needs to be retransmitted. The fact that SUBSCRIBE_UPDATE is sent on a reliable stream and could be lost and need to be retransmitted means it could end up being received a while after it was originally sent.

Also, updating to a group that is in the past by the time it's received seems rather easy to handle and innocuous. Does it create a problem I'm not thinking of?

@ianswett ianswett added the Subscribe Related to SUBSCRIBE message and subscription handling label Nov 10, 2024
@martinduke
Copy link
Contributor Author

It seems inconsistent to me that SUBSCRIBE to the past is an error and SUBSCRIBE_UPDATE isn't. Aren't all the arguments you made equally valid to gracefully handle such a SUBSCRIBE?

@afrind
Copy link
Collaborator

afrind commented Nov 11, 2024

Individual Comment:

In implementing FETCH / SUBSCRIBE, I ran into trouble by making Start Group in the past an error for SUBSCRIBE. My recommendation is to treat this like the range in FETCH -- as long as the receiver will get anything it's not an error, and the OK will tell you where the thing actually starts. Maybe then we can be consistent with SUBSCRIBE and SUBSCRIBE_UPDATE, though UPDATE has no OK.

Here's what I said in slack

FETCH with start in the future or SUBSCRIBE with start in the past are both errors.
If I have an absolute start and end group and no idea where the live head is, I need to:

latest = SUBSCRIBE(LatestObject)
If start <= latest:
  if end <= latest:
    // entirely in the past
    UNSUBSCRIBE()
  // Fetch all the past bits
  FETCH(start, min(latest, end))
if end > latest:
  SUBSCRIBE_UPDATE(end=end)

On the plus side, this tests out 4 verbs!
But maybe we can relax SUBSCRIBE so I can pass start and end, and just return SUBSCRIBE_OK with latest. I won't get any objects <= latest. That would eliminate the need to SUBSCRIBE_UPDATE or UNSUBSCRIBE.

@martinduke
Copy link
Contributor Author

Individual Comment:

In implementing FETCH / SUBSCRIBE, I ran into trouble by making Start Group in the past an error for SUBSCRIBE. My recommendation is to treat this like the range in FETCH -- as long as the receiver will get anything it's not an error, and the OK will tell you where the thing actually starts. Maybe then we can be consistent with SUBSCRIBE and SUBSCRIBE_UPDATE, though UPDATE has no OK.

Here's what I said in slack

FETCH with start in the future or SUBSCRIBE with start in the past are both errors.
If I have an absolute start and end group and no idea where the live head is, I need to:

latest = SUBSCRIBE(LatestObject)
If start <= latest:
  if end <= latest:
    // entirely in the past
    UNSUBSCRIBE()
  // Fetch all the past bits
  FETCH(start, min(latest, end))
if end > latest:
  SUBSCRIBE_UPDATE(end=end)

On the plus side, this tests out 4 verbs!
But maybe we can relax SUBSCRIBE so I can pass start and end, and just return SUBSCRIBE_OK with latest. I won't get any objects <= latest. That would eliminate the need to SUBSCRIBE_UPDATE or UNSUBSCRIBE.

SGTM

@ianswett ianswett added Editorial The draft is difficult to understand on a specific point, or it is open to multiple interpretations. Needs PR labels Nov 13, 2024
@ianswett
Copy link
Collaborator

I added Needs PR, under the assumption that an editorial PR might be helpful, but I'm not sure exactly what that PR would contain?

@afrind
Copy link
Collaborator

afrind commented Nov 13, 2024

Individual Comment:

My strawman is:

  1. SUBSCRIBE with start group in the past is not an error. If the live head is past start group, it will be conveyed in SUBSCRIBE_OK.
  2. SUBSCRIBE with end group in the past is still an error, because the subscription will deliver no objects. Use an error code like FETCH_ERROR when the entire range is in the future.
  3. SUBSCRIBE_UPDATE cannot widen the subscription, but narrowing it with start in the past is not an error. Might need some informative guidance on how a publisher cleans up streams in newly skipped groups.

Separate question if SUBSCRIBE_UPDATE should operate on group boundaries only? I'm concerned about the case where I subscribe at (0,0) and relay begins delivering subgroup 1 which contains objects 0-10. After object 2 I update subscribe to start at object 7. Does relay send:

Stream X: 0, 1, 2, 7, 8, 9, 10

or

Stream X: 0, 1, 2<RELIABLE_RESET>
Stream Y: 7, 8, 9, 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Editorial The draft is difficult to understand on a specific point, or it is open to multiple interpretations. Needs PR Subscribe Related to SUBSCRIBE message and subscription handling
Projects
None yet
Development

No branches or pull requests

3 participants