From cd74fa23ea4d5d30d74a5830a8e358b2c527f485 Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Wed, 15 Jan 2020 08:54:42 -0800 Subject: [PATCH] internal: remove withResolverBuilder and use WithResolvers instead (#3321) --- balancer/grpclb/grpclb.go | 10 +++--- balancer/grpclb/grpclb_remote_balancer.go | 6 ++-- clientconn.go | 39 ++++++++++++----------- dialoptions.go | 14 ++------ internal/internal.go | 2 -- resolver_conn_wrapper.go | 12 ++----- 6 files changed, 31 insertions(+), 52 deletions(-) diff --git a/balancer/grpclb/grpclb.go b/balancer/grpclb/grpclb.go index 1e3478683..219ca7235 100644 --- a/balancer/grpclb/grpclb.go +++ b/balancer/grpclb/grpclb.go @@ -27,7 +27,6 @@ package grpclb import ( "context" "errors" - "strconv" "sync" "time" @@ -127,11 +126,10 @@ func (b *lbBuilder) Name() string { } func (b *lbBuilder) Build(cc balancer.ClientConn, opt balancer.BuildOptions) balancer.Balancer { - // This generates a manual resolver builder with a random scheme. This - // scheme will be used to dial to remote LB, so we can send filtered address - // updates to remote LB ClientConn using this manual resolver. - scheme := "grpclb_internal_" + strconv.FormatInt(time.Now().UnixNano(), 36) - r := &lbManualResolver{scheme: scheme, ccb: cc} + // This generates a manual resolver builder with a fixed scheme. This + // scheme will be used to dial to remote LB, so we can send filtered + // address updates to remote LB ClientConn using this manual resolver. + r := &lbManualResolver{scheme: "grpclb-internal", ccb: cc} lb := &lbBalancer{ cc: newLBCacheClientConn(cc), diff --git a/balancer/grpclb/grpclb_remote_balancer.go b/balancer/grpclb/grpclb_remote_balancer.go index ecaa1e787..e70ce7500 100644 --- a/balancer/grpclb/grpclb_remote_balancer.go +++ b/balancer/grpclb/grpclb_remote_balancer.go @@ -34,7 +34,6 @@ import ( lbpb "google.golang.org/grpc/balancer/grpclb/grpc_lb_v1" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/grpclog" - "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/backoff" "google.golang.org/grpc/internal/channelz" "google.golang.org/grpc/keepalive" @@ -228,8 +227,7 @@ func (lb *lbBalancer) newRemoteBalancerCCWrapper() { } // Explicitly set pickfirst as the balancer. dopts = append(dopts, grpc.WithDefaultServiceConfig(`{"loadBalancingPolicy":"pick_first"}`)) - wrb := internal.WithResolverBuilder.(func(resolver.Builder) grpc.DialOption) - dopts = append(dopts, wrb(lb.manualResolver)) + dopts = append(dopts, grpc.WithResolvers(lb.manualResolver)) if channelz.IsOn() { dopts = append(dopts, grpc.WithChannelzParentID(lb.opt.ChannelzParentID)) } @@ -245,7 +243,7 @@ func (lb *lbBalancer) newRemoteBalancerCCWrapper() { // // The grpclb server addresses will set field ServerName, and creds will // receive ServerName as authority. - cc, err := grpc.DialContext(context.Background(), "grpclb.subClientConn", dopts...) + cc, err := grpc.DialContext(context.Background(), lb.manualResolver.Scheme()+":///grpclb.subClientConn", dopts...) if err != nil { grpclog.Fatalf("failed to dial: %v", err) } diff --git a/clientconn.go b/clientconn.go index 8aeac74ae..e0d6f4666 100644 --- a/clientconn.go +++ b/clientconn.go @@ -239,25 +239,26 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn * if cc.dopts.bs == nil { cc.dopts.bs = backoff.DefaultExponential } - if cc.dopts.resolverBuilder == nil { - // Only try to parse target when resolver builder is not already set. - cc.parsedTarget = parseTarget(cc.target) - grpclog.Infof("parsed scheme: %q", cc.parsedTarget.Scheme) - cc.dopts.resolverBuilder = cc.getResolver(cc.parsedTarget.Scheme) - if cc.dopts.resolverBuilder == nil { - // If resolver builder is still nil, the parsed target's scheme is - // not registered. Fallback to default resolver and set Endpoint to - // the original target. - grpclog.Infof("scheme %q not registered, fallback to default scheme", cc.parsedTarget.Scheme) - cc.parsedTarget = resolver.Target{ - Scheme: resolver.GetDefaultScheme(), - Endpoint: target, - } - cc.dopts.resolverBuilder = cc.getResolver(cc.parsedTarget.Scheme) + + // Determine the resolver to use. + cc.parsedTarget = parseTarget(cc.target) + grpclog.Infof("parsed scheme: %q", cc.parsedTarget.Scheme) + resolverBuilder := cc.getResolver(cc.parsedTarget.Scheme) + if resolverBuilder == nil { + // If resolver builder is still nil, the parsed target's scheme is + // not registered. Fallback to default resolver and set Endpoint to + // the original target. + grpclog.Infof("scheme %q not registered, fallback to default scheme", cc.parsedTarget.Scheme) + cc.parsedTarget = resolver.Target{ + Scheme: resolver.GetDefaultScheme(), + Endpoint: target, + } + resolverBuilder = cc.getResolver(cc.parsedTarget.Scheme) + if resolverBuilder == nil { + return nil, fmt.Errorf("could not get resolver for default scheme: %q", cc.parsedTarget.Scheme) } - } else { - cc.parsedTarget = resolver.Target{Endpoint: target} } + creds := cc.dopts.copts.TransportCredentials if creds != nil && creds.Info().ServerName != "" { cc.authority = creds.Info().ServerName @@ -297,14 +298,14 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn * } // Build the resolver. - rWrapper, err := newCCResolverWrapper(cc) + rWrapper, err := newCCResolverWrapper(cc, resolverBuilder) if err != nil { return nil, fmt.Errorf("failed to build resolver: %v", err) } - cc.mu.Lock() cc.resolverWrapper = rWrapper cc.mu.Unlock() + // A blocking dial blocks until the clientConn is ready. if cc.dopts.block { for { diff --git a/dialoptions.go b/dialoptions.go index 2fb70e30f..63f5ae21d 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -58,9 +58,7 @@ type dialOptions struct { callOptions []CallOption // This is used by v1 balancer dial option WithBalancer to support v1 // balancer, and also by WithBalancerName dial option. - balancerBuilder balancer.Builder - // This is to support grpclb. - resolverBuilder resolver.Builder + balancerBuilder balancer.Builder channelzParentID int64 disableServiceConfig bool disableRetry bool @@ -232,13 +230,6 @@ func WithBalancerName(balancerName string) DialOption { }) } -// withResolverBuilder is only for grpclb. -func withResolverBuilder(b resolver.Builder) DialOption { - return newFuncDialOption(func(o *dialOptions) { - o.resolverBuilder = b - }) -} - // WithServiceConfig returns a DialOption which has a channel to read the // service configuration. // @@ -366,7 +357,6 @@ func WithContextDialer(f func(context.Context, string) (net.Conn, error)) DialOp } func init() { - internal.WithResolverBuilder = withResolverBuilder internal.WithHealthCheckFunc = withHealthCheckFunc } @@ -599,6 +589,6 @@ func withResolveNowBackoff(f func(int) time.Duration) DialOption { // This API is EXPERIMENTAL. func WithResolvers(rs ...resolver.Builder) DialOption { return newFuncDialOption(func(o *dialOptions) { - o.resolvers = rs + o.resolvers = append(o.resolvers, rs...) }) } diff --git a/internal/internal.go b/internal/internal.go index eae18e18c..0912f0bf4 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -28,8 +28,6 @@ import ( ) var ( - // WithResolverBuilder is set by dialoptions.go - WithResolverBuilder interface{} // func (resolver.Builder) grpc.DialOption // WithHealthCheckFunc is set by dialoptions.go WithHealthCheckFunc interface{} // func (HealthChecker) DialOption // HealthCheckFunc is used to provide client-side LB channel health checking diff --git a/resolver_conn_wrapper.go b/resolver_conn_wrapper.go index f4dca7d21..3eaf724cd 100644 --- a/resolver_conn_wrapper.go +++ b/resolver_conn_wrapper.go @@ -74,15 +74,9 @@ func parseTarget(target string) (ret resolver.Target) { return ret } -// newCCResolverWrapper uses the resolver.Builder stored in the ClientConn to -// build a Resolver and returns a ccResolverWrapper object which wraps the -// newly built resolver. -func newCCResolverWrapper(cc *ClientConn) (*ccResolverWrapper, error) { - rb := cc.dopts.resolverBuilder - if rb == nil { - return nil, fmt.Errorf("could not get resolver for scheme: %q", cc.parsedTarget.Scheme) - } - +// newCCResolverWrapper uses the resolver.Builder to build a Resolver and +// returns a ccResolverWrapper object which wraps the newly built resolver. +func newCCResolverWrapper(cc *ClientConn, rb resolver.Builder) (*ccResolverWrapper, error) { ccr := &ccResolverWrapper{ cc: cc, done: grpcsync.NewEvent(),