diff --git a/DEPS.bzl b/DEPS.bzl index 9b6a33e2a8a6..3b0168c6b511 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -1565,10 +1565,10 @@ def go_deps(): patches = [ "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch", ], - sha256 = "e58192b93c07d773152894f78a9b47808a4d90dc1649f53949b0f4eafa58ce2a", - strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230614133735-32834aa62738", + sha256 = "2379e7d32e1579131ada504c332c8e938d85c2048fa57684a84a8400f123a844", + strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230615160155-e9a280333ef7", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230614133735-32834aa62738.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230615160155-e9a280333ef7.zip", ], ) go_repository( diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl index 7b783d15a691..1b49c941be1c 100644 --- a/build/bazelutil/distdir_files.bzl +++ b/build/bazelutil/distdir_files.bzl @@ -313,7 +313,7 @@ DISTDIR_FILES = { "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/go-test-teamcity/com_github_cockroachdb_go_test_teamcity-v0.0.0-20191211140407-cff980ad0a55.zip": "bac30148e525b79d004da84d16453ddd2d5cd20528e9187f1d7dac708335674b", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230614133735-32834aa62738.zip": "e58192b93c07d773152894f78a9b47808a4d90dc1649f53949b0f4eafa58ce2a", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230615160155-e9a280333ef7.zip": "2379e7d32e1579131ada504c332c8e938d85c2048fa57684a84a8400f123a844", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9", diff --git a/go.mod b/go.mod index 81394041a539..53c8e5563336 100644 --- a/go.mod +++ b/go.mod @@ -116,7 +116,7 @@ require ( github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55 github.com/cockroachdb/gostdlib v1.19.0 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b - github.com/cockroachdb/pebble v0.0.0-20230614133735-32834aa62738 + github.com/cockroachdb/pebble v0.0.0-20230615160155-e9a280333ef7 github.com/cockroachdb/redact v1.1.5 github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b diff --git a/go.sum b/go.sum index 9669b6d7ac01..979a5817a34b 100644 --- a/go.sum +++ b/go.sum @@ -486,8 +486,8 @@ github.com/cockroachdb/gostdlib v1.19.0/go.mod h1:+dqqpARXbE/gRDEhCak6dm0l14AaTy github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs= github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZeQy818SGhaone5OnYfxFR/+AzdY3sf5aE= github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs= -github.com/cockroachdb/pebble v0.0.0-20230614133735-32834aa62738 h1:pKUZ8PCP23se600eVZs1HM7zwipLqWAKldxeWfgDGCk= -github.com/cockroachdb/pebble v0.0.0-20230614133735-32834aa62738/go.mod h1:ZoLpjYdSY0BC1it7VZT20L4tm0OhBawe5awAtC0S5Vk= +github.com/cockroachdb/pebble v0.0.0-20230615160155-e9a280333ef7 h1:/pKQG5yaM44AY8ODkUToGfn/6hpXq5Qb8KWIUzbjWcU= +github.com/cockroachdb/pebble v0.0.0-20230615160155-e9a280333ef7/go.mod h1:ZoLpjYdSY0BC1it7VZT20L4tm0OhBawe5awAtC0S5Vk= github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg= github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30= github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg= diff --git a/pkg/ccl/logictestccl/testdata/logic_test/changefeed b/pkg/ccl/logictestccl/testdata/logic_test/changefeed index bec3b0165132..e0a07c061eb2 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/changefeed +++ b/pkg/ccl/logictestccl/testdata/logic_test/changefeed @@ -1,4 +1,4 @@ -# LogicTest: local +# LogicTest: local local-mixed-22.2-23.1 statement ok CREATE TABLE t () @@ -49,3 +49,19 @@ user testuser statement error user testuser requires the CHANGEFEED privilege on all target tables to be able to run an enterprise changefeed CREATE CHANGEFEED FOR t INTO 'null://sink' with initial_scan='only' + + +user root + +let $job_id +SELECT job_id FROM [SHOW CHANGEFEED JOBS] WHERE user_name = 'testuser' + +query TT +SELECT user_name, description FROM [SHOW CHANGEFEED JOB $job_id] +---- +testuser CREATE CHANGEFEED FOR TABLE t INTO 'null://sink' WITH initial_scan = 'only' + +query TT +SELECT user_name, description FROM [SHOW CHANGEFEED JOBS] +---- +testuser CREATE CHANGEFEED FOR TABLE t INTO 'null://sink' WITH initial_scan = 'only' \ No newline at end of file diff --git a/pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1/BUILD.bazel b/pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1/BUILD.bazel index af9376cf8f9f..43abeb591264 100644 --- a/pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1/BUILD.bazel +++ b/pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1/BUILD.bazel @@ -13,7 +13,7 @@ go_test( "//c-deps:libgeos", # keep "//pkg/ccl/logictestccl:testdata", # keep ], - shard_count = 5, + shard_count = 6, tags = [ "ccl_test", "cpu:1", diff --git a/pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1/generated_test.go b/pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1/generated_test.go index da6488a41bb7..f2f9544a15d4 100644 --- a/pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1/generated_test.go +++ b/pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1/generated_test.go @@ -72,6 +72,13 @@ func TestLogic_tmp(t *testing.T) { logictest.RunLogicTests(t, logictest.TestServerArgs{}, configIdx, glob) } +func TestCCLLogic_changefeed( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runCCLLogicTest(t, "changefeed") +} + func TestCCLLogic_new_schema_changer( t *testing.T, ) { diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go index b362d56a015d..21e041573bff 100644 --- a/pkg/cmd/roachtest/test_runner.go +++ b/pkg/cmd/roachtest/test_runner.go @@ -427,6 +427,8 @@ func defaultClusterAllocator( lopt.l.PrintfCtx(ctx, "Attaching to existing cluster %s for test %s", existingClusterName, t.Name) c, err := attachToExistingCluster(ctx, existingClusterName, clusterL, t.Cluster, opt, r.cr) if err == nil { + // Pretend pre-existing's cluster architecture matches the desired one; see the above TODO wrt validation. + c.arch = arch return c, nil, nil } if !errors.Is(err, errClusterNotFound) { @@ -632,7 +634,7 @@ func (r *testRunner) runWorker( // N.B. FIPS is only supported on 'amd64' at this time. arch = vm.ArchFIPS } - if testToRun.spec.Benchmark { + if testToRun.spec.Benchmark && testToRun.spec.Cluster.Cloud != spec.Local { // TODO(srosenberg): enable after https://github.com/cockroachdb/cockroach/issues/104213 l.PrintfCtx(ctx, "Disabling randomly chosen arch=%q, %s", arch, testToRun.spec.Name) arch = vm.ArchAMD64 @@ -645,7 +647,7 @@ func (r *testRunner) runWorker( if testToRun.canReuseCluster && c != nil && c.arch != arch { // Non-local cluster that's being reused must have the same architecture as was ensured above. if c.spec.Cloud != spec.Local { - return errors.New("infeasible path: non-local cluster arch mismatch") + return errors.Newf("infeasible path: non-local cluster arch=%q differs from selected arch=%q", c.arch, arch) } // Local cluster is now reused to emulate a different CPU architecture. c.arch = arch diff --git a/pkg/roachpb/span_stats.go b/pkg/roachpb/span_stats.go index 3518820bb9e5..fe2a0f48cde4 100644 --- a/pkg/roachpb/span_stats.go +++ b/pkg/roachpb/span_stats.go @@ -10,7 +10,11 @@ package roachpb -import "github.com/cockroachdb/cockroach/pkg/settings" +import ( + "fmt" + + "github.com/cockroachdb/cockroach/pkg/settings" +) // Put span statistics cluster settings here to avoid import cycle. @@ -37,3 +41,18 @@ var RangeStatsBatchLimit = settings.RegisterIntSetting( defaultRangeStatsBatchLimit, settings.PositiveInt, ) + +// RangeDescPageSize controls the page size when iterating through range +// descriptors. +var RangeDescPageSize = settings.RegisterIntSetting( + settings.TenantWritable, + "server.span_stats.range_desc_page_size", + "the page size when iterating through range descriptors", + 100, + func(i int64) error { + if i < 5 || i > 25000 { + return fmt.Errorf("expected range_desc_page_size to be in range [5, 25000], got %d", i) + } + return nil + }, +) diff --git a/pkg/server/admin.go b/pkg/server/admin.go index 94f42e75e24d..92832e11188c 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -1368,8 +1368,9 @@ func (s *adminServer) statsForSpan( return nil, err } - // Get a list of node ids and range count for the specified span. - nodeIDs, rangeCount, err := nodeIDsAndRangeCountForSpan( + // Get a list of nodeIDs, range counts, and replica counts per node + // for the specified span. + nodeIDs, rangeCount, replCounts, err := getNodeIDsRangeCountReplCountForSpan( ctx, s.distSender, rSpan, ) if err != nil { @@ -1439,6 +1440,15 @@ func (s *adminServer) statsForSpan( return nil, err } } + + // The semantics of tableStatResponse.ReplicaCount counts replicas + // found for this span returned by a cluster-wide fan-out. + // We can use descriptors to know what the final count _should_ be, + // if we assume every request succeeds (nodes and replicas are reachable). + for _, replCount := range replCounts { + tableStatResponse.ReplicaCount += replCount + } + for remainingResponses := len(nodeIDs); remainingResponses > 0; remainingResponses-- { select { case resp := <-responses: @@ -1449,6 +1459,10 @@ func (s *adminServer) statsForSpan( return nil, serverError(ctx, resp.err) } + // If this node is unreachable, + // it's replicas can not be counted. + tableStatResponse.ReplicaCount -= replCounts[resp.nodeID] + tableStatResponse.MissingNodes = append( tableStatResponse.MissingNodes, serverpb.TableStatsResponse_MissingNode{ @@ -1458,7 +1472,6 @@ func (s *adminServer) statsForSpan( ) } else { tableStatResponse.Stats.Add(resp.resp.SpanToStats[span.String()].TotalStats) - tableStatResponse.ReplicaCount += int64(resp.resp.SpanToStats[span.String()].RangeCount) tableStatResponse.ApproximateDiskBytes += resp.resp.SpanToStats[span.String()].ApproximateDiskBytes } case <-ctx.Done(): @@ -1470,16 +1483,19 @@ func (s *adminServer) statsForSpan( return &tableStatResponse, nil } -// Returns the list of node ids for the specified span. -func nodeIDsAndRangeCountForSpan( +// Returns the list of node ids, range count, +// and replica count for the specified span. +func getNodeIDsRangeCountReplCountForSpan( ctx context.Context, ds *kvcoord.DistSender, rSpan roachpb.RSpan, -) (nodeIDList []roachpb.NodeID, rangeCount int64, _ error) { +) (nodeIDList []roachpb.NodeID, rangeCount int64, replCounts map[roachpb.NodeID]int64, _ error) { nodeIDs := make(map[roachpb.NodeID]struct{}) + replCountForNodeID := make(map[roachpb.NodeID]int64) ri := kvcoord.MakeRangeIterator(ds) ri.Seek(ctx, rSpan.Key, kvcoord.Ascending) for ; ri.Valid(); ri.Next(ctx) { rangeCount++ for _, repl := range ri.Desc().Replicas().Descriptors() { + replCountForNodeID[repl.NodeID]++ nodeIDs[repl.NodeID] = struct{}{} } if !ri.NeedAnother(rSpan) { @@ -1487,7 +1503,7 @@ func nodeIDsAndRangeCountForSpan( } } if err := ri.Error(); err != nil { - return nil, 0, err + return nil, 0, nil, err } nodeIDList = make([]roachpb.NodeID, 0, len(nodeIDs)) @@ -1497,7 +1513,7 @@ func nodeIDsAndRangeCountForSpan( sort.Slice(nodeIDList, func(i, j int) bool { return nodeIDList[i] < nodeIDList[j] }) - return nodeIDList, rangeCount, nil + return nodeIDList, rangeCount, replCountForNodeID, nil } // Users returns a list of users, stripped of any passwords. diff --git a/pkg/server/server.go b/pkg/server/server.go index aba8e51157bc..f54bb5ca1108 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -998,7 +998,6 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) { serverIterator, spanConfig.reporter, clock, - distSender, rangestats.NewFetcher(db), node, ) diff --git a/pkg/server/span_stats_server.go b/pkg/server/span_stats_server.go index 6403f62f4f0d..7d7bd42fda24 100644 --- a/pkg/server/span_stats_server.go +++ b/pkg/server/span_stats_server.go @@ -15,13 +15,14 @@ import ( "strconv" "github.com/cockroachdb/cockroach/pkg/keys" - "github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord" + "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangestats" "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/server/serverpb" "github.com/cockroachdb/cockroach/pkg/storage" "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/cockroach/pkg/util/rangedesc" "github.com/cockroachdb/cockroach/pkg/util/timeutil" ) @@ -39,7 +40,7 @@ func (s *systemStatusServer) spanStatsFanOut( // Response level error var respErr error - spansPerNode, err := s.getSpansPerNode(ctx, req, s.distSender) + spansPerNode, err := s.getSpansPerNode(ctx, req) if err != nil { return nil, err } @@ -86,7 +87,6 @@ func (s *systemStatusServer) spanStatsFanOut( } else { res.SpanToStats[spanStr].ApproximateDiskBytes += spanStats.ApproximateDiskBytes res.SpanToStats[spanStr].TotalStats.Add(spanStats.TotalStats) - res.SpanToStats[spanStr].RangeCount += spanStats.RangeCount } } } @@ -114,17 +114,8 @@ func (s *systemStatusServer) getLocalStats( ctx context.Context, req *roachpb.SpanStatsRequest, ) (*roachpb.SpanStatsResponse, error) { var res = &roachpb.SpanStatsResponse{SpanToStats: make(map[string]*roachpb.SpanStats)} - ri := kvcoord.MakeRangeIterator(s.distSender) - - // For each span for _, span := range req.Spans { - rSpan, err := keys.SpanAddr(span) - if err != nil { - return nil, err - } - // Seek to the span's start key. - ri.Seek(ctx, rSpan.Key, kvcoord.Ascending) - spanStats, err := s.statsForSpan(ctx, ri, rSpan) + spanStats, err := s.statsForSpan(ctx, span) if err != nil { return nil, err } @@ -134,21 +125,35 @@ func (s *systemStatusServer) getLocalStats( } func (s *systemStatusServer) statsForSpan( - ctx context.Context, ri kvcoord.RangeIterator, rSpan roachpb.RSpan, + ctx context.Context, span roachpb.Span, ) (*roachpb.SpanStats, error) { - // Seek to the span's start key. - ri.Seek(ctx, rSpan.Key, kvcoord.Ascending) + + var descriptors []roachpb.RangeDescriptor + scanner := rangedesc.NewScanner(s.db) + pageSize := int(roachpb.RangeDescPageSize.Get(&s.st.SV)) + err := scanner.Scan(ctx, pageSize, func() { + // If the underlying txn fails and needs to be retried, + // clear the descriptors we've collected so far. + descriptors = nil + }, span, func(scanned ...roachpb.RangeDescriptor) error { + descriptors = append(descriptors, scanned...) + return nil + }) + + if err != nil { + return nil, err + } + spanStats := &roachpb.SpanStats{} var fullyContainedKeysBatch []roachpb.Key - var err error + rSpan, err := keys.SpanAddr(span) + if err != nil { + return nil, err + } // Iterate through the span's ranges. - for { - if !ri.Valid() { - return nil, ri.Error() - } + for _, desc := range descriptors { // Get the descriptor for the current range of the span. - desc := ri.Desc() descSpan := desc.RSpan() spanStats.RangeCount += 1 @@ -200,11 +205,6 @@ func (s *systemStatusServer) statsForSpan( return nil, err } } - - if !ri.NeedAnother(rSpan) { - break - } - ri.Next(ctx) } // If we still have some remaining ranges, request range stats for the current batch. if len(fullyContainedKeysBatch) > 0 { @@ -270,19 +270,13 @@ func (s *systemStatusServer) getSpanStatsInternal( } func (s *systemStatusServer) getSpansPerNode( - ctx context.Context, req *roachpb.SpanStatsRequest, ds *kvcoord.DistSender, + ctx context.Context, req *roachpb.SpanStatsRequest, ) (map[roachpb.NodeID][]roachpb.Span, error) { // Mapping of node ids to spans with a replica on the node. spansPerNode := make(map[roachpb.NodeID][]roachpb.Span) - - // Iterate over the request spans. + pageSize := int(roachpb.RangeDescPageSize.Get(&s.st.SV)) for _, span := range req.Spans { - rSpan, err := keys.SpanAddr(span) - if err != nil { - return nil, err - } - // Get the node ids belonging to the span. - nodeIDs, _, err := nodeIDsAndRangeCountForSpan(ctx, ds, rSpan) + nodeIDs, err := nodeIDsForSpan(ctx, s.db, span, pageSize) if err != nil { return nil, err } @@ -294,6 +288,37 @@ func (s *systemStatusServer) getSpansPerNode( return spansPerNode, nil } +// nodeIDsForSpan returns a list of nodeIDs that contain at least one replica +// for the argument span. Descriptors are found via ScanMetaKVs. +func nodeIDsForSpan( + ctx context.Context, db *kv.DB, span roachpb.Span, pageSize int, +) ([]roachpb.NodeID, error) { + nodeIDs := make(map[roachpb.NodeID]struct{}) + scanner := rangedesc.NewScanner(db) + err := scanner.Scan(ctx, pageSize, func() { + // If the underlying txn fails and needs to be retried, + // clear the nodeIDs we've collected so far. + nodeIDs = map[roachpb.NodeID]struct{}{} + }, span, func(scanned ...roachpb.RangeDescriptor) error { + for _, desc := range scanned { + for _, repl := range desc.Replicas().Descriptors() { + nodeIDs[repl.NodeID] = struct{}{} + } + } + return nil + }) + if err != nil { + return nil, err + } + + nodeIDList := make([]roachpb.NodeID, 0, len(nodeIDs)) + for id := range nodeIDs { + nodeIDList = append(nodeIDList, id) + } + + return nodeIDList, nil +} + func flushBatchedContainedKeys( ctx context.Context, fetcher *rangestats.Fetcher, diff --git a/pkg/server/span_stats_test.go b/pkg/server/span_stats_test.go index 2d5ab3b3ba56..b66eec552655 100644 --- a/pkg/server/span_stats_test.go +++ b/pkg/server/span_stats_test.go @@ -8,41 +8,79 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -package server +package server_test import ( "bytes" "context" "fmt" - "strconv" "testing" "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/keys" - "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/server" "github.com/cockroachdb/cockroach/pkg/server/serverpb" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" "github.com/stretchr/testify/require" ) -func TestLocalSpanStats(t *testing.T) { +func TestSpanStatsMetaScan(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - ctx := context.Background() - serv, _, _ := serverutils.StartServer(t, base.TestServerArgs{ - Knobs: base.TestingKnobs{ - Store: &kvserver.StoreTestingKnobs{DisableCanAckBeforeApplication: true}, + testCluster := serverutils.StartNewTestCluster(t, 3, base.TestClusterArgs{}) + defer testCluster.Stopper().Stop(context.Background()) + s := testCluster.Server(0) + + testSpans := []roachpb.Span{ + { + Key: keys.Meta1Prefix, + EndKey: keys.Meta1KeyMax, }, - }) - s := serv.(*TestServer) - defer s.Stopper().Stop(ctx) + { + Key: keys.LocalMax, + EndKey: keys.Meta2KeyMax, + }, + { + Key: keys.Meta2Prefix, + EndKey: keys.Meta2KeyMax, + }, + } + + // SpanStats should have no problem finding descriptors for + // spans up to and including Meta2KeyMax. + for _, span := range testSpans { + res, err := s.StatusServer().(serverpb.StatusServer).SpanStats(ctx, + &roachpb.SpanStatsRequest{ + NodeID: "0", + Spans: []roachpb.Span{ + span, + }, + }, + ) + require.NoError(t, err) + require.Equal(t, int32(1), res.SpanToStats[span.String()].RangeCount) + } +} + +func TestLocalSpanStats(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + skip.UnderStress(t) + ctx := context.Background() + numNodes := 3 + tc := testcluster.StartTestCluster(t, numNodes, base.TestClusterArgs{}) + defer tc.Stopper().Stop(ctx) + + s := tc.Server(0).(*server.TestServer) + store, err := s.Stores().GetStore(s.GetFirstStoreID()) require.NoError(t, err) // Create a number of ranges using splits. @@ -104,15 +142,15 @@ func TestLocalSpanStats(t *testing.T) { } testCases := []testCase{ - {spans[0], 4, 6}, - {spans[1], 1, 3}, - {spans[2], 2, 5}, - {spans[3], 2, 1}, - {spans[4], 2, 3}, - {spans[5], 1, 2}, + {spans[0], 4, int64(numNodes * 6)}, + {spans[1], 1, int64(numNodes * 3)}, + {spans[2], 2, int64(numNodes * 5)}, + {spans[3], 2, int64(numNodes * 1)}, + {spans[4], 2, int64(numNodes * 3)}, + {spans[5], 1, int64(numNodes * 2)}, } // Multi-span request - multiResult, err := s.status.getLocalStats(ctx, + multiResult, err := s.StatusServer().(serverpb.StatusServer).SpanStats(ctx, &roachpb.SpanStatsRequest{ NodeID: "0", Spans: spans, @@ -127,10 +165,30 @@ func TestLocalSpanStats(t *testing.T) { // Assert expected values from multi-span request spanStats := multiResult.SpanToStats[tcase.span.String()] - require.Equal(t, spanStats.RangeCount, tcase.expectedRanges, fmt.Sprintf( - "Multi-span: expected %d ranges in span [%s - %s], found %d", tcase.expectedRanges, rSpan.Key.String(), rSpan.EndKey.String(), spanStats.RangeCount)) - require.Equal(t, spanStats.TotalStats.LiveCount, tcase.expectedKeys, fmt.Sprintf( - "Multi-span: expected %d keys in span [%s - %s], found %d", tcase.expectedKeys, rSpan.Key.String(), rSpan.EndKey.String(), spanStats.TotalStats.LiveCount)) + require.Equal( + t, + tcase.expectedRanges, + spanStats.RangeCount, + fmt.Sprintf( + "Multi-span: expected %d ranges in span [%s - %s], found %d", + tcase.expectedRanges, + rSpan.Key.String(), + rSpan.EndKey.String(), + spanStats.RangeCount, + ), + ) + require.Equal( + t, + tcase.expectedKeys, + spanStats.TotalStats.LiveCount, + fmt.Sprintf( + "Multi-span: expected %d keys in span [%s - %s], found %d", + tcase.expectedKeys, + rSpan.Key.String(), + rSpan.EndKey.String(), + spanStats.TotalStats.LiveCount, + ), + ) } } @@ -193,22 +251,24 @@ func BenchmarkSpanStats(b *testing.B) { var spans []roachpb.Span // Create a table spans - var spanStartKey roachpb.Key for i := 0; i < ts.numSpans; i++ { - spanStartKey = nil + spanStartKey := makeKey( + tenantPrefix, + []byte{byte(i)}, + ) + spanEndKey := makeKey( + tenantPrefix, + []byte{byte(i + 1)}, + ) // Create ranges. - var key roachpb.Key for j := 0; j < ts.numRanges; j++ { - key = makeKey(tenantPrefix, []byte(strconv.Itoa(i*j))) - if spanStartKey == nil { - spanStartKey = key - } - _, _, err := tc.Server(0).SplitRange(key) + splitKey := makeKey(spanStartKey, []byte{byte(j)}) + _, _, err := tc.Server(0).SplitRange(splitKey) require.NoError(b, err) } spans = append(spans, roachpb.Span{ Key: spanStartKey, - EndKey: key, + EndKey: spanEndKey, }) } diff --git a/pkg/server/status.go b/pkg/server/status.go index 51106754f466..f1aef2a2b6f9 100644 --- a/pkg/server/status.go +++ b/pkg/server/status.go @@ -40,7 +40,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/keyvisualizer/keyvisstorage" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/kv/kvclient" - "github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord" "github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangestats" "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/storepool" @@ -509,7 +508,6 @@ type systemStatusServer struct { stores *kvserver.Stores nodeLiveness *liveness.NodeLiveness spanConfigReporter spanconfig.Reporter - distSender *kvcoord.DistSender rangeStatsFetcher *rangestats.Fetcher node *Node } @@ -619,7 +617,6 @@ func newSystemStatusServer( serverIterator ServerIterator, spanConfigReporter spanconfig.Reporter, clock *hlc.Clock, - distSender *kvcoord.DistSender, rangeStatsFetcher *rangestats.Fetcher, node *Node, ) *systemStatusServer { @@ -647,7 +644,6 @@ func newSystemStatusServer( stores: stores, nodeLiveness: nodeLiveness, spanConfigReporter: spanConfigReporter, - distSender: distSender, rangeStatsFetcher: rangeStatsFetcher, node: node, } diff --git a/pkg/sql/delegate/BUILD.bazel b/pkg/sql/delegate/BUILD.bazel index 3a61a3f4a804..f1081f2c0d0c 100644 --- a/pkg/sql/delegate/BUILD.bazel +++ b/pkg/sql/delegate/BUILD.bazel @@ -41,6 +41,7 @@ go_library( importpath = "github.com/cockroachdb/cockroach/pkg/sql/delegate", visibility = ["//visibility:public"], deps = [ + "//pkg/clusterversion", "//pkg/jobs", "//pkg/jobs/jobspb", "//pkg/security/username", diff --git a/pkg/sql/delegate/show_changefeed_jobs.go b/pkg/sql/delegate/show_changefeed_jobs.go index 504c24b571cc..7a1f91aad205 100644 --- a/pkg/sql/delegate/show_changefeed_jobs.go +++ b/pkg/sql/delegate/show_changefeed_jobs.go @@ -13,6 +13,8 @@ package delegate import ( "fmt" + "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" ) @@ -23,7 +25,15 @@ func (d *delegator) delegateShowChangefeedJobs(n *tree.ShowChangefeedJobs) (tree // Note: changefeed_details may contain sensitive credentials in sink_uri. This information is redacted when marshaling // to JSON in ChangefeedDetails.MarshalJSONPB. const ( - selectClause = ` + // In 23.1, we can use the job_type column to filter jobs. + queryTarget23_1 = ` + crdb_internal.system_jobs + WHERE job_type = 'CHANGEFEED' + ` + queryTargetPre23_1 = ` + system.jobs + ` + baseSelectClause = ` WITH payload AS ( SELECT id, @@ -32,8 +42,7 @@ WITH payload AS ( payload, false, true )->'changefeed' AS changefeed_details FROM - crdb_internal.system_jobs - WHERE job_type = 'CHANGEFEED' + %s ) SELECT job_id, @@ -69,6 +78,15 @@ FROM INNER JOIN payload ON id = job_id` ) + use23_1 := d.evalCtx.Settings.Version.IsActive(d.ctx, clusterversion.V23_1BackfillTypeColumnInJobsTable) + + var selectClause string + if use23_1 { + selectClause = fmt.Sprintf(baseSelectClause, queryTarget23_1) + } else { + selectClause = fmt.Sprintf(baseSelectClause, queryTargetPre23_1) + } + var whereClause, orderbyClause string if n.Jobs == nil { // The query intends to present: @@ -78,9 +96,17 @@ FROM // The "ORDER BY" clause below exploits the fact that all // running jobs have finished = NULL. orderbyClause = `ORDER BY COALESCE(finished, now()) DESC, started DESC` + if !use23_1 { + whereClause = fmt.Sprintf("WHERE job_type = '%s'", jobspb.TypeChangefeed) + } } else { // Limit the jobs displayed to the select statement in n.Jobs. - whereClause = fmt.Sprintf(`WHERE job_id in (%s)`, n.Jobs.String()) + if use23_1 { + whereClause = fmt.Sprintf(`WHERE job_id in (%s)`, n.Jobs.String()) + } else { + whereClause = fmt.Sprintf("WHERE job_type = '%s' AND job_id in (%s)", + jobspb.TypeChangefeed, n.Jobs.String()) + } } sqlStmt := fmt.Sprintf("%s %s %s", selectClause, whereClause, orderbyClause) diff --git a/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx index 264343cfbd2c..cf776149b128 100644 --- a/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx @@ -7,7 +7,7 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import React from "react"; +import React, { useContext } from "react"; import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import { ArrowLeft } from "@cockroachlabs/icons"; import { Col, Row, Tabs } from "antd"; @@ -41,6 +41,8 @@ import classNames from "classnames/bind"; import { Timestamp } from "../../timestamp"; import { RequestState } from "../../api"; import moment from "moment-timezone"; +import { CockroachCloudContext } from "src/contexts"; +import { InlineAlert } from "@cockroachlabs/ui-components"; const { TabPane } = Tabs; @@ -49,6 +51,7 @@ const jobCx = classNames.bind(jobStyles); enum TabKeysEnum { OVERVIEW = "Overview", + PROFILER = "Profiler", } export interface JobDetailsStateProps { @@ -59,15 +62,26 @@ export interface JobDetailsDispatchProps { refreshJob: (req: JobRequest) => void; } +export interface JobDetailsState { + currentTab?: string; +} + export type JobDetailsProps = JobDetailsStateProps & JobDetailsDispatchProps & RouteComponentProps; -export class JobDetails extends React.Component { +export class JobDetails extends React.Component< + JobDetailsProps, + JobDetailsState +> { refreshDataInterval: NodeJS.Timeout; constructor(props: JobDetailsProps) { super(props); + const searchParams = new URLSearchParams(props.history.location.search); + this.state = { + currentTab: searchParams.get("tab") || "overview", + }; } private refresh(): void { @@ -99,6 +113,32 @@ export class JobDetails extends React.Component { prevPage = (): void => this.props.history.goBack(); + renderProfilerTabContent = ( + job: cockroach.server.serverpb.JobResponse, + ): React.ReactElement => { + const id = job?.id; + // This URL results in a cluster-wide CPU profile to be collected for 5 + // seconds. We set `tagfocus` (tf) to only view the samples corresponding to + // this job's execution. + const url = `debug/pprof/ui/cpu?node=all&seconds=5&labels=true&tf=job.*${id}`; + return ( + + + + Profile} + /> + + + + + ); + }; + renderOverviewTabContent = ( hasNextRun: boolean, nextRun: moment.Moment, @@ -193,12 +233,26 @@ export class JobDetails extends React.Component { ); }; + onTabChange = (tabId: string): void => { + const { history } = this.props; + const searchParams = new URLSearchParams(history.location.search); + searchParams.set("tab", tabId); + history.replace({ + ...history.location, + search: searchParams.toString(), + }); + this.setState({ + currentTab: tabId, + }); + }; + render(): React.ReactElement { const isLoading = this.props.jobRequest.inFlight; const error = this.props.jobRequest.error; const job = this.props.jobRequest.data; const nextRun = TimestampToMoment(job?.next_run); const hasNextRun = nextRun?.isAfter(); + const { currentTab } = this.state; return (
@@ -238,10 +292,17 @@ export class JobDetails extends React.Component { {this.renderOverviewTabContent(hasNextRun, nextRun, job)} + {!useContext(CockroachCloudContext) && ( + + {this.renderProfilerTabContent(job)} + + )} )}