PUSH, RST and spdy/3.1

[Fedor Indutny]

Hello devs!

While debugging node-spdy issue: https://github.com/indutny/node-spdy/issues/158

I have found that the problem seems to be more of the protocol level, rather than the implementation one. Could you please check this scenario and tell me if my assumption holds?

1. Client sends SYN_STREAM

2. Server sends SYN_REPLY

3. Server sends SYN_STREAM

4. Client sends RST for that PUSHed stream

4. Server sends a lot of DATA frames, up until session send window limit is reached

5. Server receives RST

6. Connection is stalled, because session window appears to be negative on server

The protocol description seems to be very loose about this corner case, or haven't I just found it there?

Please let me know, if I'm doing anything wrong.

Cheers,

Fedor. 

[Roberto Peon]

The client would need to account for those frames in connection-level flow-control, even for a stream it believes it closed, which would prompt it to send a WINDOW_UPDATE.

data-frames received alway affect the connection-level flow control.

-=R

--

---
You received this message because you are subscribed to the Google Groups "spdy-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to spdy-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Fedor Indutny]

Thank you for a prompt reply, I'll look into node-spdy more carefully!

[William Chan (陈智昌)]

https://code.google.com/p/chromium/codesearch#chromium/src/net/spdy/spdy_session.cc&l=1941 shows where Chromium will decrease the connection window for the DATA frame. As can be seen, this is done *before* the stream is looked up in our map of active streams, so even if the stream isn't present, we account it properly for the window. Cheers.

[Fedor Indutny]

William,

What about https://code.google.com/p/chromium/codesearch#chromium/src/net/spdy/spdy_session.cc&l=1942 and https://code.google.com/p/chromium/codesearch#chromium/src/net/spdy/spdy_session.cc&l=2956 ?

It appears to me that I have 11 bytes in `Unacked received data`...

Attaching a log from net-internals.

Cheers,

Fedor.

[Fedor Indutny]

Yeah, it seems to be exactly the case that I am hitting:

SPDY_STREAM_UPDATE_RECV_WINDOW with positive delta is reached

and no window update is sent, because there is not "much" acked bytes.

First of all, it seems that logging is a bit incorrect here as it accounts those

bytes as a session window delta, not as a stream delta. Also, why is it taking

it in account after all?

Cheers,

Fedor.

[Fedor Indutny]

Bump.

[William Chan (陈智昌)]

I can't tell for sure what's going on because the net log is incomplete. It's just the text, not the full JSON.

It looks to me at a glance that the connection window is shrinking to 0, being mostly utilized by the push stream. But since we have no request that is "claiming" the push stream, we will never "consume" the bytes, so the flow control window never reopens. This seems working as intended. Just like with BSD TCP sockets, if you stop calling read(), the TCP receive window will eventually shrink to 0 and the sender will be stalled. Similarly, if no application code on top of Chromium's network stack is asking for the pushed stream, it'll just stall.

Open questions:

* Is the push stream not getting claimed? I need to examine the document load to see why it's not asking for the URL that's getting pushed.

* Pat previously suggested different SETTINGS for push and pull streams so they could default to different window sizes. It may be the case that in absence of that, Chromium should be manually setting the sizes. It's unfortunate here that a push stream can use up the entire connection window.

Cheers.

[Fedor Indutny]

Hello William,

Thanks for a response! I think this is a bit incorrect, if the client do not want to receive any push stream data - it should either CANCEL them or regulate it on the stream window level.

Anyway, I have started a server for you take a look: https://indutny.com:4443/ . It gets into that stalled state after one page load, if you will refresh the page - everything just hangs.

Cheers,

Fedor.

[William Chan (陈智昌)]

On May 19, 2014 9:09 AM, "Fedor Indutny" <fe...@indutny.com> wrote:
>
> Hello William,
>
> Thanks for a response! I think this is a bit incorrect, if the client do not want to receive any push stream data - it should either CANCEL them or regulate it on the stream window level.

Can you clarify what specifically you think is incorrect? We diagnosis of what happened or my open questions? I don't fully understand.

[Fedor Indutny]

Ah, I meant that this behavior is incorrect. Sorry for bad wording.

The browser should extend session window when doing new requests, even if there is unconsumed PUSH data.

[William Chan (陈智昌)]

Thanks. While it's possible that we could partially do that, to completely ignore the connection window for unconsumed push data would open us up to memory consumption issues. I do think we might want to lower the allowed stream window for push streams.

I'm on my phone at a coffee shop. I'll debug this when I get out to a real computer :)

[Fedor Indutny]

Haha, ok :) Thank you very much!

[Johnny Graettinger]

Hi Fedor,

 

Thanks for a response! I think this is a bit incorrect, if the client do not want to receive any push stream data - it should either CANCEL them or regulate it on the stream window level.

Yep! This is something I'd like to tackle for Chromium in the near future. But it's not there yet.

Anyway, I have started a server for you take a look: https://indutny.com:4443/ . It gets into that stalled state after one page load, if you will refresh the page - everything just hangs.

As Will said, server pushes currently reside with the session they were pushed over, and pushed data can only be 'consumed' by a matching client request which claims it. If no matching client requests is forthcoming (which is the case in your test server), the data is never consumed, and corresponding window updates for the session and stream aren't sent. Therefor push is only useful in Chromium right now if there's good reason to expect a matching request will claim the stream, and as things stand today your test case is working as intended.

The future intent is to treat pushes as cache update operations: to allow the cache to consume the streams and drive window updates, and to make decisions about whether push streams are useful (or are not, and should be canceled).

cheers,

-johnny

 

 

[William Chan (陈智昌)]

Yeah, I basically agree with Johnny here. Fedor, do you agree? Or do you think this test case should be working? If so, can you outline what you think Chromium should be doing here?

To be clear, now that I've gotten to a computer and loaded it up myself, what I see is there's a plain text document that just says "hello world". And the push stream is presumably associated to that document, even though that document will never issue a request for that push stream URL, so it'll never "claim" it. I think this is a server bug.

That said, I think the permanently hung connection is a robustness issue. Users have certain expectations around the browser 'reload' button. In many ways, 'reload' is the new ctrl-alt-del. So if 'reload' doesn't fix the issue, that sucks. It might be the case that the browser needs to remember the mapping from the document to the SPDY stream id, so it can reset all associated streams for that stream id. Chromium currently has no good way of doing this because of our layering boundaries (our network stack is fundamentally structured with requests/responses, with no concept of 'push', which is shimmed in as a caching abstraction internal to the network stack).

I think you've pointed out something fairly interesting to push and browsers and 'leaked' buffer usage. I'd be interesting to hear how other client implementors handle this case. And this might be something that is worth raising in ietf-http-wg@.

[Fedor Indutny]

William,

I do agree up with you guys, but up to some extent.

Possible solution to this "stall" could be transforming the list of unacked PUSHes into a LRU cache, evicting the stuff that wasn't used if the client wants to get more data from server (i.e. `reload` button in browsers). However, associating stream id with a browser's page should work just fine as well, but it does seem to be less fundamental fix to me.

I could not tell you anything about other clients, but node-spdy provides pretty raw access to the PUSH requests, because it isn't parsing any incoming data and assumes that the user will figure out, which data is needed and which is not.

Speaking of companies, the Voxer is using PUSH streams for completely different purpose: we are using it for delivery of events and some realtime data. I think that while this use case is rather unusual, it still has its own merits to be present and supported.

Let me know if I could help you with making it work better.

Cheers,

Fedor.

[William Chan (陈智昌)]

On Mon, May 19, 2014 at 10:10 AM, Fedor Indutny <fe...@indutny.com> wrote:

William,

I do agree up with you guys, but up to some extent.

Possible solution to this "stall" could be transforming the list of unacked PUSHes into a LRU cache, evicting the stuff that wasn't used if the client wants to get more data from server (i.e. `reload` button in browsers). However, associating stream id with a browser's page should work just fine as well, but it does seem to be less fundamental fix to me.

This indeed is another potential fix. It's a bit coarse-grained in comparison to a protocol based solution of separating pull and push streams into separate windows. And I think we probably still want to lower our windows for push streams in comparison to pull streams. But yeah, the LRU cache has some potential. I'd need to think further if there are any situations where it'd perform poorly though. I'd be interested in hearing this discussed with a larger audience (ietf-http-wg@). 

 

I could not tell you anything about other clients, but node-spdy provides pretty raw access to the PUSH requests, because it isn't parsing any incoming data and assumes that the user will figure out, which data is needed and which is not.

Speaking of companies, the Voxer is using PUSH streams for completely different purpose: we are using it for delivery of events and some realtime data. I think that while this use case is rather unusual, it still has its own merits to be present and supported.

I'd love to hear more about this use case. Just to be clear, this is not supposed to interact with a browser, right? Is there any need for this push stream to be associated to another stream, or is it just a server-initiated stream without a PUSH_PROMISE?

Let me know if I could help you with making it work better.

Keep writing test cases and telling us where our push implementation sucks. We know it sucks, but we necessarily don't know all the way it sucks :)

[Fedor Indutny]

William,

I don't mind writing to ietf-http-wg@ if that's what you want me to do, but it may be much better if you would do it :) You have much more arguments than I do.

Regarding our use case, I couldn't really tell you how exactly it is interacting with associating streams, since I'm just a guy behind the protocol implementation itself. I'm quite sure that it could work without associated stream id as well, but it may make some thing more complicated. It isn't intended to work with any browser, just a mobile clients.

Cheers,

Fedor.

[William Chan (陈智昌)]

I need to run off for some appointments, but I'll email ietf-http-wg@ later today and cc you. Cheers.

[Fedor Indutny]

Thank you!

[William Chan (陈智昌)]

I actually missed Johnny's last statement about cache update operations. If the cache is always consuming the streams, then yes, we don't have the unclaimed push streams sucking up connection window issues. And the cache already has an eviction policy (ours is similar to LRU, but not exactly), so that probably solves the issue.

I'm still going to write ietf-http-wg@ to notify them about this, but I think for smart implementations, this will be a non-issue. Chromium's impl is not smart yet :)

[Fedor Indutny]

Thank you!