Clarify subscribe / annouce interaction #525

fluffy · 2024-09-13T01:05:42Z

Fixes #362 #267 #227 #225 #79

afrind

Individual Review

This is really helpful clarification as I didn't necessarily think it had to work this way.

You have a number of random typos etc but just giving high-level feedback.

draft-ietf-moq-transport.md

afrind · 2024-09-13T17:07:12Z

draft-ietf-moq-transport.md

+A Relay can receive announcements from multiple publishers for the same
+tracknamespace and it MUST respond with appropriate response to each of the
+publishers, in the same way as it would responds when processing ANNOUNCE
+from a single publisher for a given tracknamespace.


Can relays limit the number of publishers for a given namespace? It seems like for DoS reasons they can. Maybe we should specify a minimum: "Relays MUST support at least X publishers announcing the same namespace"

well ... I suspect this is going to fall in policy for relays region. Lets say you have 10 million viewers of a Taylor Swift livestream that can all publish text comments to various chat tracks. You OK with the number of max publishers on the same namespace being in the millions ? It seems that different relays will want very different limits here.

I was hoping SUBSCRIBE_NAMESPACE would solve some of that problem -- all 10M viewers have their own namespace tswift/chat/unique_id. We may need to indicate here what limits are likely to exist, because it influences how higher level protocols will use moqt in terms of namespace and track naming.

Agreed the max will be different, but is there a min that we need to specify to get basic interop, eg: for make-before-break, min=2.

draft-ietf-moq-transport.md

fluffy · 2024-09-14T15:32:59Z

Individual Review

This is really helpful clarification as I didn't necessarily think it had to work this way.

You have a number of random typos etc but just giving high-level feedback.

Many thanks on review and any help of fixing all the small stuff (point me too it, or PR on the PR, anything) much appreciated.

afrind

Individual Comments

draft-ietf-moq-transport.md

afrind · 2024-09-14T20:27:46Z

draft-ietf-moq-transport.md

+A Relay can receive announcements from multiple publishers for the same
+tracknamespace and it MUST respond with appropriate response to each of the
+publishers, in the same way as it would responds when processing ANNOUNCE
+from a single publisher for a given tracknamespace.


I was hoping SUBSCRIBE_NAMESPACE would solve some of that problem -- all 10M viewers have their own namespace tswift/chat/unique_id. We may need to indicate here what limits are likely to exist, because it influences how higher level protocols will use moqt in terms of namespace and track naming.

Agreed the max will be different, but is there a min that we need to specify to get basic interop, eg: for make-before-break, min=2.

draft-ietf-moq-transport.md

ianswett

This looks great, and clarifies a lot about how one might use ANNOUNCE. I have one normative comment, which is that I think some of the MUSTs should be a SHOULD.

draft-ietf-moq-transport.md

suhasHere

This clarifies lot of ambiguities on implementing make before break use-cases.
LGTM

Co-authored-by: ianswett <ianswett@users.noreply.github.com>

Co-authored-by: Martin Duke <martin.h.duke@gmail.com>

Co-authored-by: ianswett <ianswett@users.noreply.github.com>

fluffy · 2024-09-18T18:45:35Z

@martinduke - thanks for review. I think I fixed all the stuff you raised. Can you take another look

@ianswett - handing the baton to you on this one. Thanks you

martinduke · 2024-09-18T19:31:44Z

draft-ietf-moq-transport.md

@@ -710,7 +710,18 @@ subscribers for each track. Each new OBJECT belonging to the
 track within the subscription range is forwarded to each active
 subscriber, dependent on the congestion response. A subscription
 remains active until the publisher of the track terminates the
-track with a SUBSCRIBE_DONE (see {{message-subscribe-done}}).
+subscription with a SUBSCRIBE_DONE (see {{message-subscribe-done}})
+or all the downstream subscriptions are ended. A


The "publisher" is a term local to the session, unless you mean "original publisher." So a subscription ends when the publisher sends SUBSCRIBE_DONE or when the subscriber sends UNSUBSCRIBE. Downstream subscriptions don't enter into it.

