SlyMarbo / spdy

[deprecated] A full-featured SPDY library for the Go language.
BSD 2-Clause "Simplified" License
118 stars 13 forks source link

Concurrent streams limit #71

Closed chendo closed 9 years ago

chendo commented 9 years ago

When using the same http.Client and doing concurrent requests to a single domain, I eventually start seeing "Max concurrent streams limit exceeded." errors.

There doesn't appear to be an option to adjust this limit. Do I need to maintain multiple http.Clients?

SlyMarbo commented 9 years ago

The limit is imposed by the peer, so this isn't something you can configure. As a quick workaround, try delaying requests for a while after getting that error. A long-term fix will take some more thought. On 10 Oct 2014 15:39, "Jack Chen" notifications@github.com wrote:

When using the same http.Client and doing concurrent requests to a single domain, I eventually start seeing "Max concurrent streams limit exceeded." errors.

There doesn't appear to be an option to adjust this limit. Do I need to maintain multiple http.Clients?

— Reply to this email directly or view it on GitHub https://github.com/SlyMarbo/spdy/issues/71.

chendo commented 9 years ago

Delaying requests won't work for my case cause I'm doing proxying on behalf of other clients... I'll try creating another connection when I receive that error

rnapier commented 9 years ago

What's unclear from our experiments is whether the library is correctly clearing streams with FIN or removing FIN streams from the active list. We're seeing "concurrent streams exceeded" on a system that should only have a short-lived heartbeat.

SlyMarbo commented 9 years ago

Sorry guys, entirely my fault. Not sure how, but I'd forgotten to decrement the count when the streams ended, as you suspected.

rnapier commented 9 years ago

We're still seeing this problem. ResponseStream.Close() is never called at the end of the responses, so the stream is never released. My assumption is that the response stream should be closed automatically at the end of the response handler.

We're also assuming that it's normal to have a separate stream for every HTTP request since that seems to be what the library is doing. While that may not be necessary, it is convenient for us given how we use this protocol (like chendo, we're proxying arbitrary requests).

SlyMarbo commented 9 years ago

f2d520c463ef83b3728c24c63195bf3621324057 should have fixed that. Apologies again.

rnapier commented 9 years ago

f2d520c seems to fix it. Open streams are hovering at 0, exactly as expected. Thanks.