Skip to content

Commit

Permalink
Merge #96250
Browse files Browse the repository at this point in the history
96250: admin: fix nil panic when retrieving metric metadata r=knz a=dhartunian

Previously, the code that retrieved metric metadata assumed that the underlying node would have at least a single store configured in the `MetricRecorder` instance. This is not true on a SQL tenant since the storage node abstracts away all store-related information.

The code now checks to make sure that at least one store is found before moving ahead with retrieving store metric metadata.

Epic: CRDB-12100
Release note: None

Co-authored-by: David Hartunian <[email protected]>
  • Loading branch information
craig[bot] and dhartunian committed Feb 1, 2023
2 parents cc77415 + 9c114e2 commit 4162a72
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
13 changes: 13 additions & 0 deletions pkg/ccl/serverccl/adminccl/tenant_admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ func TestTenantAdminAPI(t *testing.T) {
t.Run("tenant_unimplemented", func(t *testing.T) {
testUnimplementedRPCs(ctx, t, testHelper)
})

t.Run("tenant_metricmetadata", func(t *testing.T) {
testMetricMetadataRPC(ctx, t, testHelper)
})
}

func testMetricMetadataRPC(ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper) {
http := helper.TestCluster().TenantAdminHTTPClient(t, 1)
defer http.Close()

metricMetadataResp := serverpb.MetricMetadataResponse{}
http.GetJSON("/_admin/v1/metricmetadata", &metricMetadataResp)
require.NotEmpty(t, metricMetadataResp.Metadata)
}

func testUnimplementedRPCs(ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper) {
Expand Down
6 changes: 5 additions & 1 deletion pkg/server/status/recorder.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,17 @@ func (mr *MetricsRecorder) GetMetricsMetadata() map[string]metric.Metadata {
// Get a random storeID.
var sID roachpb.StoreID

storeFound := false
for storeID := range mr.mu.storeRegistries {
sID = storeID
storeFound = true
break
}

// Get metric metadata from that store because all stores have the same metadata.
mr.mu.storeRegistries[sID].WriteMetricsMetadata(metrics)
if storeFound {
mr.mu.storeRegistries[sID].WriteMetricsMetadata(metrics)
}

return metrics
}
Expand Down

0 comments on commit 4162a72

Please sign in to comment.