Never use infinity request timeouts #160
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Backport of 5aa1ab0 As described in #156, there are several types of timeouts in the client. The timeout that is generally provided as the last argument to client operations is used to create timers which prevent us from waiting for every on messages for TCP data (from gen_tcp). There are several cases where this timeout was hardcoded to infinity. This can cause the client to hang on these requests for a (mostly) unbounded time. Even when using a gen_server timeout, the gen_server itself will continue to wait for the message to come, with no timeout. Further, because of #155, we simply use the `ServerTimeout` as the `RequestTimeout`, if there is not a separate `RequestTimeout`. It's possible that the `RequestTimeout` can fire before the `ServerTimeout` (this timeout is remote), but we'd otherwise just be picking some random number to be the difference between them. Addressing #155 will shed more light on this.
|
This is a partial 1.4 backport of #156. |
|
+1, test pass, looks good. |
|
@evanmcc you mind giving this a second pair of eyes, since we had talked about it before? |
|
looks sensible to me, there don't seem to be any lingering hardcoded infinity timeouts remaining for the request timeout. |
reiddraper
added a commit
that referenced
this pull request
Feb 19, 2014
Never use infinity request timeouts
reiddraper
added a commit
to basho/riak_cs
that referenced
this pull request
Feb 19, 2014
* Revert part of 6e7027e. It is preferrable to use the request timeout over gen_server:call timoeuts. Further, these timeouts have been removed in the client in the master branch. * Make the fold objects request timeout configurable, using `fold_objects_timeout`. * Update riakc dependency to get features added in * basho/riak-erlang-client#160
reiddraper
added a commit
to basho/riak_cs
that referenced
this pull request
Feb 19, 2014
* Revert part of 6e7027e. It is preferrable to use the request timeout over gen_server:call timoeuts. Further, these timeouts have been removed in the client in the master branch. * Make the fold objects request timeout configurable, using `fold_objects_timeout`. * Update riakc dependency to get features added in * basho/riak-erlang-client#160 * Update riak_repl_pb_api (to get a consistent dependency on riak_pb)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of 5aa1ab0
As described in #156, there are several types of timeouts in the client.
The timeout that is generally provided as the last argument to client
operations is used to create timers which prevent us from waiting for
every on messages for TCP data (from gen_tcp). There are several cases
where this timeout was hardcoded to infinity. This can cause the client
to hang on these requests for a (mostly) unbounded time. Even when using
a gen_server timeout, the gen_server itself will continue to wait for
the message to come, with no timeout. Further, because of #155, we
simply use the
ServerTimeoutas theRequestTimeout, if there is nota separate
RequestTimeout. It's possible that theRequestTimeoutcanfire before the
ServerTimeout(this timeout is remote), but we'dotherwise just be picking some random number to be the difference
between them. Addressing #155 will shed more light on this.