Merge #152634

152634: upgrades: one-time rewrite of all descriptors r=rafiss a=rafiss

This will allow us to safely remove the upgradeType logic when we drop
25.3 compatibility.

informs #152629
Release note: None



Co-authored-by: Rafi Shamim <[email protected]>

Author: craig[bot]

pkg/sql/types/types.go

@@ -2537,11 +2537,8 @@ func (t *T) Unmarshal(data []byte) error {
 // setting required values. This is necessary to preserve backwards-
 // compatibility with older formats (e.g. restoring database from old backup).
 //
-// This can be removed once we can ensure that all descriptors have been
-// rewritten with the newer format. Note that the migration in first_upgrade.go
-// is not sufficient to ensure that all descriptors have been rewritten, since
-// that migration is only based on post-deserialization changes, but the type
-// upgrade logic happens _during_ deserialization.
+// This can be removed once we no longer need compatibility with v25.3 or
+// earlier. See https://github.com/cockroachdb/cockroach/issues/152629.
 func (t *T) upgradeType() error {
 	switch t.Family() {
 	case IntFamily:

pkg/upgrade/upgrades/BUILD.bazel

@@ -43,7 +43,6 @@ go_library(
         "//pkg/sql/catalog/descidgen",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/descs",
-        "//pkg/sql/catalog/lease",
         "//pkg/sql/catalog/nstree",
         "//pkg/sql/catalog/replication",
         "//pkg/sql/catalog/systemschema",

pkg/upgrade/upgrades/first_upgrade.go

@@ -8,6 +8,7 @@ package upgrades
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/kv"
@@ -16,7 +17,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog/lease"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/upgrade"
@@ -79,7 +79,7 @@ func upgradeDescriptors(
 	batchSize := 100
 	// Any batch size below this will use high priority.
 	const HighPriBatchSize = 25
-	repairBatchTimeLimit := lease.LeaseDuration.Get(&d.Settings.SV)
+	repairBatchTimeLimit := 1 * time.Minute
 	currentIdx := 0
 	idsToRewrite := ids.Ordered()
 	for currentIdx <= len(idsToRewrite) {
@@ -102,7 +102,18 @@ func upgradeDescriptors(
 				b := txn.KV().NewBatch()
 				for _, mut := range muts {
 					if !mut.GetPostDeserializationChanges().HasChanges() {
-						continue
+						// In the upgrade to 25.4, we do a one-time rewrite of all
+						// descriptors in order to upgrade them to use the new type
+						// serialization format.
+						// See https://github.com/cockroachdb/cockroach/issues/152629.
+						if d.Settings.Version.IsActive(ctx, clusterversion.V25_4) {
+							continue
+						}
+						// Skip the unconditional rewrite if this is a database descriptor,
+						// as those never reference types.
+						if mut.DescriptorType() == catalog.Database {
+							continue
+						}
 					}
 					key := catalogkeys.MakeDescMetadataKey(d.Codec, mut.GetID())
 					b.CPut(key, mut.DescriptorProto(), mut.GetRawBytesInStorage())
@@ -215,7 +226,7 @@ WHERE
 		batchSize := 100
 		// Any batch size below this will use high priority.
 		const HighPriBatchSize = 25
-		repairBatchTimeLimit := lease.LeaseDuration.Get(&d.Settings.SV)
+		repairBatchTimeLimit := 1 * time.Minute
 		for {
 			var rowsUpdated tree.DInt
 			err := timeutil.RunWithTimeout(ctx, "descriptor-repair", repairBatchTimeLimit, func(ctx context.Context) error {

pkg/upgrade/upgrades/first_upgrade_test.go

@@ -27,7 +27,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog/lease"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
@@ -164,7 +163,6 @@ func TestFirstUpgradeRepair(t *testing.T) {
 	// also holds a lease on the system database descriptor, which we will wait to
 	// be released. Reducing the lease duration makes this part of the test speed
 	// up.
-	lease.LeaseDuration.Override(ctx, &settings.SV, time.Second*30)
 	require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV))
 	upgradePausePoint := make(chan struct{})
 	upgradeResumePoint := make(chan struct{})
@@ -377,7 +375,6 @@ func TestFirstUpgradeRepairBatchSize(t *testing.T) {
 	// also holds a lease on the system database descriptor, which we will wait to
 	// be released. Reducing the lease duration makes this part of the test speed
 	// up.
-	lease.LeaseDuration.Override(ctx, &settings.SV, time.Second*30)
 	require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV))
 	testServer, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{
 		Settings: settings,