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

Ensure the ping port is timed out after a reasonable amount of time #277

Merged
merged 2 commits into from
Feb 1, 2016

Conversation

film42
Copy link
Member

@film42 film42 commented Jan 18, 2016

Should a server's ping port get jammed, overwhelmed, or access a port that is not open, then the ping port check will block until the TCPSocket default timeout rescues the blocked request. Instead, we can wrap Socket to add a reasonable timeout to ping port checks. By default, the timeout is set to 5 seconds, but you can customize this by setting the PB_RPC_PING_PORT_TIMEOUT environment variable.

There is probably some more work that can be done here, but I wanted this PR to be about solving this ping port problem before I make a few more changes, like using PB_RPC_PING_PORT inside of the new Ping class.

cc @abrandoned @quixoten @mmmries @liveh2o @brianstien @localshred

Should a server's ping port get jammed, overwhelmed, or access a port
that is not open, then the ping port check will block until the
TCPSocket default timeout rescues the blocked request. Instead, we can
wrap Socket to add a reasonable timeout to ping port checks. By default,
the timeout is set to 5 seconds, but you can customize this by setting
the "PB_RPC_PING_PORT_TIMEOUT" environment variable.
@film42 film42 added the Bug label Jan 18, 2016
@mmmries
Copy link
Member

mmmries commented Jan 18, 2016

Build failing looks like a bundler issue :dealwithbundler:

end

def timeout
@timeout ||= begin
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't we want the timeout to be in millis? (seconds seems like a long time)

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea.

abrandoned added a commit that referenced this pull request Feb 1, 2016
Ensure the ping port is timed out after a reasonable amount of time
@abrandoned abrandoned merged commit 00bc4a8 into master Feb 1, 2016
@abrandoned abrandoned deleted the gt/timeout_ping_port branch February 1, 2016 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants