kvserver: abstract away system config span usage in kv

pkg/cli/debug.go

@@ -32,7 +32,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
 	"github.com/cockroachdb/cockroach/pkg/cli/syncbench"
 	"github.com/cockroachdb/cockroach/pkg/config"
-	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
 	"github.com/cockroachdb/cockroach/pkg/gossip"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
@@ -672,7 +671,7 @@ func runDebugGCCmd(cmd *cobra.Command, args []string) error {
 	defer stopper.Stop(context.Background())
 
 	var rangeID roachpb.RangeID
-	gcTTLInSeconds := int64((24 * time.Hour).Seconds())
+	gcTTL := 24 * time.Hour
 	intentAgeThreshold := gc.IntentAgeThreshold.Default()
 	intentBatchSize := gc.MaxIntentsPerCleanupBatch.Default()
 
@@ -683,10 +682,11 @@ func runDebugGCCmd(cmd *cobra.Command, args []string) error {
 		}
 	}
 	if len(args) > 2 {
-		var err error
-		if gcTTLInSeconds, err = parsePositiveInt(args[2]); err != nil {
+		gcTTLInSeconds, err := parsePositiveInt(args[2])
+		if err != nil {
 			return errors.Wrapf(err, "unable to parse %v as TTL", args[2])
 		}
+		gcTTL = time.Duration(gcTTLInSeconds) * time.Second
 	}
 	if len(args) > 1 {
 		var err error
@@ -736,14 +736,14 @@ func runDebugGCCmd(cmd *cobra.Command, args []string) error {
 	for _, desc := range descs {
 		snap := db.NewSnapshot()
 		defer snap.Close()
-		policy := zonepb.GCPolicy{TTLSeconds: int32(gcTTLInSeconds)}
 		now := hlc.Timestamp{WallTime: timeutil.Now().UnixNano()}
-		thresh := gc.CalculateThreshold(now, policy)
+		thresh := gc.CalculateThreshold(now, gcTTL)
 		info, err := gc.Run(
 			context.Background(),
 			&desc, snap,
-			now, thresh, gc.RunOptions{IntentAgeThreshold: intentAgeThreshold, MaxIntentsPerIntentCleanupBatch: intentBatchSize}, policy,
-			gc.NoopGCer{},
+			now, thresh,
+			gc.RunOptions{IntentAgeThreshold: intentAgeThreshold, MaxIntentsPerIntentCleanupBatch: intentBatchSize},
+			gcTTL, gc.NoopGCer{},
 			func(_ context.Context, _ []roachpb.Intent) error { return nil },
 			func(_ context.Context, _ *roachpb.Transaction) error { return nil },
 		)

pkg/config/system.go

@@ -294,6 +294,18 @@ func (s *SystemConfig) GetZoneConfigForKey(key roachpb.RKey) (*zonepb.ZoneConfig
 	return s.getZoneConfigForKey(DecodeKeyIntoZoneIDAndSuffix(key))
 }
 
+// GetSpanConfigForKey looks of the span config for the given key. It's part of
+// spanconfig.StoreReader interface.
+func (s *SystemConfig) GetSpanConfigForKey(
+	ctx context.Context, key roachpb.RKey,
+) (roachpb.SpanConfig, error) {
+	zone, err := s.GetZoneConfigForKey(key)
+	if err != nil {
+		return roachpb.SpanConfig{}, err
+	}
+	return zone.AsSpanConfig(), nil
+}
+
 // DecodeKeyIntoZoneIDAndSuffix figures out the zone that the key belongs to.
 func DecodeKeyIntoZoneIDAndSuffix(key roachpb.RKey) (id SystemTenantObjectID, keySuffix []byte) {
 	objectID, keySuffix, ok := DecodeSystemTenantObjectID(key)

pkg/config/system_test.go

@@ -544,6 +544,7 @@ func TestComputeSplitKeyTenantBoundaries(t *testing.T) {
 func TestGetZoneConfigForKey(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 
+	ctx := context.Background()
 	testCases := []struct {
 		key        roachpb.RKey
 		expectedID config.SystemTenantObjectID
@@ -614,11 +615,11 @@ func TestGetZoneConfigForKey(t *testing.T) {
 			_ *config.SystemConfig, id config.SystemTenantObjectID,
 		) (*zonepb.ZoneConfig, *zonepb.ZoneConfig, bool, error) {
 			objectID = id
-			return &zonepb.ZoneConfig{}, nil, false, nil
+			return cfg.DefaultZoneConfig, nil, false, nil
 		}
-		_, err := cfg.GetZoneConfigForKey(tc.key)
+		_, err := cfg.GetSpanConfigForKey(ctx, tc.key)
 		if err != nil {
-			t.Errorf("#%d: GetZoneConfigForKey(%v) got error: %v", tcNum, tc.key, err)
+			t.Errorf("#%d: GetSpanConfigForKey(%v) got error: %v", tcNum, tc.key, err)
 		}
 		if objectID != tc.expectedID {
 			t.Errorf("#%d: GetZoneConfigForKey(%v) got %d; want %d", tcNum, tc.key, objectID, tc.expectedID)

pkg/config/zonepb/zone.go

@@ -15,7 +15,6 @@ import (
 	"context"
 	"fmt"
 	"strings"
-	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/keys"
@@ -206,20 +205,6 @@ func NewZoneConfig() *ZoneConfig {
 	}
 }
 
-// EmptyCompleteZoneConfig is the zone configuration where
-// all fields are set but set to their respective zero values.
-func EmptyCompleteZoneConfig() *ZoneConfig {
-	return &ZoneConfig{
-		NumReplicas:               proto.Int32(0),
-		NumVoters:                 proto.Int32(0),
-		RangeMinBytes:             proto.Int64(0),
-		RangeMaxBytes:             proto.Int64(0),
-		GC:                        &GCPolicy{TTLSeconds: 0},
-		InheritedConstraints:      true,
-		InheritedLeasePreferences: true,
-	}
-}
-
 // DefaultZoneConfig is the default zone configuration used when no custom
 // config has been specified.
 func DefaultZoneConfig() ZoneConfig {
@@ -977,37 +962,6 @@ func (z ZoneConfig) GetSubzoneForKeySuffix(keySuffix []byte) (*Subzone, int32) {
 	return nil, -1
 }
 
-// GetNumVoters returns the number of voting replicas for the given zone config.
-//
-// This method will panic if called on a ZoneConfig with an uninitialized
-// NumReplicas attribute.
-func (z *ZoneConfig) GetNumVoters() int32 {
-	if z.NumReplicas == nil {
-		panic("NumReplicas must not be nil")
-	}
-	if z.NumVoters != nil && *z.NumVoters != 0 {
-		return *z.NumVoters
-	}
-	return *z.NumReplicas
-}
-
-// GetNumNonVoters returns the number of non-voting replicas as defined in the
-// zone config.
-//
-// This method will panic if called on a ZoneConfig with an uninitialized
-// NumReplicas attribute.
-func (z *ZoneConfig) GetNumNonVoters() int32 {
-	if z.NumReplicas == nil {
-		panic("NumReplicas must not be nil")
-	}
-	if z.NumVoters != nil && *z.NumVoters != 0 {
-		return *z.NumReplicas - *z.NumVoters
-	}
-	// `num_voters` hasn't been explicitly configured. Every replica should be a
-	// voting replica.
-	return 0
-}
-
 // SetSubzone installs subzone into the ZoneConfig, overwriting any existing
 // subzone with the same IndexID and PartitionName.
 func (z *ZoneConfig) SetSubzone(subzone Subzone) {
@@ -1150,11 +1104,6 @@ func (c *Constraint) GetValue() string {
 	return c.Value
 }
 
-// TTL returns the implies TTL as a time.Duration.
-func (m *GCPolicy) TTL() time.Duration {
-	return time.Duration(m.TTLSeconds) * time.Second
-}
-
 // EnsureFullyHydrated returns an assertion error if the zone config is not
 // fully hydrated. A fully hydrated zone configuration must have all required
 // fields set, which are RangeMaxBytes, RangeMinBytes, GC, and NumReplicas.
@@ -1201,7 +1150,7 @@ func (z *ZoneConfig) toSpanConfig() (roachpb.SpanConfig, error) {
 	// Copy over the values.
 	sc.RangeMinBytes = *z.RangeMinBytes
 	sc.RangeMaxBytes = *z.RangeMaxBytes
-	sc.GCTTL = z.GC.TTLSeconds
+	sc.GCPolicy.TTLSeconds = z.GC.TTLSeconds
 
 	// GlobalReads is false by default.
 	if z.GlobalReads != nil {

pkg/config/zonepb/zone_test.go

@@ -1183,9 +1183,11 @@ func TestZoneConfigToSpanConfigConversion(t *testing.T) {
 				},
 			},
 			expectSpanConfig: roachpb.SpanConfig{
-				RangeMinBytes:    100000,
-				RangeMaxBytes:    200000,
-				GCTTL:            2400,
+				RangeMinBytes: 100000,
+				RangeMaxBytes: 200000,
+				GCPolicy: roachpb.GCPolicy{
+					TTLSeconds: 2400,
+				},
 				GlobalReads:      false,
 				NumVoters:        0,
 				NumReplicas:      3,
@@ -1206,9 +1208,11 @@ func TestZoneConfigToSpanConfigConversion(t *testing.T) {
 				},
 			},
 			expectSpanConfig: roachpb.SpanConfig{
-				RangeMinBytes:    100000,
-				RangeMaxBytes:    200000,
-				GCTTL:            2400,
+				RangeMinBytes: 100000,
+				RangeMaxBytes: 200000,
+				GCPolicy: roachpb.GCPolicy{
+					TTLSeconds: 2400,
+				},
 				GlobalReads:      true,
 				NumVoters:        0,
 				NumReplicas:      3,
@@ -1229,9 +1233,11 @@ func TestZoneConfigToSpanConfigConversion(t *testing.T) {
 				},
 			},
 			expectSpanConfig: roachpb.SpanConfig{
-				RangeMinBytes:    100000,
-				RangeMaxBytes:    200000,
-				GCTTL:            2400,
+				RangeMinBytes: 100000,
+				RangeMaxBytes: 200000,
+				GCPolicy: roachpb.GCPolicy{
+					TTLSeconds: 2400,
+				},
 				GlobalReads:      false,
 				NumVoters:        0,
 				NumReplicas:      3,
@@ -1287,10 +1293,12 @@ func TestZoneConfigToSpanConfigConversion(t *testing.T) {
 			expectSpanConfig: roachpb.SpanConfig{
 				RangeMinBytes: 100000,
 				RangeMaxBytes: 200000,
-				GCTTL:         2400,
-				GlobalReads:   false,
-				NumVoters:     0,
-				NumReplicas:   3,
+				GCPolicy: roachpb.GCPolicy{
+					TTLSeconds: 2400,
+				},
+				GlobalReads: false,
+				NumVoters:   0,
+				NumReplicas: 3,
 				Constraints: []roachpb.ConstraintsConjunction{
 					{
 						NumReplicas: 1,
@@ -1333,11 +1341,13 @@ func TestZoneConfigToSpanConfigConversion(t *testing.T) {
 			expectSpanConfig: roachpb.SpanConfig{
 				RangeMinBytes: 100000,
 				RangeMaxBytes: 200000,
-				GCTTL:         2400,
-				GlobalReads:   false,
-				NumVoters:     0,
-				NumReplicas:   3,
-				Constraints:   []roachpb.ConstraintsConjunction{},
+				GCPolicy: roachpb.GCPolicy{
+					TTLSeconds: 2400,
+				},
+				GlobalReads: false,
+				NumVoters:   0,
+				NumReplicas: 3,
+				Constraints: []roachpb.ConstraintsConjunction{},
 				VoterConstraints: []roachpb.ConstraintsConjunction{
 					{
 						Constraints: []roachpb.Constraint{
@@ -1381,9 +1391,11 @@ func TestZoneConfigToSpanConfigConversion(t *testing.T) {
 				},
 			},
 			expectSpanConfig: roachpb.SpanConfig{
-				RangeMinBytes:    100000,
-				RangeMaxBytes:    200000,
-				GCTTL:            2400,
+				RangeMinBytes: 100000,
+				RangeMaxBytes: 200000,
+				GCPolicy: roachpb.GCPolicy{
+					TTLSeconds: 2400,
+				},
 				GlobalReads:      false,
 				NumVoters:        0,
 				NumReplicas:      3,

pkg/kv/kvclient/kvcoord/BUILD.bazel

@@ -184,7 +184,6 @@ go_test(
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_errors//errutil",
         "@com_github_cockroachdb_redact//:redact",
-        "@com_github_gogo_protobuf//proto",
         "@com_github_golang_mock//gomock",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",

pkg/kv/kvclient/kvcoord/split_test.go

@@ -19,7 +19,6 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/config"
-	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
@@ -32,7 +31,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/errors"
-	"github.com/gogo/protobuf/proto"
 )
 
 // startTestWriter creates a writer which initiates a sequence of
@@ -176,15 +174,15 @@ func TestRangeSplitsWithConcurrentTxns(t *testing.T) {
 func TestRangeSplitsWithWritePressure(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
-	// Override default zone config.
-	cfg := zonepb.DefaultZoneConfigRef()
-	cfg.RangeMaxBytes = proto.Int64(1 << 18)
+	// Override default span config.
+	cfg := kvserver.TestingDefaultSpanConfig()
+	cfg.RangeMaxBytes = 1 << 18
 
 	// Manually create the local test cluster so that the split queue
 	// is not disabled (LocalTestCluster disables it by default).
 	s := &localtestcluster.LocalTestCluster{
 		Cfg: kvserver.StoreConfig{
-			DefaultZoneConfig: cfg,
+			DefaultSpanConfig: cfg,
 		},
 		StoreTestingKnobs: &kvserver.StoreTestingKnobs{
 			DisableScanner: true,

pkg/kv/kvserver/BUILD.bazel

@@ -152,6 +152,7 @@ go_library(
         "//pkg/server/telemetry",
         "//pkg/settings",
         "//pkg/settings/cluster",
+        "//pkg/spanconfig",
         "//pkg/sql/sessiondata",
         "//pkg/sql/sqlutil",
         "//pkg/storage",
@@ -340,6 +341,7 @@ go_test(
         "//pkg/server/serverpb",
         "//pkg/server/telemetry",
         "//pkg/settings/cluster",
+        "//pkg/spanconfig",
         "//pkg/sql",
         "//pkg/sql/catalog/bootstrap",
         "//pkg/sql/catalog/catalogkeys",