Merge #30611

30611: sql, config: make zone configs cascade r=ridwanmsharif a=ridwanmsharif

Modifies the `ZoneConfig` struct to
make fields optional now. If fields are not set, they
are looked for in the heirarchy. Refer to the RFC for
more details.

Issue: #28901
Implements most of RFC: #30426

TODO:
- [x] Update `set_zone_config` to use the new structure and validate them
- [x] Exhaustively test cascading zone configs

Release note: Changes behavior of Zone Configs.

Co-authored-by: Ridwan Sharif <[email protected]>

docs/generated/settings/settings.html

@@ -81,6 +81,6 @@
 <tr><td><code>trace.debug.enable</code></td><td>boolean</td><td><code>false</code></td><td>if set, traces for recent requests can be seen in the /debug page</td></tr>
 <tr><td><code>trace.lightstep.token</code></td><td>string</td><td><code></code></td><td>if set, traces go to Lightstep using this token</td></tr>
 <tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'); ignored if trace.lightstep.token is set.</td></tr>
-<tr><td><code>version</code></td><td>custom validation</td><td><code>2.1</code></td><td>set the active cluster version in the format '<major>.<minor>'.</td></tr>
+<tr><td><code>version</code></td><td>custom validation</td><td><code>2.1-1</code></td><td>set the active cluster version in the format '<major>.<minor>'.</td></tr>
 </tbody>
 </table>

pkg/acceptance/cluster/dockercluster.go

@@ -71,7 +71,7 @@ const CockroachBinaryInContainer = "/cockroach/cockroach"
 var cockroachImage = flag.String("i", defaultImage, "the docker image to run")
 var cockroachEntry = flag.String("e", "", "the entry point for the image")
 var waitOnStop = flag.Bool("w", false, "wait for the user to interrupt before tearing down the cluster")
