Invoke hangs in client after server presents an incorrect certificate

[rra]

Please answer these questions before submitting your issue.

What version of gRPC are you using?

a5986a5

What version of Go are you using (go version)?

1.6 and 1.8 (same behavior in both)

What operating system (Linux, Windows, …) and version?

Linux (Ubuntu 16.04)

What did you do?

When attempting to update our internal grpc-go at Dropbox to the above version, one of our tests started hanging until killed by a timeout. The test tries to connect to a gRPC server that is presenting a different certificate identity than the client is expecting. This is a regression from the previous version of grpc-go we were using (66c9ed8).

What did you expect to see?

The client should return from Invoke with an error.

What did you see instead?

The client hung forever unless a timeout was set. This is true even if FastFail is set.

Tracing into the grpc-go code, the call sequence appears to be as follows:

1. DialContext succeeds.
2. WaitForStateChange eventually fails due to a timeout (as expected because a successful connection cannot be opened to the server presenting the incorrect certificate)
3. Client calls Invoke
4. Invoke -> invoke (in call.go) calls getTransport
5. getTransport in clientconn.go calls pick
6. pick in picker_wrapper.go sets ch to bp.blockingCh (not the original value on entry)
7. pick calls Pick, which returns the error balancer.ErrNoSubConnAvailable, causing continue to restart the loop.
8. ch is now bp.blockingCh, so pick enters the select on ch and at that point blocks forever.

At this point, there is no background activity or attempt to re-establish the connection (as determined by annotations inside ClientHandshake in the TransportCredentials), so I see no sign that the Invoke call will ever recover barring a timeout.

[dzbarsky]

@dfawley can you please take a look? thanks!

[menghanl]

The failfast RPCs should fail immediately with unavailable errors. Fix for this bug is in #1657.

The error will be a general error "all SubConns are in TransientFailure" now. We will try to include the reason (for example creds handshake errors) in the error message.

Non-failfast RPCs will keep waiting for connections to be ready, and will fail with DeadlineExceeded (in theory, the connection could recover, e.g. the resolver might return a new address).

Let us know what you think of this behavior.

[rra]

Thanks for the fix! We set failfast by default in our internal code, so I think this works for us, but @dzbarsky to confirm.

[dzbarsky]

Yep, yyour fix makes sense. Thank you.

[menghanl]

Sounds good. Thanks for confirming.

We will still work on including the error details in the RPC errors, but this issue should not be a blocker now.
Moving this to P2.