Merge pull request #110569 from cockroachdb/blathers/backport-staging…

…-v23.1.10-110500

staging-v23.1.10: release-23.1: upgrade: remove buggy TTL repair

pkg/sql/catalog/tabledesc/table_desc_builder.go

@@ -244,13 +244,6 @@ func (tdb *tableDescriptorBuilder) StripDanglingBackReferences(
 			tdb.changes.Add(catalog.StrippedDanglingBackReferences)
 		}
 	}
-	// ... in the row_level_ttl field,
-	if ttl := tbl.RowLevelTTL; ttl != nil {
-		if id := jobspb.JobID(ttl.ScheduleID); id != jobspb.InvalidJobID && !nonTerminalJobIDMightExist(id) {
-			tbl.RowLevelTTL = nil
-			tdb.changes.Add(catalog.StrippedDanglingBackReferences)
-		}
-	}
 	// ... in the sequence ownership field.
 	if seq := tbl.SequenceOpts; seq != nil {
 		if id := seq.SequenceOwner.OwnerTableID; id != descpb.InvalidID && !descIDMightExist(id) {

pkg/sql/catalog/tabledesc/table_desc_test.go

@@ -15,7 +15,6 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/stretchr/testify/require"
 )
@@ -116,9 +115,6 @@ func TestStripDanglingBackReferences(t *testing.T) {
 					{MutationID: 2},
 				},
 				DropJobID: 1,
-				RowLevelTTL: &catpb.RowLevelTTL{
-					ScheduleID: 123,
-				},
 			},
 			expectedOutput: descpb.TableDescriptor{
 				Name: "foo",

pkg/sql/logictest/logic.go

@@ -3051,34 +3051,45 @@ func (t *logicTest) processSubtest(
 			if t.testserverCluster == nil {
 				return errors.Errorf(`could not perform "upgrade", not a cockroach-go/testserver cluster`)
 			}
-			nodeIdx, err := strconv.Atoi(fields[1])
-			if err != nil {
-				t.Fatal(err)
-			}
-			if err := t.testserverCluster.UpgradeNode(nodeIdx); err != nil {
-				t.Fatal(err)
-			}
-			for i := 0; i < t.cfg.NumNodes; i++ {
-				// Wait for each node to be reachable, since UpgradeNode uses `kill`
-				// to terminate nodes, and may introduce temporary unavailability in
-				// the system range.
-				if err := t.testserverCluster.WaitForInitFinishForNode(i); err != nil {
+			upgradeNode := func(nodeIdx int) {
+				if err := t.testserverCluster.UpgradeNode(nodeIdx); err != nil {
 					t.Fatal(err)
 				}
-			}
-			// The port may have changed, so we must remove all the cached connections
-			// to this node.
-			for _, m := range t.clients {
-				if c, ok := m[nodeIdx]; ok {
-					_ = c.Close()
+				for i := 0; i < t.cfg.NumNodes; i++ {
+					// Wait for each node to be reachable, since UpgradeNode uses `kill`
+					// to terminate nodes, and may introduce temporary unavailability in
+					// the system range.
+					if err := t.testserverCluster.WaitForInitFinishForNode(i); err != nil {
+						t.Fatal(err)
+					}
+				}
+				// The port may have changed, so we must remove all the cached connections
+				// to this node.
+				for _, m := range t.clients {
+					if c, ok := m[nodeIdx]; ok {
+						_ = c.Close()
+					}
+					delete(m, nodeIdx)
+				}
+				// If we upgraded the node we are currently on, we need to open a new
+				// connection since the previous one might now be invalid.
+				if t.nodeIdx == nodeIdx {
+					t.setUser(t.user, nodeIdx)
 				}
-				delete(m, nodeIdx)
 			}
-			// If we upgraded the node we are currently on, we need to open a new
-			// connection since the previous one might now be invalid.
-			if t.nodeIdx == nodeIdx {
-				t.setUser(t.user, nodeIdx)
+			nodeStr := fields[1]
+			if nodeStr == "all" {
+				for i := 0; i < t.cfg.NumNodes; i++ {
+					upgradeNode(i)
+				}
+			} else {
+				nodeIdx, err := strconv.Atoi(nodeStr)
+				if err != nil {
+					t.Fatal(err)
+				}
+				upgradeNode(nodeIdx)
 			}
+
 		default:
 			return errors.Errorf("%s:%d: unknown command: %s",
 				path, s.Line+subtest.lineLineIndexIntoFile, cmd,

pkg/sql/logictest/testdata/logic_test/mixed_version_upgrade_repair_descriptors

@@ -0,0 +1,22 @@
+# LogicTest: cockroach-go-testserver-upgrade-to-master
+
+statement ok
+CREATE TABLE tbl (
+  id INT PRIMARY KEY
+) WITH (ttl_expire_after = '10 minutes')
+
+upgrade all
+
+query B retry
+SELECT version LIKE '%22.2-%' FROM [SHOW CLUSTER SETTING version]
+----
+true
+
+query T
+SELECT create_statement FROM [SHOW CREATE TABLE tbl]
+----
+CREATE TABLE public.tbl (
+  id INT8 NOT NULL,
+  crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
+  CONSTRAINT tbl_pkey PRIMARY KEY (id ASC)
+) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@hourly')

pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/BUILD.bazel

@@ -10,7 +10,7 @@ go_test(
         "//pkg/cmd/cockroach-short",  # keep
         "//pkg/sql/logictest:testdata",  # keep
     ],
-    shard_count = 8,
+    shard_count = 9,
     tags = ["cpu:2"],
     deps = [
         "//pkg/build/bazel",

pkg/upgrade/upgrades/first_upgrade.go

@@ -17,10 +17,14 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/upgrade"
+	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
 )
 
+// RunFirstUpgradePrecondition short-circuits FirstUpgradeFromReleasePrecondition if set to false.
+var RunFirstUpgradePrecondition = envutil.EnvOrDefaultBool("COCKROACH_RUN_FIRST_UPGRADE_PRECONDITION", false)
+
 // FirstUpgradeFromReleasePrecondition is the precondition check for upgrading
 // from any supported major release.
 //
@@ -38,6 +42,9 @@ import (
 func FirstUpgradeFromReleasePrecondition(
 	ctx context.Context, _ clusterversion.ClusterVersion, d upgrade.TenantDeps,
 ) error {
+	if !RunFirstUpgradePrecondition {
+		return nil
+	}
 	// For performance reasons, we look back in time when performing
 	// a diagnostic query. If no corruptions were found back then, we assume that
 	// there are no corruptions now. Otherwise, we retry and do everything

pkg/upgrade/upgrades/first_upgrade_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
+	"github.com/cockroachdb/cockroach/pkg/upgrade/upgrades"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/stretchr/testify/require"
@@ -39,6 +40,8 @@ func TestFirstUpgrade(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	upgrades.RunFirstUpgradePrecondition = true
+
 	var (
 		v0 = clusterversion.TestingBinaryMinSupportedVersion
 		v1 = clusterversion.ByKey(clusterversion.BinaryVersionKey)
@@ -112,6 +115,8 @@ func TestFirstUpgradeRepair(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	upgrades.RunFirstUpgradePrecondition = true
+
 	var (
 		v0 = clusterversion.TestingBinaryMinSupportedVersion
 		v1 = clusterversion.ByKey(clusterversion.BinaryVersionKey)
@@ -144,7 +149,7 @@ func TestFirstUpgradeRepair(t *testing.T) {
 	execStmts(t,
 		"CREATE DATABASE test",
 		"USE test",
-		"CREATE TABLE foo (i INT PRIMARY KEY, j INT, INDEX idx(j))",
+		"CREATE TABLE foo (i INT PRIMARY KEY, j INT, INDEX idx(j)) WITH (ttl_expire_after = '10m')",
 		"INSERT INTO foo VALUES (1, 2)",
 		"CREATE FUNCTION test.public.f() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$",
 	)