nautilus: msg/async: connection race + winner fault can leave connection stuck at replacing foreve #27915
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
pdvian
commented
May 2, 2019
| @@ -164,7 +164,7 @@ Pipe::Pipe(SimpleMessenger *r, int st, PipeConnection *con) | |||
|
|
|||
| randomize_out_seq(); | |||
|
|
|||
| msgr->timeout = msgr->cct->_conf->ms_tcp_read_timeout * 1000; //convert to ms | |||
| msgr->timeout = msgr->cct->_conf->ms_connection_idle_timeout * 1000; //convert to ms | |||
There was a problem hiding this comment.
@xiexingguo Need review here. The Pipe constructor still referring to ms_tcp_read_timeout conf and has to be renamed to ms_connection_idle_timeout.
xiexingguo
approved these changes
May 5, 2019
smithfarm
added
messenger
|
@dillaman @xiexingguo Sure. Let me work on it. |
The old naming is confusing, e.g., it actually indicates we should tear down the underlying connection which has no read/write activities at both sides (namely connection is idle) for over 15 minutes. Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn> (cherry picked from commit 1d46422) Conflicts: src/common/legacy_config_opts.h : Resolved for ms_connection_idle_timeout src/common/options.cc : Resolved for ms_connection_idle_timeout src/msg/simple/Pipe.cc : rename ms_tcp_read_timeout to ms_connection_idle_timeout
There could be various corner cases that may cause an async connection stuck in the connecting stage (e.g., by manually creating some loop back connections on the switches of our test cluster, we can almost 100% reproduce http://tracker.ceph.com/issues/37499). In 61b9432 I try to employ the existing keep_alive mechanism to get those stuck connections out of the trap but it does not work if the corresponding connection is not yet ready, since we always require the underlying connection to be **ready** in order to send out a keep_alive message. Fix by making a more general connecting timeout strategy. If a connecting process can not be finished within a specific interval, then we simply cut it off and retry. Fixes: http://tracker.ceph.com/issues/37499 Fixes: http://tracker.ceph.com/issues/38493 Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn> (cherry picked from commit 7209cc6) Conflicts: src/common/legacy_config_opts.h src/common/options.cc : Resolved for ms_connection_ready_timeout
Long running clients connected to thrashing OSDs could result in a "see no progress in more than <timeout>" message printed to stderr. This is not an error but can result in test failures when console output is compared against expected output. Fixes: http://tracker.ceph.com/issues/39448 Signed-off-by: Jason Dillaman <dillaman@redhat.com> (cherry picked from commit 6b4f972)
pdvian
force-pushed
the
wip-39241-nautilus
branch
from
d536379
to
e52c645
Compare
|
@dillaman @xiexingguo Cherry-picked #28050. Kindly review. |
xiexingguo
approved these changes
Jun 5, 2019
dillaman
added
nautilus-batch-1
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.
http://tracker.ceph.com/issues/39241