Subject: [quicwg/base-drafts] PUSH_PROMISE ordering guarantee (#557)

From: Martin Thomson <[email-masked]>
Date: Wed, 24 May 2017 21:29:18 -0700

HTTP/2 has a pretty strong recommendation to have PUSH_PROMISE precede any text that mentions it.  Failing to do so could mean that a client requests a resource that was pushed, which is wasteful.

Splitting the headers and data into two streams means that there is no way to order the PUSH_PROMISE relative to the part of the body that mentions the resource.

I'm sure that @MikeBishop mentioned this once before, but I couldn't find any issues for it.  Returning to a model where data and headers share a stream would fix this.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/557

---

From: Mike Bishop <[email-masked]>
Date: Thu, 25 May 2017 08:02:38 -0700

Good point.  Don't recall an issue on this specifically -- maybe it came up on the issue about whether to recombine them?  Moot point until #176 is sorted out, though.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/557#issuecomment-304033067

---

From: krasic <[email-masked]>
Date: Thu, 25 May 2017 16:03:57 -0700

GCUIC didn't have headers and data in the same stream, it just had all headers on stream 3.  It enforced that a application reader receive header bytes before body bytes, and honored the recommendation wrt PUSH_PROMISE ordering.   I don't think that should change under two streams per HTTP request.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/557#issuecomment-304147562

---

From: Martin Thomson <[email-masked]>
Date: Thu, 25 May 2017 17:57:15 -0700

@krasic, GQUIC has exactly the same issue.  The body of a response can mention a resource and appear before a PUSH_PROMISE.

Unless, by this:

>  It enforced that a application reader receive header bytes before body bytes

...you mean that the stack was required to withhold stream data based until stream 3 explicitly released it.  Yes, we could do the same with split headers and body, but we can't do that generically, you have to have an explicit marker in the headers stream that says "you may read the body now".  That's equivalent to just having data on the same stream as headers, with a bunch of extra complexity stacked on.

@MikeBishop, #175 :)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/557#issuecomment-304162926

---

From: Mike Bishop <[email-masked]>
Date: Fri, 26 May 2017 09:14:34 -0700

For a simple push application where the server generates `HEADERS, PUSH_PROMISE, DATA*`, that kind of works -- if the client knows to keep looking for PUSH_PROMISE frames as well.  That effectively requires PUSH_PROMISE to be part of the header block.

For a more complex push application where the server generates `HEADERS (PUSH_PROMISE? DATA*)*`, there's no way for a client to know when it should return to pick up another PUSH_PROMISE.  In the best case, it's just processing data as it arrives, and always treats bytes on the headers stream as higher-priority than bytes on the data stream.  However, it's then subject to reordering -- or even partial arrival of a PUSH_PROMISE frame, given that QUIC doesn't know app-layer frame boundaries.

My unidirectional2 branch is just as vulnerable here, because it still allocates separate streams for each and keeps PUSH_PROMISE on the parent request's headers stream.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/557#issuecomment-304324143

---

From: krasic <[email-masked]>
Date: Fri, 26 May 2017 10:54:48 -0700

@martinthomson yes, I did mean that the stack withholds stream data until the headers have been consumed.  

@MikeBishop Thanks again for a great explanation.  I didn't think about promises interleaved with data after the initial headers.   I agree with your assessment.   FWIW, I don't think either GQUIC nor Google's server-side HTTP/2 actually support generating promises post initial headers, so in that sense they aren't fully HTTP/2 spec compliant.   I can envision use cases for streaming pushes, but not without a bunch of other stuff fixed (like cached digests etc.).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/557#issuecomment-304348399

---

From: Mike Bishop <[email-masked]>
Date: Fri, 26 May 2017 11:46:50 -0700

The primary case for us (as a server app platform) was a server generating and streaming back partial content as it continues to process.  The HTTP/2 requirement is that the PUSH_PROMISE SHOULD come before the DATA or HEADERS frame which contains the reference, which isn't a hard requirement that PUSH_PROMISE precede all the data.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/557#issuecomment-304360100

---

From: krasic <[email-masked]>
Date: Fri, 26 May 2017 12:30:49 -0700

I *think* by far the most common use of push in the wild is where the pull stream contains rel=preload headers (or similar) in its response headers, and the push promises precede said response headers.   At least I know it is status quo for us.

I'd be curious to know about use cases with different patterns.  I don't have a clear mapping of how your "server generating and streaming partial content as it continues to process" actually looks wrt server push.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/557#issuecomment-304369498