Merge #133216 #133217 #134411 #134875

133216: roachtest: add kv/splits/nodes=3/quiesce=false/lease=leader r=nvanbenschoten a=nvanbenschoten

Part of #132762.

This commit adds a `kv/splits/nodes=3/quiesce=false/lease=leader` test. It should be able to support 80k ranges even without quiescence, because leader leases use store liveness for failure detection and lease extension.

To push higher than this, we will likely need to address #133885.

Release note: None

133217: roachtest: metamorphically enable leader leases r=nvanbenschoten a=nvanbenschoten

Part of #132762.

To increase test coverage of leader leases, this patch adds leader leases to the set of possible lease types when tests opt-in to metamorphic leases. As of fc68b0f, most roachtests do enable metamorphic leases, so this will provide good coverage of leader leases.

Before merging this, we should manually run all of these tests with leader leases to verify that they pass.

Release note: None

134411: sql/schemachanger: Prevent Temporary Disappearance of Altered Columns r=spilchen a=spilchen

Previously, during an ALTER COLUMN ... TYPE operation, the column being modified could briefly disappear, leading to "column does not exist" errors for any queries attempting to access it. This issue arose from the incorrect ordering of dependency rules, which has now been addressed.

The changes to the dependency rules during the column type alteration are as follows:
- The name of the new column is not set until we are ready to make it public. Previously, the column name was assigned too early in the pre-commit phase, which necessitated additional shadow transient column name logic. This new approach simplifies the process and eliminates the need for that extra rename logic.
- The ColumnName and Column elements for both the old and new columns are swapped within the same stage.

I have also maintained the existing rules for cases where the column type is not being altered by utilizing the IsAlterColumnTypeOp and IsNotAlterColumnTypeOp predicates.

I updated existing tests to query the column during each stage of the alter. This allowed me to reproduce the issue.

Epic: CRDB-25314
Closes #133996
Release note: none

134875: license: unredact logs written by license enforcer r=rafiss a=rafiss

This makes it so log messages are not redacted unnecessarily.

- Use redact.StringBuilder instead of strings.Builder.
- Avoid using `.String()` arguments for log.Infof, since strings are always redacted.
- Mark license type as a redact.SafeValue.

Epic: None
Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

pkg/ccl/schemachangerccl/testdata/decomp/multiregion

@@ -190,37 +190,31 @@ ElementState:
     tableId: 110
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 1
     name: b
     tableId: 110
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 2
     name: rowid
     tableId: 110
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967292e+09
     name: crdb_internal_origin_timestamp
     tableId: 110
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967293e+09
     name: crdb_internal_origin_id
     tableId: 110
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967294e+09
     name: tableoid
     tableId: 110
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967295e+09
     name: crdb_internal_mvcc_timestamp
     tableId: 110
@@ -591,37 +585,31 @@ ElementState:
     tableId: 109
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 1
     name: a
     tableId: 109
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 2
     name: rowid
     tableId: 109
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967292e+09
     name: crdb_internal_origin_timestamp
     tableId: 109
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967293e+09
     name: crdb_internal_origin_id
     tableId: 109
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967294e+09
     name: tableoid
     tableId: 109
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967295e+09
     name: crdb_internal_mvcc_timestamp
     tableId: 109
@@ -1021,43 +1009,36 @@ ElementState:
     tableId: 108
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 1
     name: k
     tableId: 108
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 2
     name: v
     tableId: 108
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 3
     name: crdb_region
     tableId: 108
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967292e+09
     name: crdb_internal_origin_timestamp
     tableId: 108
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967293e+09
     name: crdb_internal_origin_id
     tableId: 108
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967294e+09
     name: tableoid
     tableId: 108
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967295e+09
     name: crdb_internal_mvcc_timestamp
     tableId: 108

pkg/ccl/schemachangerccl/testdata/decomp/partitioning

@@ -111,43 +111,36 @@ ElementState:
     tableId: 104
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 1
     name: pk
     tableId: 104
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 2
     name: a
     tableId: 104
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 3
     name: j
     tableId: 104
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967292e+09
     name: crdb_internal_origin_timestamp
     tableId: 104
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967293e+09
     name: crdb_internal_origin_id
     tableId: 104
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967294e+09
     name: tableoid
     tableId: 104
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967295e+09
     name: crdb_internal_mvcc_timestamp
     tableId: 104
@@ -631,37 +624,31 @@ ElementState:
     tableId: 105
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 1
     name: a
     tableId: 105
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 2
     name: b
     tableId: 105
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967292e+09
     name: crdb_internal_origin_timestamp
     tableId: 105
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967293e+09
     name: crdb_internal_origin_id
     tableId: 105
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967294e+09
     name: tableoid
     tableId: 105
   Status: PUBLIC
 - ColumnName:
-    absentName: ""
     columnId: 4.294967295e+09
     name: crdb_internal_mvcc_timestamp
     tableId: 105

pkg/cmd/roachtest/registry/test_spec.go

@@ -244,10 +244,14 @@ const (
 	LeaderLeases
 	// MetamorphicLeases randomly chooses epoch or expiration
 	// leases (across the entire cluster).
-	// TODO(nvanbenschoten): add leader leases to this mix.
 	MetamorphicLeases
 )
 
