Skip to content

Commit 4d076ec

Browse files
craig[bot]cthumuluru-crdbtbg
committed
148657: server: register `DistSQL`, `Tracing` service with DRPC server r=cthumuluru-crdb a=cthumuluru-crdb Enable `DistSQL` and `Tracing` services on the DRPC server in addition to gRPC. This is controlled by `rpc.experimental_drpc.enabled` (off by default). This change is part of a series and is similar to: #146926 Note: This only registers the service; the client is not updated to use the DRPC client, so this service will not have any functional effect. Epic: CRDB-48925 Release note: None 148658: roachtest: more CPU profiling in rebalancing tests r=tbg a=tbg Closes #147487. Co-authored-by: Chandra Thumuluru <[email protected]> Co-authored-by: Tobias Grieger <[email protected]>
3 parents 5775bac + 8d24172 + 8993e26 commit 4d076ec

File tree

5 files changed

+17
-2
lines changed

5 files changed

+17
-2
lines changed

pkg/cmd/roachtest/tests/rebalance_load.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ func registerRebalanceLoad(r registry.Registry) {
9797
settings.ClusterSettings["kv.allocator.load_based_rebalancing"] = rebalanceMode
9898
settings.ClusterSettings["kv.range_split.by_load_enabled"] = "false"
9999

100+
// Take a 10s profile every minute.
101+
settings.ClusterSettings["server.cpu_profile.duration"] = "10s"
102+
settings.ClusterSettings["server.cpu_profile.interval"] = "1m"
103+
settings.ClusterSettings["server.cpu_profile.cpu_usage_combined_threshold"] = "1" // basically always true
104+
settings.ClusterSettings["server.cpu_profile.total_dump_size_limit"] = "256 MiB"
105+
100106
if mixedVersion {
101107
mvt := mixedversion.NewTest(ctx, t, t.L(), c, roachNodes, mixedversion.NeverUseFixtures,
102108
mixedversion.ClusterSettingOption(
@@ -119,8 +125,6 @@ func registerRebalanceLoad(r registry.Registry) {
119125
})
120126
mvt.Run()
121127
} else {
122-
// Note that CPU profiling is already enabled by default, should there be
123-
// a failure it will be available in the artifacts.
124128
c.Start(ctx, t.L(), startOpts, settings, roachNodes)
125129
require.NoError(t, rebalanceByLoad(
126130
ctx, t, t.L(), c, maxDuration,

pkg/server/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ go_library(
375375
"@com_github_prometheus_client_model//go",
376376
"@com_github_prometheus_common//expfmt",
377377
"@in_gopkg_yaml_v2//:yaml_v2",
378+
"@io_storj_drpc//:drpc",
378379
"@io_storj_drpc//drpcerr",
379380
"@io_storj_drpc//drpcmigrate",
380381
"@org_golang_google_grpc//:grpc",

pkg/server/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,6 +1152,7 @@ func NewServer(cfg Config, stopper *stop.Stopper) (serverctl.ServerStartupInterf
11521152
nodeLiveness: optionalnodeliveness.MakeContainer(nodeLiveness),
11531153
gossip: gossip.MakeOptionalGossip(g),
11541154
grpcServer: grpcServer.Server,
1155+
drpcMux: drpcServer.DRPCServer,
11551156
nodeIDContainer: idContainer,
11561157
externalStorage: externalStorage,
11571158
externalStorageFromURI: externalStorageFromURI,

pkg/server/server_sql.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ import (
140140
"github.com/marusama/semaphore"
141141
"github.com/nightlyone/lockfile"
142142
"google.golang.org/grpc"
143+
"storj.io/drpc"
143144
)
144145

145146
// SQLServer encapsulates the part of a CRDB server that is dedicated to SQL
@@ -232,6 +233,7 @@ type sqlServerOptionalKVArgs struct {
232233
gossip gossip.OptionalGossip
233234
// To register blob and DistSQL servers.
234235
grpcServer *grpc.Server
236+
drpcMux drpc.Mux
235237
// For the temporaryObjectCleaner.
236238
isMeta1Leaseholder func(context.Context, hlc.ClockTimestamp) (bool, error)
237239
// DistSQL, lease management, and others want to know the node they're on.
@@ -555,6 +557,9 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
555557
// Create trace service for inter-node sharing of inflight trace spans.
556558
tracingService := service.New(cfg.Tracer)
557559
tracingservicepb.RegisterTracingServer(cfg.grpcServer, tracingService)
560+
if err := tracingservicepb.DRPCRegisterTracing(cfg.drpcMux, tracingService); err != nil {
561+
return nil, err
562+
}
558563

559564
// If the node id is already populated, we only need to create a placeholder
560565
// instance provider without initializing the instance, since this is not a
@@ -873,6 +878,9 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
873878

874879
distSQLServer := distsql.NewServer(ctx, distSQLCfg, cfg.remoteFlowRunner)
875880
execinfrapb.RegisterDistSQLServer(cfg.grpcServer, distSQLServer)
881+
if err := execinfrapb.DRPCRegisterDistSQL(cfg.drpcMux, distSQLServer.AsDRPCServer()); err != nil {
882+
return nil, err
883+
}
876884

877885
// Set up Executor
878886

pkg/server/tenant.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,6 +1335,7 @@ func makeTenantSQLServerArgs(
13351335
nodeLiveness: optionalnodeliveness.MakeContainer(nil),
13361336
gossip: gossip.MakeOptionalGossip(nil),
13371337
grpcServer: grpcServer.Server,
1338+
drpcMux: drpcServer.DRPCServer,
13381339
isMeta1Leaseholder: func(_ context.Context, _ hlc.ClockTimestamp) (bool, error) {
13391340
return false, errors.New("isMeta1Leaseholder is not available to secondary tenants")
13401341
},

0 commit comments

Comments
 (0)