Skip to content

Commit 5dc1548

Browse files
rpc,server,testutils: move DRPC cluster setting to rpcbase
Epic: CRDB-48935 Informs: None Release note: None
1 parent 760a77e commit 5dc1548

File tree

10 files changed

+40
-43
lines changed

10 files changed

+40
-43
lines changed

pkg/rpc/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ go_library(
2020
"keepalive.go",
2121
"metrics.go",
2222
"peer.go",
23-
"peer_drpc.go",
2423
"peer_map.go",
2524
"restricted_internal_client.go",
2625
"settings.go",

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 !rpc.ExperimentalDRPCEnabled.Get(&n.rpcContext.Settings.SV) {
174+
if !rpcbase.ExperimentalDRPCEnabled.Get(&n.rpcContext.Settings.SV) {
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/peer_drpc.go

Lines changed: 0 additions & 34 deletions
This file was deleted.

pkg/rpc/rpcbase/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ go_library(
2525
"//pkg/keys",
2626
"//pkg/roachpb",
2727
"//pkg/rpc/rpcpb",
28+
"//pkg/settings",
29+
"//pkg/util/buildutil",
2830
"//pkg/util/envutil",
31+
"@com_github_cockroachdb_errors//:errors",
2932
"@io_storj_drpc//:drpc",
3033
"@org_golang_google_grpc//:grpc",
3134
],

pkg/rpc/rpcbase/nodedialer.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,35 @@ import (
99
"context"
1010

1111
"github.com/cockroachdb/cockroach/pkg/roachpb"
12+
"github.com/cockroachdb/cockroach/pkg/settings"
13+
"github.com/cockroachdb/cockroach/pkg/util/buildutil"
14+
"github.com/cockroachdb/cockroach/pkg/util/envutil"
15+
"github.com/cockroachdb/errors"
1216
"google.golang.org/grpc"
1317
"storj.io/drpc"
1418
)
1519

20+
var envExperimentalDRPCEnabled = envutil.EnvOrDefaultBool("COCKROACH_EXPERIMENTAL_DRPC_ENABLED", false)
21+
22+
// ExperimentalDRPCEnabled determines whether a drpc server accepting BatchRequest
23+
// is enabled. This server is experimental and completely unsuitable to production
24+
// usage (for example, does not implement authorization checks).
25+
var ExperimentalDRPCEnabled = settings.RegisterBoolSetting(
26+
settings.ApplicationLevel,
27+
"rpc.experimental_drpc.enabled",
28+
"if true, use drpc to execute Batch RPCs (instead of gRPC)",
29+
envExperimentalDRPCEnabled,
30+
settings.WithValidateBool(func(values *settings.Values, b bool) error {
31+
// drpc support is highly experimental and should not be enabled in production.
32+
// Since authorization is not implemented, we only even host the server if the
33+
// env var is set or it's a CRDB test build. Consequently, these are prereqs
34+
// for setting the cluster setting.
35+
if b && !(envExperimentalDRPCEnabled || buildutil.CrdbTestBuild) {
36+
return errors.New("experimental drpc is not allowed in this environment")
37+
}
38+
return nil
39+
}))
40+
1641
// TODODRPC is a marker to identify each RPC client creation site that needs to
1742
// be updated to support DRPC.
1843
const TODODRPC = false

pkg/server/drpc_server.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"crypto/tls"
1111

1212
"github.com/cockroachdb/cockroach/pkg/rpc"
13+
"github.com/cockroachdb/cockroach/pkg/rpc/rpcbase"
1314
"github.com/cockroachdb/cockroach/pkg/server/srverrors"
1415
"github.com/cockroachdb/errors"
1516
"google.golang.org/grpc/codes"
@@ -34,7 +35,7 @@ type drpcServer struct {
3435
// DRPC if the experimental setting is on, otherwise returns a dummy server.
3536
func newDRPCServer(ctx context.Context, rpcCtx *rpc.Context) (*drpcServer, error) {
3637
drpcServer := &drpcServer{}
37-
if rpc.ExperimentalDRPCEnabled.Get(&rpcCtx.Settings.SV) {
38+
if rpcbase.ExperimentalDRPCEnabled.Get(&rpcCtx.Settings.SV) {
3839
d, err := rpc.NewDRPCServer(ctx, rpcCtx)
3940
if err != nil {
4041
return nil, err

pkg/server/drpc_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func TestDRPCBatchServer(t *testing.T) {
4343
args.ServerArgs.Insecure = insecure
4444
args.ReplicationMode = base.ReplicationManual
4545
args.ServerArgs.Settings = cluster.MakeClusterSettings()
46-
rpc.ExperimentalDRPCEnabled.Override(ctx, &args.ServerArgs.Settings.SV, true)
46+
rpcbase.ExperimentalDRPCEnabled.Override(ctx, &args.ServerArgs.Settings.SV, true)
4747
c := testcluster.StartTestCluster(t, numNodes, args)
4848
defer c.Stopper().Stop(ctx)
4949

@@ -99,7 +99,7 @@ func TestStreamContextCancel(t *testing.T) {
9999
}
100100

101101
ctx := context.Background()
102-
rpc.ExperimentalDRPCEnabled.Override(ctx, &args.ServerArgs.Settings.SV, true)
102+
rpcbase.ExperimentalDRPCEnabled.Override(ctx, &args.ServerArgs.Settings.SV, true)
103103
c := testcluster.StartTestCluster(t, numNodes, args)
104104
defer c.Stopper().Stop(ctx)
105105

@@ -225,6 +225,7 @@ func TestDialDRPC_InterceptorsAreSet(t *testing.T) {
225225
},
226226
}
227227

228+
rpcbase.ExperimentalDRPCEnabled.Override(ctx, &args.ServerArgs.Settings.SV, true)
228229
c := testcluster.StartTestCluster(t, numNodes, args)
229230
defer c.Stopper().Stop(ctx)
230231
rpcAddr := c.Server(0).RPCAddr()

pkg/server/server_drpc_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"testing"
1212

1313
"github.com/cockroachdb/cockroach/pkg/base"
14-
"github.com/cockroachdb/cockroach/pkg/rpc"
14+
"github.com/cockroachdb/cockroach/pkg/rpc/rpcbase"
1515
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
1616
"github.com/cockroachdb/cockroach/pkg/testutils"
1717
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
@@ -29,7 +29,7 @@ func TestDRPCSelectQuery(t *testing.T) {
2929
defer cancel()
3030

3131
st := cluster.MakeTestingClusterSettings()
32-
rpc.ExperimentalDRPCEnabled.Override(ctx, &st.SV, true)
32+
rpcbase.ExperimentalDRPCEnabled.Override(ctx, &st.SV, true)
3333

3434
tc := serverutils.StartCluster(t, 3, base.TestClusterArgs{
3535
ServerArgs: base.TestServerArgs{

pkg/server/start_listen.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/cockroachdb/cmux"
1616
"github.com/cockroachdb/cockroach/pkg/rpc"
17+
"github.com/cockroachdb/cockroach/pkg/rpc/rpcbase"
1718
"github.com/cockroachdb/cockroach/pkg/sql/pgwire"
1819
"github.com/cockroachdb/cockroach/pkg/util/log"
1920
"github.com/cockroachdb/cockroach/pkg/util/netutil"
@@ -137,8 +138,8 @@ func startListenRPCAndSQL(
137138
// Host drpc only if it's _possible_ to turn it on (this requires a test build
138139
// or env var). If the setting _is_ on, then it was overridden in testing and
139140
// we want to host the server too.
140-
hostDRPC := rpc.ExperimentalDRPCEnabled.Validate(nil /* not used */, true) == nil ||
141-
rpc.ExperimentalDRPCEnabled.Get(&cfg.Settings.SV)
141+
hostDRPC := rpcbase.ExperimentalDRPCEnabled.Validate(nil /* not used */, true) == nil ||
142+
rpcbase.ExperimentalDRPCEnabled.Get(&cfg.Settings.SV)
142143

143144
// If we're not hosting drpc, make a listener that never accepts anything.
144145
// We will start the dRPC server all the same; it barely consumes any

pkg/testutils/serverutils/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ go_library(
3131
"//pkg/multitenant/tenantcapabilitiespb",
3232
"//pkg/roachpb",
3333
"//pkg/rpc",
34+
"//pkg/rpc/rpcbase",
3435
"//pkg/security/securitytest",
3536
"//pkg/security/username",
3637
"//pkg/server/decommissioning",

0 commit comments

Comments
 (0)