search

spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

Why default maxStreams to 100? #177

Closed jeffreydwalter closed 9 years ago

jeffreydwalter commented 9 years ago

In the app I'm writing I'm seeing the spdy client intermittently send: RST_STREAM - status: 3, message: "Received rst: 3".

After looking at the node-spdy library and reading the spdy protocol, it seems odd to me that node-spdy defaults the maxStreams to 100, when that is the minimum recommendation. The spdy protocol doesn't impose a limit by default.

See the protocol: http://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft2#TOC-SETTINGS 4 - SETTINGS_MAX_CONCURRENT_STREAMS allows the sender to inform the remote endpoint the maximum number of concurrent streams which it will allow. By default there is no limit. For implementors it is recommended that this value be no smaller than 100.

Indeed, setting maxStreams = Infinity; makes the RST_STREAM error go away in my case.

Why not default to Infinity like the protocol?

indutny commented 9 years ago

There was a limit in spdy/2, AFAIK. I guess we could safely remove it then! Do you want to submit a PR that will fix it?

jeffreydwalter commented 9 years ago

Sure! Coming up!

jeffreydwalter commented 9 years ago

Actually, that link I sent you was from the v2 draft. It says the same thing in the v1, v2, and v3 drafts, so it should be safe to default to Infinity for the general case. I will make that the case, unless you disagree.

v1 - http://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft1#TOC-Hello-message v2 - http://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft2#TOC-SETTINGS v3 - http://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft3#TOC-2.6.4-SETTINGS

indutny commented 9 years ago

Oh goodness! Remove it then! :) But please let the users provide it as an API argument.

jeffreydwalter commented 9 years ago

Done. https://github.com/indutny/node-spdy/pull/178

Just changed the default from 100 to Infinity.