I think this section is describing how a relay handles downstream subscriptions, so I agree with Martin. Those subscriptions end on DONE (track end, reached the end of what the subscribe was for), UNSUBSCRIBE, or also a session error (eg: timeout).

I don't think we need to mandate anything about when a relay terminates its upstream subscription. Even if there are no subscribers, maybe the relay wants to keep the track warm in case they come back, etc.

draft-ietf-moq-transport.md

martinduke · 2024-09-18T19:36:26Z

draft-ietf-moq-transport.md

+that would cause it to send an upstream subscription, for
+each publisher that has announced that namespace, the relay SHOULD send a
+SUBSCRIBE to that publisher unless it already has an active subscription
+to that publisher for the Full Track Name in the incoming SUBSCRIBE.


s/for the Full Track Name/for the objects in the specified range

draft-ietf-moq-transport.md

afrind · 2024-09-18T21:51:21Z

draft-ietf-moq-transport.md

@@ -710,7 +710,18 @@ subscribers for each track. Each new OBJECT belonging to the
 track within the subscription range is forwarded to each active
 subscriber, dependent on the congestion response. A subscription
 remains active until the publisher of the track terminates the
-track with a SUBSCRIBE_DONE (see {{message-subscribe-done}}).
+subscription with a SUBSCRIBE_DONE (see {{message-subscribe-done}})
+or all the downstream subscriptions are ended. A


I think this section is describing how a relay handles downstream subscriptions, so I agree with Martin. Those subscriptions end on DONE (track end, reached the end of what the subscribe was for), UNSUBSCRIBE, or also a session error (eg: timeout).

I don't think we need to mandate anything about when a relay terminates its upstream subscription. Even if there are no subscribers, maybe the relay wants to keep the track warm in case they come back, etc.

afrind · 2024-09-18T21:52:27Z

draft-ietf-moq-transport.md

+Implementations that update the cache need to be protect against cache
+poisoning.


SHOULD protect?

TBH, I'm not sure it matters if we say something normative, given we don't have proscribed behavior about how to prevent against cache poisoning?

draft-ietf-moq-transport.md

Co-authored-by: Martin Duke <martin.h.duke@gmail.com>

Co-authored-by: afrind <afrind@users.noreply.github.com>

Co-authored-by: Martin Duke <martin.h.duke@gmail.com>

draft-ietf-moq-transport.md

fluffy added 4 commits September 12, 2024 08:37

start changes for announce

ef4e97b

cleanup

b9eb195

add gracefull behavior

983221b

clean up

bae358d

fluffy requested a review from suhasHere September 13, 2024 01:05

fluffy self-assigned this Sep 13, 2024

fluffy added Announce Issues with Announce message and handling Subscribe Related to SUBSCRIBE message and subscription handling labels Sep 13, 2024

fluffy requested a review from ianswett September 13, 2024 13:00

afrind reviewed Sep 13, 2024

View reviewed changes

afrind reviewed Sep 14, 2024

View reviewed changes

ianswett added 6 commits September 16, 2024 11:50

Merge branch 'main' into anc1

9975e17

Editorial update to "Graceful Publisher Network Switchover"

bc2bbd5

Update draft-ietf-moq-transport.md

d566967

Update draft-ietf-moq-transport.md

f48aadd

Update draft-ietf-moq-transport.md

206a023

Update draft-ietf-moq-transport.md

155cc70

ianswett reviewed Sep 16, 2024

View reviewed changes

ianswett added 6 commits September 16, 2024 18:31

Update draft-ietf-moq-transport.md

edde941

Update draft-ietf-moq-transport.md

5964caa

Update draft-ietf-moq-transport.md

252c203

Update draft-ietf-moq-transport.md

e7b3cc5

Update draft-ietf-moq-transport.md

2dbe780

Merge branch 'main' into anc1

1e960a9

suhasHere approved these changes Sep 18, 2024

View reviewed changes

fluffy and others added 4 commits September 18, 2024 10:15

