Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: expose NewClient method to end users #7010

Merged
merged 17 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions balancer/rls/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,13 @@ func parseRLSProto(rlsProto *rlspb.RouteLookupConfig) (*lbConfig, error) {
parsedTarget, err := url.Parse(lookupService)
if err != nil {
// url.Parse() fails if scheme is missing. Retry with default scheme.
parsedTarget, err = url.Parse(resolver.GetDefaultScheme() + ":///" + lookupService)
parsedTarget, err = url.Parse(resolver.GetDefaultSchemeOrPassthrough() + ":///" + lookupService)
bruuuuuuuce marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("rls: invalid target URI in lookup_service %s", lookupService)
}
}
if parsedTarget.Scheme == "" {
parsedTarget.Scheme = resolver.GetDefaultScheme()
parsedTarget.Scheme = resolver.GetDefaultSchemeOrPassthrough()
}
if resolver.Get(parsedTarget.Scheme) == nil {
return nil, fmt.Errorf("rls: unregistered scheme in lookup_service %s", lookupService)
Expand Down
19 changes: 14 additions & 5 deletions clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ func (dcs *defaultConfigSelector) SelectConfig(rpcInfo iresolver.RPCInfo) (*ires
}, nil
}

// newClient returns a new client in idle mode.
func newClient(target string, opts ...DialOption) (conn *ClientConn, err error) {
// NewClient returns a new client in idle mode.
func NewClient(target string, opts ...DialOption) (conn *ClientConn, err error) {
cc := &ClientConn{
target: target,
conns: make(map[*addrConn]struct{}),
Expand Down Expand Up @@ -208,7 +208,11 @@ func newClient(target string, opts ...DialOption) (conn *ClientConn, err error)
// https://github.com/grpc/grpc/blob/master/doc/naming.md.
// e.g. to use dns resolver, a "dns:///" prefix should be applied to the target.
func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *ClientConn, err error) {
cc, err := newClient(target, opts...)
// At the end of this method, we kick the channel out of idle, rather than waiting for the first rpc.
// The eagerConnect option is used to tell the ClientChannel that DialContext was called to make
// this channel, and hence tries to connect immediately.
bruuuuuuuce marked this conversation as resolved.
Show resolved Hide resolved
opts = append(opts, WithEagerConnect())
cc, err := NewClient(target, opts...)
bruuuuuuuce marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1740,8 +1744,13 @@ func (cc *ClientConn) parseTargetAndFindResolver() error {
// We are here because the user's dial target did not contain a scheme or
// specified an unregistered scheme. We should fallback to the default
// scheme, except when a custom dialer is specified in which case, we should
// always use passthrough scheme.
defScheme := resolver.GetDefaultScheme()
// always use passthrough scheme. For either case, we need to respect any overridden
// global defaults set by the user.
defScheme := resolver.GetDefaultSchemeOrDNS()
if cc.dopts.eagerConnect {
defScheme = resolver.GetDefaultSchemeOrPassthrough()
}
bruuuuuuuce marked this conversation as resolved.
Show resolved Hide resolved

channelz.Infof(logger, cc.channelzID, "fallback to scheme %q", defScheme)
canonicalTarget := defScheme + ":///" + cc.target

Expand Down
4 changes: 2 additions & 2 deletions clientconn_parsed_target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
)

func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) {
defScheme := resolver.GetDefaultScheme()
defScheme := resolver.GetDefaultSchemeOrPassthrough()
tests := []struct {
target string
wantParsed resolver.Target
Expand Down Expand Up @@ -93,7 +93,7 @@ func (s) TestParsedTarget_Failure_WithoutCustomDialer(t *testing.T) {
}

func (s) TestParsedTarget_WithCustomDialer(t *testing.T) {
defScheme := resolver.GetDefaultScheme()
defScheme := resolver.GetDefaultSchemeOrPassthrough()
tests := []struct {
target string
wantParsed resolver.Target
Expand Down
10 changes: 10 additions & 0 deletions dialoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type dialOptions struct {
resolvers []resolver.Builder
idleTimeout time.Duration
recvBufferPool SharedBufferPool
eagerConnect bool
dfawley marked this conversation as resolved.
Show resolved Hide resolved
}

// DialOption configures how we set up the connection.
Expand Down Expand Up @@ -487,6 +488,15 @@ func WithUserAgent(s string) DialOption {
})
}

// WithEagerConnect tells the ClientConnection that the connection was created directly
// with either Dial or DialContext, meaning that the connection is automatically put into
// a connecting state.
func WithEagerConnect() DialOption {
return newFuncDialOption(func(o *dialOptions) {
o.eagerConnect = true
})
}

// WithKeepaliveParams returns a DialOption that specifies keepalive parameters
// for the client transport.
func WithKeepaliveParams(kp keepalive.ClientParameters) DialOption {
Expand Down
23 changes: 20 additions & 3 deletions resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
// m is a map from scheme to resolver builder.
m = make(map[string]Builder)
// defaultScheme is the default scheme to use.
defaultScheme = "passthrough"
defaultScheme = ""
dfawley marked this conversation as resolved.
Show resolved Hide resolved
passthroughScheme = "passthrough"
dnsScheme = "dns"
)

// TODO(bar) install dns resolver in init(){}.
Expand Down Expand Up @@ -72,8 +74,23 @@
defaultScheme = scheme
bruuuuuuuce marked this conversation as resolved.
Show resolved Hide resolved
}

// GetDefaultScheme gets the default scheme that will be used.
func GetDefaultScheme() string {
// GetDefaultSchemeOrPassthrough gets the default scheme that will be used, if no scheme was
// overridden, defaults to the "passthrough" scheme. This scheme is used when directly calling
// the Dial method to create a client connection.
func GetDefaultSchemeOrPassthrough() string {
if defaultScheme == "" {
return passthroughScheme
}
return defaultScheme

Check warning on line 84 in resolver/resolver.go

View check run for this annotation

Codecov / codecov/patch

resolver/resolver.go#L84

Added line #L84 was not covered by tests
}

// GetDefaultSchemeOrDNS gets the default scheme that will be used, if no scheme was overridden,
// defaults to the "dns" scheme. This is the default scheme when calling NewClient to generate a
// new client connection.
func GetDefaultSchemeOrDNS() string {
if defaultScheme == "" {
return dnsScheme
}
return defaultScheme
}

Expand Down
Loading