Closing `ClientConn` sets `balancerWrapper` to nil.
If service config switches balancer, the new balancer will be notified of the existing addresses.
When these two happens together, there's a chance that a method will be called on the nil `balancerWrapper`. This change adds a check to make sure that never happens.
fixes#2367
internal: fix onClose state transitions
When onClose occurs during WaitForHandshake, it should immediately
exit createTransport instead of leaving CONNECTING and entering READY.
Furthermore, when any onClose happens, the state should change to
TRANSIENT FAILURE.
Fixes#2340Fixes#2341
Also fixes an unreported bug in which entering READY causes a
Dial call to end prematurely, instead of blocking until a READY
transport is found.
Google default creds is a combo of ALTS, TLS and OAuth2. The right set of creds will be picked to use based on environment.
This PR contains:
- A new `creds.Bundle` type
- changes to use it in ClientConn and transport
- dial option to set the bundle for a ClientConn
- balancer options and NewSubConnOption to set it for SubConn
- Google default creds implementation by @cesarghali
- grpclb changes to use different creds mode for different servers
- interop client changes for google default creds testing
This fixes a race in ac.tearDown and ac.resetTransport. If ac.resetTransport
is in backoff when ac.tearDown occurs, there's a race between the state
changing to Shutdown and ac.resetTransport calling ac.createTransport.
This fixes it by returning when ac.resetTransport encounters an error
during ac.nextAddr (specifically ac.ctx.Error()). It also fixes it by
making sure that ac.tearDown changes state to Shutdown before canceling
the context.
Both fixes were implemented because they both seem to be valuable
standalone additions: the former makes ac.resetTransport more
understandable and less dependent on behavior happening elsewhere,
and the latter makes ac.tearDown more correct.
Finally, TestDialParseTargetUnknownScheme had its buffer removed; the
buffer was likely added a while ago to assuage this issue. It should
not be necessary anymore.
internal: remove transportMonitor, replace with callbacks
This refactors the internal http2 transport to use callbacks instead
of continuously monitoring the transport in a separate goroutine. This
has several advantages:
- Less goroutines.
- Less complexity: synchronous callbacks are much easier to reason to
reason about than asynchronous monitoring goroutines.
- Callbacks: these provide definitive locations for monitoring the
creation and closure of a transport, paving the way for GracefulStop.
This CL also consolidates all the logic about backoff and iterating
through the list of addresses into a single method.
This is a breaking change, but the transport package was never intended for use outside of grpc. Any current users that we are aware of are incorrect or have a preferred alternative.
This PR splits out grpclb from grpc. I have made the PR in several commits so you can see more clearly the steps that happened.
There are a few possibly contentious points that I would like to make clear up front:
* grpclb will no longer autoload as a load balancer. I think this is okay, as service config is not widely (at all?) used, and I believe this is the only way to access it.
* `internal` is used more, as a way of having code shared between packages without exposing types
* ConnectivityStateEvaluator, as used by grpclb, is no longer thread safe. I believe there is an outer mutex that guards access, but I want to point out this subtle change up here.
All but one tests pass with this, due to another cyclic dependency. I can fix this, but it is a little more widely scoped (such as exposing grpc.server and grpc.errorDesc in the internal package). This PR is a nearly-passing sample of that last step to get this working.
PTAL @menghanl @dfawley
This fixes:
clientconn.go:948:3: should write m = cc.sc.Methods[method[:i+1]] instead of m, _ = cc.sc.Methods[method[:i+1]] (S1005)
encoding/proto/proto_test.go:43:5: should use !bytes.Equal(p.GetBody(), expectedBody) instead (S1004)
resolver/dns/dns_resolver.go:260:2: should merge variable declaration with assignment on next line (S1021)
resolver/dns/dns_resolver.go:344:2: should use 'return <expr>' instead of 'if <expr> { return <bool> }; return <bool>' (S1008)
Doug Fawley
Menghan Li
Jean de Klerk
lyuxuan
Dustin Spicuzza
Mike Cheng
mmukhi
Carl Mastrangelo
Chyroc
Karsten Weiss
Sam Batschelet
Thomas Meson