Update draft-ietf-moq-transport.md

bd7003c

Co-authored-by: ianswett <ianswett@users.noreply.github.com>

Update draft-ietf-moq-transport.md

4e3a0d4

Co-authored-by: ianswett <ianswett@users.noreply.github.com>

Update draft-ietf-moq-transport.md

837408d

Co-authored-by: ianswett <ianswett@users.noreply.github.com>

Update draft-ietf-moq-transport.md

365fb5a

Co-authored-by: ianswett <ianswett@users.noreply.github.com>

fluffy and others added 7 commits September 18, 2024 10:49

Update draft-ietf-moq-transport.md

9a33793

Co-authored-by: Martin Duke <martin.h.duke@gmail.com>

update handing of duplicats

e7612b9

Merge remote-tracking branch 'origin/anc1' into anc1

50e5abd

update to deal with all data cached case

a8ca0c1

clean up ending of subscribes in relay

4484de4

Update draft-ietf-moq-transport.md

a4303a2

Co-authored-by: ianswett <ianswett@users.noreply.github.com>

Update draft-ietf-moq-transport.md

d28baa5

Co-authored-by: ianswett <ianswett@users.noreply.github.com>

martinduke reviewed Sep 18, 2024

View reviewed changes

afrind reviewed Sep 18, 2024

View reviewed changes

ianswett and others added 4 commits September 18, 2024 18:18

Update draft-ietf-moq-transport.md

790c700

Co-authored-by: Martin Duke <martin.h.duke@gmail.com>

Update draft-ietf-moq-transport.md

263c6d1

Co-authored-by: afrind <afrind@users.noreply.github.com>

Update draft-ietf-moq-transport.md

5de4ee8

Co-authored-by: afrind <afrind@users.noreply.github.com>

Update draft-ietf-moq-transport.md

2c5397e

Co-authored-by: Martin Duke <martin.h.duke@gmail.com>

ianswett reviewed Sep 18, 2024

View reviewed changes

draft-ietf-moq-transport.md Outdated Show resolved Hide resolved

ianswett added 2 commits September 18, 2024 18:23

Update draft-ietf-moq-transport.md

08b9e0e

Minor update

8c8875a

ianswett approved these changes Sep 18, 2024

View reviewed changes

ianswett added 7 commits September 18, 2024 18:30

Merge branch 'main' into anc1

6e84e0a

Merge branch 'main' into anc1

38e0456

Update draft-ietf-moq-transport.md

2cc1034

Update draft-ietf-moq-transport.md

5be3af3

Rewrite based on Alan's comment

9708a36

Update draft-ietf-moq-transport.md

f15edfa

Merge branch 'main' into anc1

24c43db

ianswett merged commit b37f0b1 into main Sep 20, 2024
2 checks passed

This was referenced Sep 20, 2024

Broadcast publisher reconnect support #79

Closed

Clarify conflicting announces across Producers #225

Closed

Announces with same name in reconnect #227

Closed

Graceful session migration #267

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify subscribe / annouce interaction #525

Clarify subscribe / annouce interaction #525

fluffy commented Sep 13, 2024

afrind left a comment

afrind Sep 13, 2024

fluffy Sep 14, 2024

afrind Sep 14, 2024

fluffy commented Sep 14, 2024

afrind left a comment

afrind Sep 14, 2024

ianswett left a comment

suhasHere left a comment

fluffy commented Sep 18, 2024

martinduke Sep 18, 2024

afrind Sep 18, 2024

martinduke Sep 18, 2024

afrind Sep 18, 2024

afrind Sep 18, 2024

ianswett Sep 19, 2024

		Implementations that update the cache need to be protect against cache
		poisoning.

Clarify subscribe / annouce interaction #525

Clarify subscribe / annouce interaction #525

Conversation

fluffy commented Sep 13, 2024

afrind left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fluffy commented Sep 14, 2024

afrind left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianswett left a comment

Choose a reason for hiding this comment

suhasHere left a comment

Choose a reason for hiding this comment

fluffy commented Sep 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment