Re: Push ID - Merge Imminent

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> wrote:
>>>>
>>>>> Bah, QUIC too.  😊
>>>>>
>>>>>
>>>>>
>>>>> *From:* Mike Bishop [mailto:Michael.Bishop@microsoft.com]
>>>>> *Sent:* Thursday, August 3, 2017 10:36 AM
>>>>> *To:* 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
>>
>
>