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

Announces with same name in reconnect #227

Closed
fluffy opened this issue Jul 24, 2023 · 11 comments
Closed

Announces with same name in reconnect #227

fluffy opened this issue Jul 24, 2023 · 11 comments
Assignees
Labels
Announce Issues with Announce message and handling Needs PR

Comments

@fluffy
Copy link
Contributor

fluffy commented Jul 24, 2023

When the relays received an ANNOUNCE with exact same name as previous ANNOUNCE, what happens?

Options seem to be replace previous ANNOUNCE data or return an error.

This is related to #225 and #204

Discussion to follow in thread

@fluffy
Copy link
Contributor Author

fluffy commented Jul 24, 2023

One of use cases we should think about is a client that crashes and reconnects.

@ianswett ianswett added Architectural Needs Discussion Tags for issues which need discussion, ideally at an interim or IETF meeting. labels Jul 24, 2023
@afrind
Copy link
Collaborator

afrind commented Jul 27, 2023

As an individual,

The crash (or disconnect) use case is compelling for "the most recent one wins". Are there applications where that policy would break something?

@fluffy
Copy link
Contributor Author

fluffy commented Jul 27, 2023

I agree with Alan that "most recent" overwrites any earlier ones is best path forward.

@suhasHere
Copy link
Collaborator

+1 on most recent overwrites. this is what we do atleast today and it works fine to the amount we have tested.

@kixelated
Copy link
Collaborator

kixelated commented Jul 27, 2023

Going with the most recent ANNOUNCE works for me, so long as the relay MUST send an ANNOUNCE_ERROR and UNSUBSCRIBE for any matching subscriptions to the original session. Otherwise we'll introduce zombie announcements and subscriptions.

@fluffy
Copy link
Contributor Author

fluffy commented Jul 27, 2023

WFM

@suhasHere
Copy link
Collaborator

Going with the most recent ANNOUNCE works for me, so long as the relay MUST send an ANNOUNCE_ERROR and UNSUBSCRIBE for any matching subscriptions to the original session. Otherwise we'll introduce zombie announcements and subscriptions.

I Dont think existing subscriptions needs to be terminated.

Let say the case where, A publisher disconnects due to getting into an elevator for a moment and sends announce on reconnect, sending out protocol messages to cancel the subscriptions on the second announce is too noisy.

@afrind
Copy link
Collaborator

afrind commented Jul 28, 2023

As an individual:

I don't think we need to force unsubscribe everyone if the relay successfully resubscribes to the new ANNOUNCE.

I think Luke is suggesting that the relay unsubscribe on the session that issued the old ANNOUNCE?

@kixelated
Copy link
Collaborator

kixelated commented Jul 28, 2023

As an individual:

I don't think we need to force unsubscribe everyone if the relay successfully resubscribes to the new ANNOUNCE.

I think Luke is suggesting that the relay unsubscribe on the session that issued the old ANNOUNCE?

Yeah exactly. The upstream subscription is reissued to the new connection, but any downstream subscriptions can be maintained by updating a lookup table. I can elaborate if this isn't clear; the glue between "upstream" and "downstream" subscriptions can be confusing.

@suhasHere
Copy link
Collaborator

Got it. Makes sense. Agree all the downstream subscriptions can stay intact for the announced namespace

@afrind afrind added the Announce Issues with Announce message and handling label Oct 3, 2023
@afrind afrind moved this to Needs Discussion in MoQT Tracking Board Aug 15, 2024
@afrind afrind removed the Needs Discussion Tags for issues which need discussion, ideally at an interim or IETF meeting. label Aug 15, 2024
@afrind afrind moved this from Needs Discussion to Needs PR in MoQT Tracking Board Aug 15, 2024
@ianswett
Copy link
Collaborator

Closed by #525

@github-project-automation github-project-automation bot moved this from Needs PR to Done in MoQT Tracking Board Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Announce Issues with Announce message and handling Needs PR
Projects
Development

No branches or pull requests

5 participants