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

Make stale connection detection configurable #505

Merged
merged 3 commits into from
Dec 19, 2016

Conversation

mbrevoort
Copy link
Contributor

A Beep Boop user ran into an issue when on connection and periodic points in time, they were doing A LOT of work that was maxing out CPU so much that it caused node.js's event loop to queue excessively. RTM connections were healthy and returning pong responses but the node process wasn't processing the pong response in time because the event queue was so backed up. This caused a lot of thrashing of connects/discconects/connects within their process:

thrash

screenshot 2016-11-16 15 06 37

Ultimately they need to better manage/throttle the work being done inside of the process, but the mitigation to avoid the connection thrashing was to increase the amount of time considered to consider a connection stale.

An overview of the changes:

  • add stale_connection_timeout to make stale connection detection configurable. This is the number of milliseconds to wait for a pong response to a ping
  • replace the use of an interval with a timeout to ensure we don't clog the event loop under duress.
  • add details to the Slack Readme

@jonchurch
Copy link
Contributor

Hey this looks good! It's a weird edge case for sure, but seems reasonable.

Any reason you took out the PING SENT and PONG RECEIVED debug messages?

@benbrown benbrown merged commit f4c50f0 into howdyai:master Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants