Define datagram backpressure control attributes

[yutakahirano]

Define the following attributes on DatagramDuplexStream.

• incomingMaxAge
• outgoingMaxAge
• incomingHighWaterMark
• outgoingHighWaterMark

Fixes #251, #221, #152

---

Preview | Diff

[yutakahirano]

I made set(Incoming|Outgoing)HighWaterMark methods rather than members in WebTransportOptions because it'd be useful to set them multiple times, especially when the network condition changes.

[yutakahirano]

@jan-ivar @ricea can you take a look? Names may change based on the discussion at #251.

[jan-ivar]

These look like simple setters, and should therefore be read-write attributes instead of methods, I claim. E.g. using latest naming from #251 (comment):

wt.datagrams.incomingMaxAge = 300; // ms
wt.datagrams.outgoingMaxAge = 800; // ms

I'm also wondering if we should take a cue from recent changes to bidirectional streams, and subclass readable and writable stream here, so we can push the attributes down:

wt.datagrams.readable.maxAge = 300; // ms
wt.datagrams.writable.maxAge = 800; // ms

[yutakahirano]

One concern is it can return null (when not set/the browser default behavior). Are you fine with the following IDL?

attribute double? incomingMaxAge;
attribute double? outgoingMaxAge;

I share the sentiment about subclassing with @ricea and I'd like to avoid that without a clear benefit. I'm not sure this is enough.

[jan-ivar]

Meeting:

• Might be better to expose the implementation-defined default than return null?

[yutakahirano]

Please note that the implementation-default value can be chosen for each receiveDatagram call when [[IncomingDatagramsExpirationDuration]]. So exposing a fixed value reduces the flexisibility. Ditto for the outgoing direction.

[jan-ivar]

When it comes to highwaterMark, there's potential confusion with the same-named concept in the streams spec:

const stream = new WritableStream(
  { ... },
  new CountQueuingStrategy({ highWaterMark: 5 })
);

But I'm not 100% sure it's the same thing. That API I think is generally understood to be the buffer in the ReadableStream/WritableStream itself, distinct from its underlying source, whereas in our case, we have a setup at least on the writable side where we pre-fill the underlying sink's write-queue (for throughput and threading purposes), and what we really mean to control here is the depth of that write queue. But maybe this is a detail and that from the viewpoint of a producer (or consumer in the readable case) this is close enough to the the same thing?

[yutakahirano]

They represent the same concept, the length of the queue in a stream. We have them in the underlying source and sink because removing stale chunks is not supported in general streams. Given that I think it's good to keep the same name. Web developers cannot control the buffer lengths in the streams, but instead they can control these values.

[yutakahirano]

I replaced the methods with attributes. PTAL again.

[ricea]

lgtm

[yutakahirano]

@jan-ivar PTAL again.

I'm aware of #267 (comment) but I'd like to land this change soon. We can file another issue and discuss it there. If you don't like the possible breaking change, I can remove the getter part of incomingMaxAge and outgoingMaxAge.

[yutakahirano]

Removing the "Discuss at next meeting" label as it is for the previous one.

[yutakahirano]

I'm going to land this change on Jun 16 (JST) if unless further comments are made.