-var maxRangeBytes = config.DefaultZoneConfig().RangeMaxBytes
+var maxRangeBytes = *config.DefaultZoneConfig().RangeMaxBytes
 
 // CockroachBinary is the path to the host-side binary to use.
 var CockroachBinary = flag.String("b", func() string {

pkg/acceptance/localcluster/cluster.go

@@ -34,6 +34,7 @@ import (
 	"text/tabwriter"
 	"time"
 
+	"github.com/gogo/protobuf/proto"
 	"github.com/pkg/errors"
 	// Import postgres driver.
 	_ "github.com/lib/pq"
@@ -375,8 +376,8 @@ func (c *Cluster) isReplicated() (bool, string) {
 // UpdateZoneConfig updates the default zone config for the cluster.
 func (c *Cluster) UpdateZoneConfig(rangeMinBytes, rangeMaxBytes int64) {
 	zone := config.DefaultZoneConfig()
-	zone.RangeMinBytes = rangeMinBytes
-	zone.RangeMaxBytes = rangeMaxBytes
+	zone.RangeMinBytes = proto.Int64(rangeMinBytes)
+	zone.RangeMaxBytes = proto.Int64(rangeMaxBytes)
 
 	buf, err := protoutil.Marshal(&zone)
 	if err != nil {

pkg/ccl/backupccl/backup_test.go

@@ -27,6 +27,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach-go/crdb"
+	"github.com/gogo/protobuf/proto"
 	"github.com/kr/pretty"
 	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"
@@ -899,7 +900,9 @@ func TestBackupRestoreControlJob(t *testing.T) {
 			t.Fatal(err)
 		}
 		last := uint32(v.ValueInt())
-		config.TestingSetZoneConfig(last+1, config.ZoneConfig{RangeMaxBytes: 5000})
+		zoneConfig := config.DefaultZoneConfig()
+		zoneConfig.RangeMaxBytes = proto.Int64(5000)
+		config.TestingSetZoneConfig(last+1, zoneConfig)
 	}
 	const numAccounts = 1000
 	_, _, outerDB, _, cleanup := backupRestoreTestSetupWithParams(t, multiNode, numAccounts, init, params)

pkg/ccl/importccl/exportcsv_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/workload"
 	"github.com/cockroachdb/cockroach/pkg/workload/bank"
+	"github.com/gogo/protobuf/proto"
 )
 
 func setupExportableBank(t *testing.T, nodes, rows int) (*sqlutils.SQLRunner, string, func()) {
@@ -49,7 +50,9 @@ func setupExportableBank(t *testing.T, nodes, rows int) (*sqlutils.SQLRunner, st
 		t.Fatal(err)
 	}
 	last := uint32(v.ValueInt())
-	config.TestingSetZoneConfig(last+1, config.ZoneConfig{RangeMaxBytes: 5000})
+	zoneConfig := config.DefaultZoneConfig()
+	zoneConfig.RangeMaxBytes = proto.Int64(5000)
+	config.TestingSetZoneConfig(last+1, zoneConfig)
 	if err := workload.Split(ctx, db.DB, wk.Tables()[0], 1 /* concurrency */); err != nil {
 		t.Fatal(err)
 	}

pkg/ccl/importccl/load.go

@@ -55,7 +55,7 @@ func Load(
 	tempPrefix string,
 ) (backupccl.BackupDescriptor, error) {
 	if loadChunkBytes == 0 {
-		loadChunkBytes = config.DefaultZoneConfig().RangeMaxBytes / 2
+		loadChunkBytes = *config.DefaultZoneConfig().RangeMaxBytes / 2
 	}
 
 	var txCtx transform.ExprTransformContext

pkg/ccl/partitionccl/partition_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
+	"github.com/gogo/protobuf/proto"
 	"github.com/pkg/errors"
 	yaml "gopkg.in/yaml.v2"
 
@@ -180,7 +181,8 @@ func (t *partitioningTest) parse() error {
 		if err := yaml.UnmarshalStrict([]byte("["+constraints+"]"), &parsedConstraints); err != nil {
 			return errors.Wrapf(err, "parsing constraints: %s", constraints)
 		}
-		subzone.Config.Constraints = ([]config.Constraints)(parsedConstraints)
+		subzone.Config.Constraints = parsedConstraints.Constraints
+		subzone.Config.InheritedConstraints = parsedConstraints.Inherited
 
 		t.parsed.subzones = append(t.parsed.subzones, subzone)
 	}
@@ -1097,7 +1099,7 @@ func verifyScansOnNode(db *gosql.DB, query string, node string) error {
 
 func setupPartitioningTestCluster(ctx context.Context, t testing.TB) (*sqlutils.SQLRunner, func()) {
 	cfg := config.DefaultZoneConfig()
-	cfg.NumReplicas = 1
+	cfg.NumReplicas = proto.Int32(1)
 	resetZoneConfig := config.TestingSetDefaultZoneConfig(cfg)
 
 	tsArgs := func(attr string) base.TestServerArgs {

pkg/ccl/partitionccl/zone_test.go

@@ -46,7 +46,9 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 	yamlDefault := fmt.Sprintf("gc: {ttlseconds: %d}", config.DefaultZoneConfig().GC.TTLSeconds)
 	yamlOverride := "gc: {ttlseconds: 42}"
 	zoneOverride := config.DefaultZoneConfig()
-	zoneOverride.GC.TTLSeconds = 42
+	zoneOverride.GC = &config.GCPolicy{TTLSeconds: 42}
+	partialZoneOverride := *config.NewZoneConfig()
+	partialZoneOverride.GC = &config.GCPolicy{TTLSeconds: 42}
 
 	dbDescID := uint32(keys.MinNonPredefinedUserDescID)
 
@@ -86,6 +88,33 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 		Config:       zoneOverride,
 	}
 
+	// Partially filled config rows
+	partialDbRow := sqlutils.ZoneRow{
+		ID:           dbDescID,
+		CLISpecifier: "d",
+		Config:       partialZoneOverride,
+	}
+	partialTableRow := sqlutils.ZoneRow{
+		ID:           dbDescID + 1,
+		CLISpecifier: "d.t",
+		Config:       partialZoneOverride,
+	}
+	partialPrimaryRow := sqlutils.ZoneRow{
+		ID:           dbDescID + 1,
+		CLISpecifier: "d.t@primary",
+		Config:       partialZoneOverride,
+	}
+	partialP0Row := sqlutils.ZoneRow{
+		ID:           dbDescID + 1,
+		CLISpecifier: "d.t.p0",
+		Config:       partialZoneOverride,
+	}
+	partialP1Row := sqlutils.ZoneRow{
+		ID:           dbDescID + 1,
+		CLISpecifier: "d.t.p1",
+		Config:       partialZoneOverride,
+	}
+
 	// Remove stock zone configs installed at cluster bootstrap. Otherwise this
 	// test breaks whenever these stock zone configs are adjusted.
 	sqlutils.RemoveAllZoneConfigs(t, sqlDB)
@@ -102,7 +131,7 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 	// Ensure a database zone config applies to that database, its tables, and its
 	// tables' indices and partitions.
 	sqlutils.SetZoneConfig(t, sqlDB, "DATABASE d", yamlOverride)
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, dbRow)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, partialDbRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "DATABASE d", dbRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "TABLE d.t", dbRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "INDEX d.t@primary", dbRow)
@@ -112,7 +141,7 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 	// Ensure a table zone config applies to that table and its indices and
 	// partitions, but no other zones.
 	sqlutils.SetZoneConfig(t, sqlDB, "TABLE d.t", yamlOverride)
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, dbRow, tableRow)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, partialDbRow, partialTableRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "DATABASE d", dbRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "TABLE d.t", tableRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "INDEX d.t@primary", tableRow)
@@ -122,7 +151,7 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 	// Ensure an index zone config applies to that index and its partitions, but
 	// no other zones.
 	sqlutils.SetZoneConfig(t, sqlDB, "INDEX d.t@primary", yamlOverride)
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, dbRow, tableRow, primaryRow)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, partialDbRow, partialTableRow, partialPrimaryRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "DATABASE d", dbRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "TABLE d.t", tableRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "INDEX d.t@primary", primaryRow)
@@ -132,7 +161,7 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 	// Ensure a partition zone config applies to that partition, but no other
 	// zones.
 	sqlutils.SetZoneConfig(t, sqlDB, "PARTITION p0 OF TABLE d.t", yamlOverride)
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, dbRow, tableRow, primaryRow, p0Row)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, partialDbRow, partialTableRow, partialPrimaryRow, partialP0Row)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "DATABASE d", dbRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "TABLE d.t", tableRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "INDEX d.t@primary", primaryRow)
@@ -142,7 +171,7 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 	// Ensure updating the default zone propagates to zones without an override,
 	// but not to those with overrides.
 	sqlutils.SetZoneConfig(t, sqlDB, "RANGE default", yamlOverride)
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultOverrideRow, dbRow, tableRow, primaryRow, p0Row)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultOverrideRow, partialDbRow, partialTableRow, partialPrimaryRow, partialP0Row)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "DATABASE d", dbRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "TABLE d.t", tableRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "INDEX d.t@primary", primaryRow)
@@ -151,7 +180,7 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 
 	// Ensure deleting a database zone leaves child overrides in place.
 	sqlutils.DeleteZoneConfig(t, sqlDB, "DATABASE d")
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultOverrideRow, tableRow, primaryRow, p0Row)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultOverrideRow, partialTableRow, partialPrimaryRow, partialP0Row)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "DATABASE d", defaultOverrideRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "TABLE d.t", tableRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "INDEX d.t@primary", primaryRow)
@@ -160,15 +189,15 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 
 	// Ensure deleting a table zone leaves child overrides in place.
 	sqlutils.DeleteZoneConfig(t, sqlDB, "TABLE d.t")
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultOverrideRow, primaryRow, p0Row)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultOverrideRow, partialPrimaryRow, partialP0Row)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "TABLE d.t", defaultOverrideRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "INDEX d.t@primary", primaryRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "PARTITION p0 OF TABLE d.t", p0Row)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "PARTITION p1 OF TABLE d.t", primaryRow)
 
 	// Ensure deleting an index zone leaves child overrides in place.
 	sqlutils.DeleteZoneConfig(t, sqlDB, "INDEX d.t@primary")
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultOverrideRow, p0Row)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultOverrideRow, partialP0Row)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "INDEX d.t@primary", defaultOverrideRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "PARTITION p0 OF TABLE d.t", p0Row)
 
