Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Curl connect timeout #841

Merged
merged 2 commits into from
May 15, 2015
Merged

Conversation

rmruano
Copy link
Contributor

@rmruano rmruano commented May 14, 2015

CURLOPT_CONNECTTIMEOUT added to the HTTP transport curl options to allow connections to unresponsive nodes to fail as soon as possible. The timeout can be customized with the connectTimeout parameter (default of 5 seconds).

…ort for a custom connection timeout through a connectTimeout parameter.
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c29490f on miniplay:connection-connect-timeout into * on ruflin:master*.

* Number of seconds after a connection timeout occurs for every request
* Use a small value if you need a fast response in case of dead servers.
*/
const CONNECT_TIMEOUT = 5;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means setting the timeout for all clients that don't implement it to 5 seconds. Isn't the default (without setting it) 300? So it would make sense to keep the behaviour the same as before to set it to 300? Or what is the default currently used?

@rmruano
Copy link
Contributor Author

rmruano commented May 14, 2015

Indeed, I've been too aggressive with the default. The idea behind of this is to prevent stacking thousands of connections in high demanded sites in case of cluster outage or unresponsiveness, firewall rules or simple issues with the connection. Recently we had a complete outage because of the default behavior which according to my research is 300, the cluster went unreachable and all our app servers kept waiting forever instead of falling back to our error page until they weren't able to keep up accepting requests. Please notice that this timeout only applies during the connection phase: If your client is unable to open the socket in 5 seconds, then something very bad is going on.

Anyway, we've got 3 options:

  • Increasing the default to 300 seconds to match the standard timeout.
  • Increasing the default to a less aggressive value value (PHP default_socket_timeout property is 60 sec)
  • Setting the default value to null and ignore the parameter if it's not been explicitly set.

What do you think?

@ruflin
Copy link
Owner

ruflin commented May 14, 2015

@rmruano I think 5 seconds is totally reasonable. But we should not change the default behaviour. So my favorite options is option number 3. Set it to null so in case the default value is changed by curl, it will also change.

@rmruano rmruano force-pushed the connection-connect-timeout branch from 3a538b1 to 04e53a8 Compare May 15, 2015 05:10
@rmruano
Copy link
Contributor Author

rmruano commented May 15, 2015

Done. I've opted to set it to 0 instead of null since it's how curl treats the option

@ruflin ruflin merged commit 04e53a8 into ruflin:master May 15, 2015
@ruflin
Copy link
Owner

ruflin commented May 15, 2015

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants