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

Octets that are sent twice #1353

Merged
merged 4 commits into from May 15, 2018
Merged

Octets that are sent twice #1353

merged 4 commits into from May 15, 2018

Conversation

martinthomson
Copy link
Member

This is obvious, but never stated. We discussed checking that they stay constant and agreed that requiring a check would be crazy. However, we never wrote anything down. I think that a MAY is fine.

This is obvious, but never stated.  We discussed checking that they stay constant and agreed that requiring a check would be crazy.  However, we never wrote anything down.  I think that a MAY is fine.
@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels May 14, 2018
the receiver's flow control limits.
MUST be less than 2^62.

A receiver MUST ensure that received stream data is delivered to the application
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Patrick wanted to allow out of order delivery, and I thought the agreement was along the lines of "A receiver MUST provide the received stream data to the application as an ordered byte-stream". The text you wrote implies it's in-order is the only possible option.

Copy link
Member Author

Choose a reason for hiding this comment

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

That text isn't changed. It's what we litigated (at some length), but moved into a new paragraph, because the old one was getting unwieldy.

You are right, though. It seems to imply that it has to deliver in order, when the intent was that it be capable of doing so.

@dtikhonov
Copy link
Member

From the proposed change:

The value for a given octet MUST NOT change if it is sent multiple times...

Why require that it not change? While inappropriate for HQ, mutable content (perhaps combined with out-of-order delivery) may make for some interesting protocol options.

@MikeBishop
Copy link
Contributor

Because it's then unpredictable what version (or combination of versions) will be received by the application. We'd need more versioning mechanisms if we wanted to employ that properly.

While I agree that the text around ordering reads as stricter than the agreed-upon intent, it's not actually modified by this PR. The change introduced by this PR is fine; I don't feel strongly about fixing the in-order text while we're here versus creating a separate PR.

@dtikhonov
Copy link
Member

Because it's then unpredictable what version (or combination of versions) will be received by the application. We'd need more versioning mechanisms if we wanted to employ that properly.

An application might be OK with it. What if the content is versioned purely as a function of time?

How about this scenario in HQ: serving a static file. An implementation may simply read(2) the file contents into STREAM frames as packets are sent out, including when lost frames are resent. The requirement that octets not be modified forces either keeping unacknowledged STREAM frames in memory or locking the file somehow.

@MikeBishop
Copy link
Contributor

In that scenario, consider that bytes 0-800 and 1000-1200 are delivered on the first try, then the file changes, and bytes 800-1000 need to be resent. If the file has changed in the meantime and the server is just reading from disk, you get a window into the new file while still having the outer bytes taken from the old file.

And yes, to avoid that, the server needs to hold read access to the file until it's done reading for good, or retain the content that it transferred, or at the least listen for filesystem change notifications.

I can see value in a solution (probably at the application layer) to abort a response and indicate that the resource has changed. But randomly intermixing content from the old and new versions without indication of which source it came from, which would be the output of the current structure, seems like an invitation to invalid state.

@mikkelfj
Copy link
Contributor

The wonderful mmap api does not guarantee MAP_PRIVATE view remains private on Linux if someone else writes to the file.

I think the requirement of unchanged data is the only sane approach at protocol level, but it can be viewed as a system requirement not the sole responsibility of the QUIC engine because that could range from very expensive to near impossible. Hence, if some other process with adequate permissions overwrite a mmap'ed file being transmitted, that process just violated the QUIC spec.

But don't imply an absolute requirement to do so.

It seems like an opportune moment to point out that no provision is made for partial reliability either.
@martinthomson
Copy link
Member Author

@ianswett, @janaiyengar, @mcmanus, can you check that I'm accurate in my updated statement here. I think that avoiding MUST is sensible here.

@martinthomson
Copy link
Member Author

@dtikhonov, if you want to change the value of an octet, I think that requires some sort of negotiation. In other words, that's a new feature.


QUIC makes no specific allowances for partial reliability or delivery of stream
data out of order. Endpoints are expected to deliver stream data to an
application as an ordered byte-stream. Delivering an order byte-stream requires
Copy link
Contributor

Choose a reason for hiding this comment

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

in-order byte stream?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or ordered?

@dtikhonov
Copy link
Member

@MikeBishop, @martinthomson -- I am not advocating for this, just want to get to the rationale behind this requirement.

@martinthomson
Copy link
Member Author

It's been an unstated assumption. We're just documenting it, that's all.

@janaiyengar janaiyengar merged commit 04eb8c6 into master May 15, 2018
@martinthomson martinthomson deleted the overlapping-data branch May 15, 2018 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants