Fix connectivity state transitions when dialing #1596
Conversation
|
@menghanl thanks for making this change. Do you think it deserves a test? |
menghanl
force-pushed
the
dial_connectivity_state
branch
from
ab4e042
to
d24a10a
Compare
menghanl
force-pushed
the
dial_connectivity_state
branch
from
d24a10a
to
3ba27f2
Compare
|
I removed the |
call_test.go
Outdated
| @@ -285,7 +285,7 @@ func TestInvokeCancelClosedNonFailFast(t *testing.T) { | |||
| req := "hello" | |||
| ctx, cancel := context.WithCancel(context.Background()) | |||
| cancel() | |||
| if err := Invoke(ctx, "/foo/bar", &req, &reply, cc, FailFast(false)); err == nil { | |||
| if err := Invoke(ctx, "/foo/bar", &req, &reply, cc); err == nil { | |||
There was a problem hiding this comment.
this test is called TestInvokeCancelClosedNonFailFast - did you mean to make this change?
There was a problem hiding this comment.
Good catch. Reverted.
clientconn.go
Outdated
| @@ -904,15 +890,17 @@ func (ac *addrConn) errorf(format string, a ...interface{}) { | |||
|
|
|||
| // resetTransport recreates a transport to the address for ac. The old | |||
| // transport will close itself on error or when the clientconn is closed. | |||
| // | |||
| // The connectivity state of ac will be set to TransientFailure if it's not | |||
There was a problem hiding this comment.
Is this accurate? It seems that ac.state will be set to connectivity.Connecting unconditionally and then to connectivity. TransientFailure on every attempt; I can't tell which part of this is sensitive to "if it's not first time doing resetTransport."
There was a problem hiding this comment.
The state is set by transportMonitor to TransientFailure before this function is called.
I removed this comment and add a comment in transportMonitor.
|
Can you guys cut 1.7.1 with this? |
|
@tamird Will do it this week. |
This commit cherry-picked the connectivity state change from b3ed81a, excluding the connect() function cleanup.
The connectivity for addrConn goes from
ConnectingtoTransientFailureand back toConnectingwhen dialing. This causes the first RPC to fail if it's not wait-for-ready. This PR makes addrConn skip the firstTransientFailureif it's dialing.