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

Handle ENOTCONN #102

Merged
merged 3 commits into from
Jan 18, 2019
Merged

Handle ENOTCONN #102

merged 3 commits into from
Jan 18, 2019

Conversation

blaines
Copy link
Contributor

@blaines blaines commented Jan 16, 2019

Unset socket under the condition ENOTCONN this could happen in the event the datadog agent is restarted and using a unix socket file.

  • Unset socket in the condition a connection is lost
  • Added test to handle ENOTCONN

Unset socket under the condition ENOTCONN this could happen in the event the datadog agent is restarted and using a unix socket file.
@@ -52,6 +52,7 @@ def write(message)
bad_socket = !@socket_path.nil? && (
boom.is_a?(Errno::ECONNREFUSED) ||
boom.is_a?(Errno::ECONNRESET) ||
boom.is_a?(Errno::ENOTCONN) ||

Choose a reason for hiding this comment

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

It might actually be better to not add this and instead relax the retry conditions on this line

if retries <= 1 && boom.is_a?(IOError) && boom.message =~ /closed stream/i

to just

        if retries <= 1

In our case as well as I'm sure in many others, we'd much rather immediately retry establishing the socket and resending the message rather than losing the message and having to wait for the next one for the socket to re-establish itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I changed the logic to retry the same message upon ENOTCONN

Copy link
Contributor

@gmmeyer gmmeyer left a comment

Choose a reason for hiding this comment

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

this looks good to me! thanks a lot for doing this! 😄

@gmmeyer gmmeyer merged commit e1c5908 into DataDog:master Jan 18, 2019
@Shadow6363
Copy link

Hey @gmmeyer, thanks for merging this! Is there a standard release schedule you follow? Just wondering when I should update our agents to pull this in.

@Shadow6363
Copy link

Also, just realized @blaines, you never removed the original version of your code. There's a return block after that if so your change to retry isn't actually ever going to get reached. :/

blaines added a commit to blaines/dogstatsd-ruby that referenced this pull request Jan 23, 2019
blaines added a commit to blaines/dogstatsd-ruby that referenced this pull request Jan 23, 2019
@masci masci added this to the 4.1.0 milestone Mar 8, 2019
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.

None yet

4 participants