Skip to content

Commit 2efb5da

Browse files
committed
server: always enable DRPC listener irrespective of the setting
Previously, the DRPC server was only started when the experimental DRPC setting was enabled. This created a problem where enabling DRPC after a node had already started would not work, as the listener would not be running and would reject all DRPC requests until the node was restarted. To address this, this patch always enables the DRPC listener regardless of the experimental setting. The cost of leaving the DRPC listener on all the time is low, and this change simplifies the codebase by removing the need to check cluster settings when creating clients. This could even help us avoid the need for a restart to enable DRPC entirely, if we can also figure out how to swap our gRPC clients for DRPC clients in various subsystems. Release note: None Epic: CRDB-52378
1 parent 2a6fdac commit 2efb5da

File tree

2 files changed

+11
-17
lines changed

2 files changed

+11
-17
lines changed

pkg/server/admin.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
3535
"github.com/cockroachdb/cockroach/pkg/roachpb"
3636
"github.com/cockroachdb/cockroach/pkg/rpc"
37+
"github.com/cockroachdb/cockroach/pkg/rpc/rpcbase"
3738
"github.com/cockroachdb/cockroach/pkg/security/username"
3839
"github.com/cockroachdb/cockroach/pkg/server/apiconstants"
3940
"github.com/cockroachdb/cockroach/pkg/server/authserver"
@@ -2134,7 +2135,7 @@ func (s *adminServer) checkReadinessForHealthCheck(ctx context.Context) error {
21342135
return err
21352136
}
21362137

2137-
if s.drpc.enabled {
2138+
if rpcbase.ExperimentalDRPCEnabled.Get(&s.st.SV) {
21382139
if err := s.drpc.health(ctx); err != nil {
21392140
return err
21402141
}
@@ -2180,7 +2181,7 @@ func (s *systemAdminServer) checkReadinessForHealthCheck(ctx context.Context) er
21802181
return err
21812182
}
21822183

2183-
if s.drpc.enabled {
2184+
if rpcbase.ExperimentalDRPCEnabled.Get(&s.st.SV) {
21842185
if err := s.drpc.health(ctx); err != nil {
21852186
return err
21862187
}

pkg/server/drpc_server.go

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

1212
"github.com/cockroachdb/cockroach/pkg/rpc"
13-
"github.com/cockroachdb/cockroach/pkg/rpc/rpcbase"
1413
"github.com/cockroachdb/cockroach/pkg/server/srverrors"
1514
"github.com/cockroachdb/errors"
1615
"google.golang.org/grpc/codes"
@@ -25,34 +24,28 @@ type drpcServer struct {
2524
serveModeHandler
2625
// Underlying DRPC server implementation.
2726
rpc.DRPCServer
28-
// Indicates if DRPC is enabled for this server.
29-
enabled bool
3027
// TLS configuration for secure connections.
3128
tlsCfg *tls.Config
3229
}
3330

3431
// newDRPCServer creates and configures a new drpcServer instance. It enables
3532
// DRPC if the experimental setting is on, otherwise returns a dummy server.
3633
func newDRPCServer(ctx context.Context, rpcCtx *rpc.Context) (*drpcServer, error) {
37-
drpcServer := &drpcServer{}
38-
if rpcbase.ExperimentalDRPCEnabled.Get(&rpcCtx.Settings.SV) {
39-
d, err := rpc.NewDRPCServer(ctx, rpcCtx)
40-
if err != nil {
41-
return nil, err
42-
}
43-
drpcServer.DRPCServer = d
44-
drpcServer.enabled = true
45-
} else {
46-
drpcServer.DRPCServer = rpc.NewDummyDRPCServer()
47-
drpcServer.enabled = false
34+
d, err := rpc.NewDRPCServer(ctx, rpcCtx)
35+
if err != nil {
36+
return nil, err
4837
}
4938

5039
tlsCfg, err := rpcCtx.GetServerTLSConfig()
5140
if err != nil {
5241
return nil, err
5342
}
5443

55-
drpcServer.tlsCfg = tlsCfg
44+
drpcServer := &drpcServer{
45+
DRPCServer: d,
46+
tlsCfg: tlsCfg,
47+
}
48+
5649
drpcServer.setMode(modeInitializing)
5750

5851
if err := rpc.DRPCRegisterHeartbeat(drpcServer, rpcCtx.NewHeartbeatService()); err != nil {

0 commit comments

Comments
 (0)