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)
This change introduces some behavior changes that should not impact users that
are following the proper stream protocol. Specifically, one of the following
conditions must be satisfied:
1. The user calls Close on the ClientConn.
2. The user cancels the context provided to NewClientStream, or its deadline
expires. (Note that it if the context is no longer needed before the deadline
expires, it is still recommended to call cancel to prevent bloat.) It is always
recommended to cancel contexts when they are no longer needed, and to
never use the background context directly, so all users should always be
doing this.
3. The user calls RecvMsg (or Recv in generated code) until a non-nil error is
returned.
4. The user receives any error from Header or SendMsg (or Send in generated
code) besides io.EOF. If none of the above happen, this will leak a goroutine
and a context, and grpc will not call the optionally-configured stats handler
with a stats.End message.
Before this change, if a user created a stream and the server ended the stream,
the stats handler would be invoked with a stats.End containing the final status
of the stream. Subsequent calls to RecvMsg would then trigger the stats handler
with InPayloads, which may be unexpected by stats handlers.
WithBalancerName dial option specifies the name of the balancer to be used by the ClientConn. Service config updates can NOT override the balancer option.
This allows ClientConn to get more up-to-date addresses from resolver.
ClientConn compares new addresses with the cached ones. So if resolver returns the same set of addresses, ClientConn will not notify balancer about it.
Also moved the initialization of resolver and balancer to avoid race. Balancer will only be started when ClientConn gets resolved addresses from balancer.
- The transport is now responsible for closing its own connection when an error
occurs or when the context given to it in NewClientTransport() is canceled.
- Remove client/server shutdown channels -- add cancel function to allow
self-cancellation.
- Plumb the clientConn's context into the client transport to allow for the
transport to be canceled even after it has been removed from the ac (due to
graceful close) when the ClientConn is closed.
A leak happens when DialContext times out before a balancer returns any
addresses or before a successful connection is established.
The loop in ClientConn.lbWatcher breaks and doneChan never gets closed.
- All logs use 1 severity level instead of printf
- All transport logs only go to verbose level 2+
- The default logger only log errors and verbosity level 1
- Add environment variable GRPC_GO_LOG_SEVERITY_LEVEL and GRPC_GO_LOG_VERBOSITY_LEVEL to set severity or verbosity levels for the default logger
When timeout is not hit `time.After` will leak unnecessary timer, so
it's better to stop timer explicitly.
Signed-off-by: Alexander Morozov <lk4d4math@gmail.com>
With this change, the default dialer checks environment variables to see if proxy is needed. If so, it dials to the proxy and does an HTTP CONNECT handshake.
This modifies the WithBlock behavior somewhat to block until there is at least
one valid connection. Previously, each connection would be made serially until
all had completed successfully, with any errors returned to the caller. Errors
are now only returned due to connecting to a backend if a balancer is not used,
or if there is an error starting the balancer itself.
Fixes#976
The :authority pseudo-header for a gRPC Client defaults to the host
portion of the dialed target and can only be overwritten by providing a
TransportCredentials. However, there are cases where setting this header
independent of any tranport security is valid. In my particular case,
in order to leverage Envoy for request routing, the cluster/service name
must be provided in the :authority header. This may also be useful in a
testing context.
This patch adds a DialOption to overwrite the authority header,
even if TransportCredentials are provided (I'd imagine you'd only ever
need to specify one or the other).
* Add the initial service config support
* start scWatcher later
* remove timeoutCh
* address the comments
* deal with dial timeout
* defer cancel for the newly created context for correct lifetime management
* fix the defer order
* added other 2 missing cancels
Prior to this change, it was possible for `DialContext` to return
`(nil, err)` without properly closing the `ClientConn`, resulting in an
unavoidable leak of the `resetAddrConn` goroutine.