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

Add FlusherTimeout to prevent blocking for long on unhealthy connections #252

Merged
merged 2 commits into from
Jan 14, 2017

Conversation

wallyqs
Copy link
Member

@wallyqs wallyqs commented Jan 14, 2017

This adds a customizable FlusherTimeout option for the flusher loop to be able to yield after a certain time in case a write to the underlying writer takes too long due to an unhealthy connection.

This prevents the client blocking for an indefinite amount of time until reaching an OS level i/o timeout since the bufio.Flush is happening while the lock is being hold, thus giving room for the ping interval to be able to processOpErr and disconnect once it reaches the max amount of missing ping replies from the server (with an unhealthy connection the reading loop would be stuck on conn.Read too because there is no deadline either, so it would take as a long as the ping interval check closes the connection or i/o timeout).

Waldemar Quevedo added 2 commits January 13, 2017 16:10
Add customizable `FlusherTimeout` option for the flusher loop to be
able to yield after a certain time in case a write to the underlying
writer takes too long due to an unhealthy connection. This prevents
the client blocking for an indefinite amount of time until reaching an
OS level `i/o timeout` since the `bufio.Flush` is happening while the
lock is being hold, thus giving room for the ping interval to be able
to `processOpErr` and disconnect once it reaches the max amount of
missing ping replies from the server (with an unhealthy connection the
reading loop would be stuck on `conn.Read` too because there is no
deadline either, so it would take as a long as the ping interval check
closes the connection or `i/o timeout`).
// Allow customizing how long we should wait for a flush to be done
// to prevent unhealthy connections blocking the client for too long.
if flusherTimeout > 0 {
conn.SetWriteDeadline(time.Now().Add(flusherTimeout))
Copy link
Member

Choose a reason for hiding this comment

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

Where do we clear it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for checking, I added resetting the WriteDeadline on successful flush now

@derekcollison derekcollison merged commit 79bdc34 into nats-io:master Jan 14, 2017
@wallyqs wallyqs deleted the flusher-loop-timeout-option branch January 14, 2017 01:21
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