Skip to content

Commit 5f95d39

Browse files
craig[bot]cthumuluru-crdb
andcommitted
Merge #148195
148195: gossip: consolidate `GossipClient` RPC client creation r=cthumuluru-crdb a=cthumuluru-crdb This commit consolidates `GossipClient` RPC client creation logic in gossip package. It is a continuation of the work done in #147606. Epic: CRDB-48923 Fixes: #148201 Release note: none Co-authored-by: Chandra Thumuluru <[email protected]>
2 parents 92aa0df + 49a77a7 commit 5f95d39

File tree

2 files changed

+38
-15
lines changed

2 files changed

+38
-15
lines changed

pkg/gossip/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ go_library(
4141
"@com_github_cockroachdb_errors//:errors",
4242
"@com_github_cockroachdb_logtags//:logtags",
4343
"@com_github_cockroachdb_redact//:redact",
44+
"@org_golang_google_grpc//:grpc",
4445
],
4546
)
4647

pkg/gossip/client.go

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/cockroachdb/cockroach/pkg/util/stop"
2222
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
2323
"github.com/cockroachdb/errors"
24+
"google.golang.org/grpc"
2425
)
2526

2627
// client is a client-side RPC connection to a gossip peer node.
@@ -100,24 +101,12 @@ func (c *client) startLocked(
100101
}()
101102

102103
stream, err := func() (Gossip_GossipClient, error) {
103-
// Note: avoid using `grpc.WithBlock` here. This code is already
104-
// asynchronous from the caller's perspective, so the only effect of
105-
// `WithBlock` here is blocking shutdown - at the time of this writing,
106-
// that ends ups up making `kv` tests take twice as long.
107-
var connection *rpc.GRPCConnection
108-
if c.peerID != 0 {
109-
connection = rpcCtx.GRPCDialNode(c.addr.String(), c.peerID, c.locality, rpcbase.SystemClass)
110-
} else {
111-
// TODO(baptist): Use this as a temporary connection for getting
112-
// onto gossip and then replace with a validated connection.
113-
log.Infof(ctx, "unvalidated bootstrap gossip dial to %s", c.addr)
114-
connection = rpcCtx.GRPCUnvalidatedDial(c.addr.String(), c.locality)
115-
}
116-
conn, err := connection.Connect(ctx)
104+
gc, err := c.dialGossipClient(ctx, rpcCtx)
117105
if err != nil {
118106
return nil, err
119107
}
120-
stream, err := NewGossipClient(conn).Gossip(ctx)
108+
109+
stream, err := gc.Gossip(ctx)
121110
if err != nil {
122111
return nil, err
123112
}
@@ -406,3 +395,36 @@ func (c *client) gossip(
406395
}
407396
}
408397
}
398+
399+
// dials the peer node and returns a gRPC connection to the peer node.
400+
func (c *client) dial(ctx context.Context, rpcCtx *rpc.Context) (*grpc.ClientConn, error) {
401+
// Note: avoid using `grpc.WithBlock` here. This code is already
402+
// asynchronous from the caller's perspective, so the only effect of
403+
// `WithBlock` here is blocking shutdown - at the time of this writing,
404+
// that ends ups up making `kv` tests take twice as long.
405+
var conn *rpc.GRPCConnection
406+
if c.peerID != 0 {
407+
conn = rpcCtx.GRPCDialNode(c.addr.String(), c.peerID, c.locality, rpcbase.SystemClass)
408+
} else {
409+
// TODO(baptist): Use this as a temporary connection for getting
410+
// onto gossip and then replace with a validated connection.
411+
log.Infof(ctx, "unvalidated bootstrap gossip dial to %s", c.addr)
412+
conn = rpcCtx.GRPCUnvalidatedDial(c.addr.String(), c.locality)
413+
}
414+
415+
return conn.Connect(ctx)
416+
}
417+
418+
// dialGossipClient establishes a DRPC connection if enabled; otherwise,
419+
// it falls back to gRPC. The established connection is used to create a
420+
// GossipClient.
421+
func (c *client) dialGossipClient(ctx context.Context, rpcCtx *rpc.Context) (GossipClient, error) {
422+
if !rpcbase.TODODRPC {
423+
conn, err := c.dial(ctx, rpcCtx)
424+
if err != nil {
425+
return nil, err
426+
}
427+
return NewGossipClient(conn), nil
428+
}
429+
return nil, nil
430+
}

0 commit comments

Comments
 (0)