@@ -196,7 +225,7 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 	// Ensure subzones can be created even when no table zone exists.
 	sqlutils.SetZoneConfig(t, sqlDB, "PARTITION p0 OF TABLE d.t", yamlOverride)
 	sqlutils.SetZoneConfig(t, sqlDB, "PARTITION p1 OF TABLE d.t", yamlOverride)
-	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, p0Row, p1Row)
+	sqlutils.VerifyAllZoneConfigs(t, sqlDB, defaultRow, partialP0Row, partialP1Row)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "TABLE d.t", defaultRow)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "PARTITION p0 OF TABLE d.t", p0Row)
 	sqlutils.VerifyZoneConfigForTarget(t, sqlDB, "PARTITION p1 OF TABLE d.t", p1Row)

pkg/cli/demo.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net/url"
 
+	"github.com/gogo/protobuf/proto"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
@@ -98,7 +99,9 @@ func setupTransientServer(
 	// Set up the default zone configuration. We are using an in-memory store
 	// so we really want to disable replication.
 	cfg := config.DefaultZoneConfig()
-	cfg.NumReplicas = 1
+	cfg.NumReplicas = proto.Int32(1)
+
+	// TODO(benesch): should this use TestingSetDefaultZone config instead?
 	restoreCfg := config.TestingSetDefaultSystemZoneConfig(cfg)
 	prevCleanup := cleanup
 	cleanup = func() { prevCleanup(); restoreCfg() }

pkg/config/system.go

@@ -292,9 +292,17 @@ func (s *SystemConfig) getZoneConfigForKey(id uint32, keySuffix []byte) (*ZoneCo
 	if entry.zone != nil {
 		if entry.placeholder != nil {
 			if subzone := entry.placeholder.GetSubzoneForKeySuffix(keySuffix); subzone != nil {
+				if indexSubzone := entry.placeholder.GetSubzone(subzone.IndexID, ""); indexSubzone != nil {
+					subzone.Config.InheritFromParent(indexSubzone.Config)
+				}
+				subzone.Config.InheritFromParent(*entry.zone)
 				return &subzone.Config, nil
 			}
 		} else if subzone := entry.zone.GetSubzoneForKeySuffix(keySuffix); subzone != nil {
+			if indexSubzone := entry.zone.GetSubzone(subzone.IndexID, ""); indexSubzone != nil {
+				subzone.Config.InheritFromParent(indexSubzone.Config)
+			}
+			subzone.Config.InheritFromParent(*entry.zone)
 			return &subzone.Config, nil
 		}
 		return entry.zone, nil