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

Crisp definition of BLOCKING and friends #1889

Closed
wants to merge 1 commit into from

Conversation

martinthomson
Copy link
Member

Recent discussion around the use of BLOCKING frames highlighted some
inconsistency - and even disagreement - about what the purpose of the
frames are. If you read through all the text for the flow
control-related frames, it is clear that the frame carries the limit
at the time the frame is sent.

These frames always include the limit that is causing blocking at the
time that they are transmitted.

This enacts that change more consistently, especially for
STREAM_ID_BLOCKED, which was the most unclear.

The discussion on #1851 suggested that there was value in knowing what
the sender (or stream opener) wanted to get to. That is, the frames
would carry a higher value. If the limit was X and the sender wanted to
send octet X+Y (or open stream X+Y), they would convey that information
instead. The theory here is that the larger value (X+Y) would be sent,
allowing the receiver to make those resources available.

The problem with this alternative approach is that the value advertised
changes over time and it is difficult to connect the signal (a BLOCKED
frame), with the limit that was in force at the time.

I suspect that there is value in signaling the desired limits as well,
but that would require greater justification. It also entails a change
and I'm leery of feature creep at this stage.

Closes #1851, #1850.

Recent discussion around the use of BLOCKING frames highlighted some
inconsistency - and even disagreement - about what the purpose of the
frames are.  If you read through all the text for the flow
control-related frames, it is clear that the frame carries the *limit*
at the time the frame is sent.

> These frames always include the limit that is causing blocking at the
> time that they are transmitted.

This enacts that change more consistently, especially for
STREAM_ID_BLOCKED, which was the most unclear.

The discussion on #1851 suggested that there was value in knowing what
the sender (or stream opener) wanted to get to.  That is, the frames
would carry a higher value.  If the limit was X and the sender wanted to
send octet X+Y (or open stream X+Y), they would convey that information
instead.  The theory here is that the larger value (X+Y) would be sent,
allowing the receiver to make those resources available.

The problem with this alternative approach is that the value advertised
changes over time and it is difficult to connect the signal (a BLOCKED
frame), with the limit that was in force at the time.

I suspect that there is value in signaling the desired limits as well,
but that would require greater justification.  It also entails a change
and I'm leery of feature creep at this stage.

Closes #1851, #1850.
: A variable-length integer indicating the highest stream ID that the sender
was permitted to open.
: A variable-length integer indicating the stream ID limit at the time the
sender was unable to open new streams.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here.

I am afraid that the text might still imply the same thing; i.e. the maximum stream ID that the sender was permitted to open at the time the sender was unable to open the streams.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, this fails to address the original issue. I will amend the definition of MAX_STREAM_ID to address that concern then - and use your "exclusive maximum" approach.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's a tricky one, changing the definition of MAX_STREAM isn't that easy.

With stream IDs, if I receive MAX_STREAM_ID of N, then the test for whether something is allowed is id <= N. With stream octets, the test is offset < MAX_STREAM_DATA. That makes this harder.

So with the latter, saying that you can't send offset is clean and simple (wording notwithstanding). With streams though, you would be saying "I can only send N (but want to send N+4)", which is awkward, I agree.

This is because we count octets on streams (and in total), but we don't use counts in MAX_STREAM_ID.

Is changing the definition of MAX_STREAM_ID worth it?

Copy link
Member

@kazuho kazuho Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like an excellent observation.

I think I'd agree to changing the definition of MAX_STREAM_ID to send the number of streams rather than the stream ID. We might also want to rename the frame to MAX_STREAM_COUNT.

There are certain aspects that are not elegant in the way MAX_STREAM_ID is defined now:

  • we have two "maximum"s; one for unidirectional streams and the other for bidirectional streams. It sounds just strange.
  • we exchange counts using TP but we transmit maximum in the frame

These issues will be resolved by sending the counts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we changed it to MAX_STREAM_COUNT, we'd have to use one additional bit from the type to indicate unidirectional/bidirectional, right? To me, my favorite thing (if we made that change) would be that I no longer have to validate if the peer is trying to set my limit on them (i.e. ownership bit).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming that MAX_STREAM_COUNT is supposed to encode the number of streams since the beginning of the connection (and not the concurrently open streams, as we had in some earlier version of the draft), I think this would be a good change to make, since we'd use the same encoding for the transport parameters and the frame.
Not sure if MAX_STREAM_COUNT is the best name for this, maybe we can go with something analogous to the transport parameters (initial_max_{uni,bidi}_streams), and name it MAX_STREAMS frame?

: A variable-length integer indicating the connection-level offset at which
the blocking occurred.
: A variable-length integer indicating the connection flow control limit at the
time the frame is sent.
Copy link
Member

@kazuho kazuho Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually prefer the old text, assuming that we do not have a clarification on what "limit" means here. In other parts of the transport protocol we represent the limit using "inclusive maximum." But here, we need a limit expressed as "exclusive maxmimum."

IMO the original text is clearer in stating that the offset is "maximum offset permitted by MAX_DATA + 1", by saying that the transmitted value is "the offset at which the blocking occurred."

@martinthomson
Copy link
Member Author

Closing in favour of a bigger change.

@martinthomson martinthomson deleted the define-blocking-values branch October 26, 2018 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants