This commit provides a new implementation of systemStatusServer.SpanS…

…tats,

proposed in #84105. The goal is to provide accurate table/index stats since
the assumption that 1 range can contain at most 1 table/index no longer holds.

Part of: https://cockroachlabs.atlassian.net/browse/CRDB-22711
Release note: None

add interface and abstract away access to KV

rename accessor implementation

pkg/BUILD.bazel

@@ -1157,6 +1157,8 @@ GO_TARGETS = [
     "//pkg/kv/kvclient/rangefeed:rangefeed",
     "//pkg/kv/kvclient/rangefeed:rangefeed_test",
     "//pkg/kv/kvclient/rangestats:rangestats",
+    "//pkg/kv/kvclient/spanstats/spanstatsaccessor:spanstatsaccessor",
+    "//pkg/kv/kvclient/spanstats:spanstats",
     "//pkg/kv/kvclient:kvclient",
     "//pkg/kv/kvnemesis/kvnemesisutil:kvnemesisutil",
     "//pkg/kv/kvnemesis:kvnemesis",
@@ -2576,6 +2578,8 @@ GET_X_DATA_TARGETS = [
     "//pkg/kv/kvclient/rangefeed/rangefeedbuffer:get_x_data",
     "//pkg/kv/kvclient/rangefeed/rangefeedcache:get_x_data",
     "//pkg/kv/kvclient/rangestats:get_x_data",
+    "//pkg/kv/kvclient/spanstats:get_x_data",
+    "//pkg/kv/kvclient/spanstats/spanstatsaccessor:get_x_data",
     "//pkg/kv/kvnemesis:get_x_data",
     "//pkg/kv/kvnemesis/kvnemesisutil:get_x_data",
     "//pkg/kv/kvprober:get_x_data",

pkg/kv/kvclient/spanstats/BUILD.bazel

@@ -0,0 +1,15 @@
+load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "spanstats",
+    srcs = ["span_stats.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvclient/spanstats",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/roachpb",
+        "//pkg/server/serverpb",
+    ],
+)
+
+get_x_data(name = "get_x_data")

pkg/kv/kvclient/spanstats/span_stats.go

@@ -0,0 +1,34 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// 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.
+
+package spanstats
+
+import (
+	"context"
+
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
+)
+
+// Accessor mediates access to SpanStats by consumers across the KV/Tenant
+// boundary.
+type Accessor interface {
+
+	// SpanStats returns the span stats between startKey and endKey.
+	// The caller can request span stats from across the cluster by setting
+	// nodeId to 0. The caller can request span stats from a specific node by
+	// setting the value of nodeID accordingly.
+	SpanStats(
+		ctx context.Context,
+		startKey,
+		endKey roachpb.Key,
+		nodeID roachpb.NodeID,
+	) (*serverpb.InternalSpanStatsResponse, error)
+}

pkg/kv/kvclient/spanstats/spanstatsaccessor/BUILD.bazel

@@ -0,0 +1,15 @@
+load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "spanstatsaccessor",
+    srcs = ["accessor.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvclient/spanstats/spanstatsaccessor",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/roachpb",
+        "//pkg/server/serverpb",
+    ],
+)
+
+get_x_data(name = "get_x_data")

pkg/kv/kvclient/spanstats/spanstatsaccessor/accessor.go

@@ -0,0 +1,42 @@
+package spanstatsaccessor
+
+import (
+	"context"
+
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
+)
+
+// LocalAccessor is an implementation of spanstats.Accessor that is meant
+// to provide access to span span stats to servers that are co-located on the KV
+// node.
+type LocalAccessor struct {
+	spanStatsServer serverpb.SpanStatsServer
+}
+
+// New returns a new instance of AccessorImpl.
+func New(server serverpb.SpanStatsServer) *LocalAccessor {
+	return &LocalAccessor{spanStatsServer: server}
+}
+
+// SpanStats implements the spanstats.Accessor interface.
+func (a *LocalAccessor) SpanStats(
+	ctx context.Context, startKey,
+	endKey roachpb.Key, nodeID roachpb.NodeID,
+) (*serverpb.InternalSpanStatsResponse, error) {
+
+	res, err := a.spanStatsServer.GetSpanStats(ctx,
+		&serverpb.InternalSpanStatsRequest{
+			Span: roachpb.Span{
+				Key:    startKey,
+				EndKey: endKey,
+			},
+			NodeID: nodeID,
+		})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return res, nil
+}

pkg/kv/kvserver/client_status_test.go

@@ -76,17 +76,19 @@ func TestComputeStatsForKeySpan(t *testing.T) {
 		{"b", "e", 2, 5},
 		{"e", "i", 2, 1},
 	} {
-		start, end := tcase.startKey, tcase.endKey
-		result, err := store.ComputeStatsForKeySpan(
-			roachpb.RKey(start), roachpb.RKey(end))
-		if err != nil {
-			t.Fatal(err)
-		}
-		if a, e := result.ReplicaCount, tcase.expectedRanges; a != e {
-			t.Errorf("Expected %d ranges in span [%s - %s], found %d", e, start, end, a)
-		}
-		if a, e := result.MVCC.LiveCount, tcase.expectedKeys; a != e {
-			t.Errorf("Expected %d keys in span [%s - %s], found %d", e, start, end, a)
-		}
+		_ = tcase
+		t.Errorf("implement me")
+		//start, end := tcase.startKey, tcase.endKey
+		//result, err := store.ComputeStatsForKeySpan(
+		//	roachpb.RKey(start), roachpb.RKey(end))
+		//if err != nil {
+		//	t.Fatal(err)
+		//}
+		//if a, e := result.ReplicaCount, tcase.expectedRanges; a != e {
+		//	t.Errorf("Expected %d ranges in span [%s - %s], found %d", e, start, end, a)
+		//}
+		//if a, e := result.MVCC.LiveCount, tcase.expectedKeys; a != e {
+		//	t.Errorf("Expected %d keys in span [%s - %s], found %d", e, start, end, a)
+		//}
 	}
 }

pkg/kv/kvserver/store.go

@@ -61,7 +61,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigstore"
 	"github.com/cockroachdb/cockroach/pkg/storage"
-	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/admission"
 	"github.com/cockroachdb/cockroach/pkg/util/admission/admissionpb"
 	"github.com/cockroachdb/cockroach/pkg/util/contextutil"
@@ -3069,33 +3068,6 @@ func mapToHotReplicasInfo(repls []CandidateReplica) []HotReplicaInfo {
 	return hotRepls
 }
 
-// StoreKeySpanStats carries the result of a stats computation over a key range.
-type StoreKeySpanStats struct {
-	ReplicaCount         int
-	MVCC                 enginepb.MVCCStats
-	ApproximateDiskBytes uint64
-}
-
-// ComputeStatsForKeySpan computes the aggregated MVCCStats for all replicas on
-// this store which contain any keys in the supplied range.
-func (s *Store) ComputeStatsForKeySpan(startKey, endKey roachpb.RKey) (StoreKeySpanStats, error) {
-	var result StoreKeySpanStats
-
-	newStoreReplicaVisitor(s).UndefinedOrder().Visit(func(repl *Replica) bool {
-		desc := repl.Desc()
-		if bytes.Compare(startKey, desc.EndKey) >= 0 || bytes.Compare(desc.StartKey, endKey) >= 0 {
-			return true // continue
-		}
-		result.MVCC.Add(repl.GetMVCCStats())
-		result.ReplicaCount++
-		return true
-	})
-
-	var err error
-	result.ApproximateDiskBytes, err = s.engine.ApproximateDiskBytes(startKey.AsRawKey(), endKey.AsRawKey())
-	return result, err
-}
-
 // ReplicateQueueDryRun runs the given replica through the replicate queue
 // (using the allocator) without actually carrying out any changes, returning
 // all trace messages collected along the way.

pkg/server/BUILD.bazel

@@ -35,6 +35,7 @@ go_library(
         "init.go",
         "init_handshake.go",
         "initial_sql.go",
+        "key_visualizer_server.go",
         "listen_and_update_addrs.go",
         "load_endpoint.go",
         "loss_of_quorum.go",
@@ -113,6 +114,8 @@ go_library(
         "//pkg/kv/kvclient/kvtenant",
         "//pkg/kv/kvclient/rangefeed",
         "//pkg/kv/kvclient/rangestats",
+        "//pkg/kv/kvclient/spanstats",
+        "//pkg/kv/kvclient/spanstats/spanstatsaccessor",
         "//pkg/kv/kvprober",
         "//pkg/kv/kvserver",
         "//pkg/kv/kvserver/allocator/allocatorimpl",

pkg/server/admin.go

@@ -1360,7 +1360,7 @@ func (s *adminServer) statsForSpan(
 							req := serverpb.SpanStatsRequest{
 								StartKey: rSpan.Key,
 								EndKey:   rSpan.EndKey,
-								NodeID:   nodeID.String(),
+								NodeID:   nodeID,
 							}
 							spanResponse, err = client.SpanStats(ctx, &req)
 						}