Re: Are HTTP/2 state changes atomic with respect to SETTINGS_MAX_CONCURRENT_STREAMS?

On Mon, Feb 11, 2019 at 07:04:34PM +1100, Martin Thomson wrote:
> On Mon, Feb 11, 2019, at 17:55, Willy Tarreau wrote:
> > I think even for H2 we could address this, because I find it extremely
> > unlikely that some real workloads depend on the unbound aspect of this.
> > The change could be as simple as sayng that the stream is counted when
> > it is created (PUSH_PROMISE) just the way it works in the other direction.
> > And it most likely matches what implementations can naturally do. I would
> > not be surprised if some of them continue to count the allocated streams
> > towards the limit anyway.
> 
> That would be a breaking change, so you would want to signal it.

Unless everyone agrees it's a bug :-)

> And I'm not
> sure that it would place any meaningful limit on the number of pushes.

I'm not speaking about a limit on the number of pushes, but on the number
of allocated streams, which are immediately created in the reserved state
once the PUSH_PROMISE frame is seen. If I were to implement push in haproxy,
I would have no choice but to enforce this limit anyway because memory is
not an infinite resource.

The problem is that the period between PUSH_PROMISE and HEADERS consumes
memory that is not accounted for. I'd just want to be sure this period is
covered by the count. That basically means that if a client says it
supports up to 100 concurrent streams, there are no more than 100 consecutive
PP frames without a HEADERS or DATA frame carrying an ES flag in the middle.
Not only I don't see this as an unreasonable expectation, but I do find the
opposite unreasonable.

> You
> can still send PUSH_PROMISE and fulfill the promise without any feedback,
> meaning that as long as you can complete the entire cycle, you can send
> unbounded pushes if even a single push is allowed.

That's perfectly fine because then streams are allocated and released.

> The real fix is to add feedback into the loop and force the server to wait
> until the client allows it.  The MAX_PUSH_ID in h3 does that; a similar fix
> could be used here.

It addresses a different problem in my opinion (though probably a very valid
one) and fixes the unbounded memory issue as a byproduct.

Cheers,
Willy

Received on Monday, 11 February 2019 09:17:45 UTC