Skip to content

Commit 722a04e

Browse files
*: drop TODODRPC override in favor of cluster setting
Currently, at many call sites, we disable DRPC using TODODRPC override. This change drops TODODRPC override and ensure DRPC is controlled exclusively through the `rpc.experimental_drpc.enabled` cluster setting. Epic: CRDB-48935 Informs: None Release note: None
1 parent a4e63be commit 722a04e

File tree

12 files changed

+19
-29
lines changed

12 files changed

+19
-29
lines changed

pkg/acceptance/localcluster/cluster.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ func (n *Node) StatusClient(ctx context.Context) serverpb.RPCStatusClient {
478478
return existingClient
479479
}
480480

481-
if !rpcbase.TODODRPC {
481+
if !rpcbase.DRPCEnabled(ctx, n.rpcCtx.Settings) {
482482
conn, err := n.rpcCtx.GRPCUnvalidatedDial(n.RPCAddr(), roachpb.Locality{}).Connect(ctx)
483483
if err != nil {
484484
log.Dev.Fatalf(context.Background(), "failed to initialize status client: %s", err)

pkg/cli/rpc_clients.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func makeRPCClientConfig(cfg server.Config) rpc.ClientConnConfig {
101101

102102
func newClientConn(ctx context.Context, cfg server.Config) (rpcConn, func(), error) {
103103
ccfg := makeRPCClientConfig(cfg)
104-
if !rpcbase.TODODRPC {
104+
if !rpcbase.DRPCEnabled(ctx, cfg.Settings) {
105105
cc, finish, err := rpc.NewClientConn(ctx, ccfg)
106106
if err != nil {
107107
return nil, nil, errors.Wrap(err, "failed to connect to the node")

pkg/gossip/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ func (c *client) drpcDial(ctx context.Context, rpcCtx *rpc.Context) (drpc.Conn,
437437
func (c *client) dialGossipClient(
438438
ctx context.Context, rpcCtx *rpc.Context,
439439
) (RPCGossipClient, error) {
440-
if !rpcbase.TODODRPC {
440+
if !rpcbase.DRPCEnabled(ctx, rpcCtx.Settings) {
441441
conn, err := c.dial(ctx, rpcCtx)
442442
if err != nil {
443443
return nil, err

pkg/kv/kvclient/kvtenant/connector.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -979,7 +979,7 @@ func (c *connector) dialAddrs(ctx context.Context) (*client, error) {
979979
// Try each address on each retry iteration (in random order).
980980
for _, i := range rand.Perm(len(c.addrs)) {
981981
addr := c.addrs[i]
982-
if !rpcbase.TODODRPC {
982+
if !rpcbase.DRPCEnabled(ctx, c.rpcContext.Settings) {
983983
conn, err := c.dialAddr(ctx, addr)
984984
if err != nil {
985985
log.Dev.Warningf(ctx, "error dialing tenant KV address %s: %v", addr, err)

pkg/rpc/nodedialer/nodedialer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func (n *Dialer) DialInternalClient(
171171

172172
var client rpc.RestrictedInternalClient
173173
useStreamPoolClient := shouldUseBatchStreamPoolClient(ctx, n.rpcContext.Settings)
174-
if !rpcbase.ExperimentalDRPCEnabled.Get(&n.rpcContext.Settings.SV) {
174+
if !rpcbase.DRPCEnabled(ctx, n.rpcContext.Settings) {
175175
gc, conn, err := dial(ctx, n.resolver, n.rpcContext.GRPCDialNode, nodeID, class, true /* checkBreaker */)
176176
if err != nil {
177177
return nil, errors.Wrapf(err, "gRPC")

pkg/rpc/rpcbase/nodedialer.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,6 @@ var ExperimentalDRPCEnabled = settings.RegisterBoolSetting(
3939
return nil
4040
}))
4141

42-
// TODODRPC is a marker to identify each RPC client creation site that needs to
43-
// be updated to support DRPC.
44-
const TODODRPC = false
45-
4642
// NodeDialer interface defines methods for dialing peer nodes using their
4743
// node IDs.
4844
type NodeDialer interface {
@@ -70,10 +66,8 @@ func DialRPCClient[C any](
7066
drpcClientFn func(drpc.Conn) C,
7167
st *cluster.Settings,
7268
) (C, error) {
73-
useDRPC := ExperimentalDRPCEnabled.Get(&st.SV)
74-
7569
var nilC C
76-
if !TODODRPC && !useDRPC {
70+
if !DRPCEnabled(ctx, st) {
7771
conn, err := nd.Dial(ctx, nodeID, class)
7872
if err != nil {
7973
return nilC, err
@@ -99,10 +93,8 @@ func DialRPCClientNoBreaker[C any](
9993
drpcClientFn func(drpc.Conn) C,
10094
st *cluster.Settings,
10195
) (C, error) {
102-
useDRPC := ExperimentalDRPCEnabled.Get(&st.SV)
103-
10496
var nilC C
105-
if !TODODRPC && !useDRPC {
97+
if !DRPCEnabled(ctx, st) {
10698
conn, err := nd.DialNoBreaker(ctx, nodeID, class)
10799
if err != nil {
108100
return nilC, err
@@ -116,3 +108,7 @@ func DialRPCClientNoBreaker[C any](
116108
}
117109
return drpcClientFn(conn), nil
118110
}
111+
112+
func DRPCEnabled(ctx context.Context, st *cluster.Settings) bool {
113+
return st != nil && ExperimentalDRPCEnabled.Get(&st.SV)
114+
}

pkg/server/admin.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2135,7 +2135,7 @@ func (s *adminServer) checkReadinessForHealthCheck(ctx context.Context) error {
21352135
return err
21362136
}
21372137

2138-
if rpcbase.ExperimentalDRPCEnabled.Get(&s.st.SV) {
2138+
if rpcbase.DRPCEnabled(ctx, s.st) {
21392139
if err := s.drpc.health(ctx); err != nil {
21402140
return err
21412141
}
@@ -2181,7 +2181,7 @@ func (s *systemAdminServer) checkReadinessForHealthCheck(ctx context.Context) er
21812181
return err
21822182
}
21832183

2184-
if rpcbase.ExperimentalDRPCEnabled.Get(&s.st.SV) {
2184+
if rpcbase.DRPCEnabled(ctx, s.st) {
21852185
if err := s.drpc.health(ctx); err != nil {
21862186
return err
21872187
}

pkg/server/application_api/main_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"os"
1010
"testing"
1111

12-
"github.com/cockroachdb/cockroach/pkg/base"
1312
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvtenant"
1413
"github.com/cockroachdb/cockroach/pkg/security/securityassets"
1514
"github.com/cockroachdb/cockroach/pkg/security/securitytest"
@@ -25,9 +24,6 @@ func TestMain(m *testing.M) {
2524
serverutils.InitTestClusterFactory(testcluster.TestClusterFactory)
2625
rangetestutils.InitRangeTestServerFactory(server.TestServerFactory)
2726
kvtenant.InitTestConnectorFactory()
28-
defer serverutils.TestingGlobalDRPCOption(
29-
base.TestDRPCEnabledRandomly,
30-
)()
3127
os.Exit(m.Run())
3228
}
3329

pkg/server/init.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -466,10 +466,8 @@ func (s *initServer) attemptJoinTo(
466466
BinaryVersion: &latestVersion,
467467
}
468468

469-
var initClient kvpb.RPCNodeClient
470-
if !rpcbase.TODODRPC {
471-
initClient = kvpb.NewGRPCInternalClientAdapter(conn)
472-
}
469+
// TODO(server): add support for DRPC initClient
470+
var initClient kvpb.RPCNodeClient = kvpb.NewGRPCInternalClientAdapter(conn)
473471
resp, err := initClient.Join(ctx, req)
474472
if err != nil {
475473
status, ok := grpcstatus.FromError(errors.UnwrapAll(err))

pkg/server/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2129,7 +2129,7 @@ func (s *topLevelServer) PreStart(ctx context.Context) error {
21292129
}
21302130
}
21312131
var apiInternalServer http.Handler
2132-
if rpcbase.ExperimentalDRPCEnabled.Get(&s.cfg.Settings.SV) {
2132+
if rpcbase.DRPCEnabled(ctx, s.cfg.Settings) {
21332133
// Pass our own node ID to connect to local RPC servers
21342134
apiInternalServer, err = apiinternal.NewAPIInternalServer(
21352135
ctx, s.kvNodeDialer, s.rpcContext.NodeID.Get(), s.cfg.Settings)

0 commit comments

Comments
 (0)