+// LeaseTypes contains all lease types.
+//
+// The list does not contain aliases like "default" and "metamorphic".
+var LeaseTypes = []LeaseType{EpochLeases, ExpirationLeases, LeaderLeases}
+
 // CloudSet represents a set of clouds.
 //
 // Instances of CloudSet are immutable. The uninitialized (zero) value is not

pkg/cmd/roachtest/test_runner.go

@@ -900,7 +900,18 @@ func (r *testRunner) runWorker(
 				c.clusterSettings = map[string]string{}
 				c.virtualClusterSettings = map[string]string{}
 
-				switch testSpec.Leases {
+				leases := testSpec.Leases
+				if leases == registry.MetamorphicLeases {
+					// 50% change of using the default lease type, 50% change of choosing
+					// a random, specific lease type.
+					if prng.Intn(2) == 0 {
+						leases = registry.DefaultLeases
+					} else {
+						leases = registry.LeaseTypes[prng.Intn(len(registry.LeaseTypes))]
+					}
+					c.status(fmt.Sprintf("metamorphically using %s leases", leases))
+				}
+				switch leases {
 				case registry.DefaultLeases:
 				case registry.EpochLeases:
 					c.clusterSettings["kv.expiration_leases_only.enabled"] = "false"
@@ -911,12 +922,9 @@ func (r *testRunner) runWorker(
 					c.clusterSettings["kv.expiration_leases_only.enabled"] = "false"
 					c.clusterSettings["kv.raft.leader_fortification.fraction_enabled"] = "1.0"
 				case registry.MetamorphicLeases:
-					enabled := prng.Float64() < 0.5
-					c.status(fmt.Sprintf("metamorphically setting kv.expiration_leases_only.enabled = %t",
-						enabled))
-					c.clusterSettings["kv.expiration_leases_only.enabled"] = fmt.Sprintf("%t", enabled)
+					t.Fatalf("metamorphic leases handled above")
 				default:
-					t.Fatalf("unknown lease type %s", testSpec.Leases)
+					t.Fatalf("unknown lease type %s", leases)
 				}
 
 				c.goCoverDir = t.GoCoverArtifactsDir()

pkg/cmd/roachtest/tests/failover.go

@@ -59,8 +59,7 @@ var rangeLeaseRenewalDuration = func() time.Duration {
 // requests are successful with nominal latencies. See also:
 // https://github.com/cockroachdb/cockroach/issues/103654
 func registerFailover(r registry.Registry) {
-	leaseTypes := []registry.LeaseType{registry.EpochLeases, registry.ExpirationLeases, registry.LeaderLeases}
-	for _, leases := range leaseTypes {
+	for _, leases := range registry.LeaseTypes {
 		var leasesStr string
 		switch leases {
 		case registry.EpochLeases:

pkg/cmd/roachtest/tests/kv.go

@@ -719,15 +719,22 @@ func registerKVSplits(r registry.Registry) {
 		// far past the number of replicas per node we support, at least if the
 		// ranges start to unquiesce (which can set off a cascade due to resource
 		// exhaustion).
-		{true, 300000, registry.EpochLeases, 2 * time.Hour},
+		{true, 300_000, registry.EpochLeases, 2 * time.Hour},
 		// This version of the test prevents range quiescence to trigger the
 		// badness described above more reliably for when we wish to improve
 		// the performance. For now, just verify that 30k unquiesced ranges
 		// is tenable.
-		{false, 30000, registry.EpochLeases, 2 * time.Hour},
+		{false, 30_000, registry.EpochLeases, 2 * time.Hour},
 		// Expiration-based leases prevent quiescence, and are also more expensive
 		// to keep alive. Again, just verify that 30k ranges is ok.
-		{false, 30000, registry.ExpirationLeases, 2 * time.Hour},
+		{false, 30_000, registry.ExpirationLeases, 2 * time.Hour},
+		// Leader leases don't need quiescence, as they use store liveness for
+		// failure detection and lease extension, so they don't issue raft
+		// heartbeats or periodic lease extensions. However, the cost of raft
+		// ticking is not entirely negligible (see #133885), so each range isn't
+		// completely free. Currently, they should be able to support 80k ranges in
+		// this cluster configuration.
+		{false, 80_000, registry.LeaderLeases, 2 * time.Hour},
 	} {
 		item := item // for use in closure below
 		r.Add(registry.TestSpec{

pkg/server/license/BUILD.bazel

@@ -24,6 +24,7 @@ go_library(
         "//pkg/util/syncutil",
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_errors//:errors",
+        "@com_github_cockroachdb_redact//:redact",
     ],
 )
 

pkg/server/license/cclbridge.go

@@ -9,6 +9,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/redact"
 )
 
 // This file serves as a bridge to the license code in the CCL packages.
@@ -24,6 +25,11 @@ var RegisterCallbackOnLicenseChange = func(context.Context, *cluster.Settings, *
 // enforcer.
 type LicType int
 
+var _ redact.SafeValue = LicType(0)
+
+// SafeValue implements the redact.SafeValue interface.
+func (i LicType) SafeValue() {}
+
 //go:generate stringer -type=LicType -linecomment
 const (
 	LicTypeNone       LicType = iota // none