RE: Push ID - Merge Imminent

Agreed that the SETTINGS entry and the frame are the same 5-8 bytes consumed, and it would be simpler to have a single code path.  Since this whole thing is a departure from HTTP/2, I’m not personally upset about diverging a little further in the name of simpler code.  This isn’t a value that either party needs to know before sending traffic to behave properly, which is the case with QUIC’s limits of this type that have the dual parameter/frame structure.

From: Martin Thomson [mailto:martin.thomson@gmail.com]
Sent: Tuesday, August 8, 2017 5:45 PM
To: Jana Iyengar <jri@google.com>
Cc: Ian Swett <ianswett@google.com>; Kazuho Oku <kazuhooku@gmail.com>; QUIC WG <quic@ietf.org>; ietf-http-wg@w3.org; Mike Bishop <Michael.Bishop@microsoft.com>
Subject: Re: Push ID - Merge Imminent

Just to follow up on this, I've opened https://github.com/quicwg/base-drafts/pull/711<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fquicwg%2Fbase-drafts%2Fpull%2F711&data=02%7C01%7CMichael.Bishop%40microsoft.com%7Ce79ff894e53147e720b208d4debfe880%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636378363209427335&sdata=4Ln%2Fu8AIWLTpnKdEDysMC6D6LWhlec0j0ISFCjOWHuo%3D&reserved=0>

It defines a new frame type that sets the limit to Push ID.  In a fit of inspiration, I called it MAX_PUSH_ID.

I have repurposed SETTINGS_ENABLE_PUSH for setting the initial limit.  The newly renamed SETTINGS_MAX_PUSH_ID sets the initial MAX_PUSH_ID.

I want to discuss whether we might remove the setting.  Sending MAX_PUSH_ID immediately after SETTINGS doesn't add any more bytes and I'm not sure that there is any particular need to optimize for HoLB here.  I'm ambivalent, so I kept it, but it is easy to remove.


On 5 August 2017 at 10:55, Jana Iyengar <jri@google.com<mailto:jri@google.com>> wrote:
Martin,

I think the idea of sequential Push ID is a good one. You have to specify a MAX_PUSH_ID, without which the client won't know how far out to expect CANCEL_PUSHes can be... this also requires then a MAX_PUSH_ID_UPDATE message which can be used to move the MAX up.

One note:

PUSH_PROMISE is sent by the server, and CANCEL_PUSH may be sent by the server. The text currently says that CANCEL_PUSH on a non-existent Push ID results in error, which does not work with the possibility of reordering between PUSH_PROMISE and CANCEL_PUSH.

That's just a bug.

I don't think so. The spec allows it, and I can easily see it happen in practice.

- jana


Similarly, it's also possible for the PUSH_PROMISE to not have been received because the stream on which it was sent was reset... basically it's possible for a client to receive a CANCEL_PUSH without seeing the corresponding PUSH_PROMISE.

This is more interesting.  It creates state on the client:  it says that clients are required to ignore pushes that it *might* receive in the future.  That suggests a different fix, akin to what we use for other similar pieces of state:

1. make Push ID sequential (right now the only requirement is for connection-wide uniqueness)

I think that using a sequential ID will be a good idea not only for fixing this issue but that it might be possible to use the sequentiality to fix the first issue raised in the PR (i.e. client knowing "when the server will stop referencing a push ID in PUSH_PROMISE").

If there is a sequential ordering guarantee for Push ID, a client can, by looking at the Push ID, determine whether if it has already consumed the pushed request. If it has, it will immediately lookup its cache, and in case it fails to find a cached object, then issue a pull. If it has not consumed the pushed request, it should wait for the pushed response to arrive.



2. have the client advertise a MAX_PUSH_ID (in SETTINGS, and with a new frame type; the setting might {ab|re}use ENABLE_PUSH)

This would bound state for clients.  A client couldn't receive an arbitrary number of CANCEL_PUSH messages that are distributed over the 32-bit Push ID space, so tracking cancellations is easy.  Clients could use a simple bitvector to track which pushes are potentially interesting, clearing bits as pushes or cancellations arrive.

MAX_PUSH_ID would give clients a finer-grained lever to control push.

We could have CANCEL_PUSH be sent on the same streams as the PUSH_PROMISE, but that doesn't help with the case where the PUSH_PROMISE may not have been received due to a stream reset.

This makes me wonder if it makes sense to always send the PUSH_PROMISE on the control stream *in addition to* any stream that it is sent on. This would ensure that no matter what, the PUSH_PROMISE is always received, and additionally ensures that CANCEL_PUSH is received after a PUSH_PROMISE.

I don't think that is a good idea.  PUSH_PROMISE includes a headers block, which is pretty big, even with effective compression.  HoLB won't affect this stream, so size shouldn't be an issue, but still.

I did consider moving the header block to the control stream completely as part of addressing the issue with duplication that I mentioned earlier.  The basic problem there is that you don't guarantee that the client see the PUSH_PROMISE before it processes the response content.


- jana

On Thu, Aug 3, 2017 at 5:49 PM, Mike Bishop <Michael.Bishop@microsoft.com<mailto:Michael.Bishop@microsoft.com>> wrote:
Bah, QUIC too.  😊

From: Mike Bishop [mailto:Michael.Bishop@microsoft.com<mailto:Michael.Bishop@microsoft.com>]
Sent: Thursday, August 3, 2017 10:36 AM
To: ietf-http-wg@w3.org<mailto:ietf-http-wg@w3.org>
Subject: Push ID - Merge Imminent

This is Martin’s Push ID PR split off from unidirectional.  Given that it has already been picked over in those contexts and the feedback from that and in-person discussion was to bring this change in separately, I’m about ready to merge this.  Everything anyone has quibbled with has been editorial.  However, I don’t see reviews from many folks, so I want to make sure that everyone interested has had a chance to express an opinion.

https://github.com/quicwg/base-drafts/pull/701<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fquicwg%2Fbase-drafts%2Fpull%2F701&data=04%7C01%7CMichael.Bishop%40microsoft.com%7C8c68a743428c43fd9e9508d4da9692a7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636373787673436338%7CUnknown%7CVW5rbm93bnx7IlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiT3RoZXIifQ%3D%3D%7C-1&sdata=lF5Cr5Fe1RuY5BOvPgiy9iaGqHS27%2B7jeM1IOipSxu4%3D&reserved=0>

I’ll give it a few hours (until everyone’s had at least a little bit of a work day), then merge unless I hear otherwise.





--
Kazuho Oku

Received on Wednesday, 9 August 2017 16:15:49 UTC