Merge #154879 #155775 #155794

154879: changefeedccl: deflake TestChangefeedSchemaTTL r=fqazi a=fqazi

Previously, TestChangefeedSchemaTTL started flaking when create_table_with_schema_locked was enabled by default in the test suite. This happened because schema_locked tables can cause historical versions of descriptors to be cached in the lease manager, as the changefeed logic intentionally queries prior versions. As a result, even after the test garbage-collected the descriptors table, these old versions could remain in the in-memory cache, leading to flakes. To address this, this patch adds a canary schema change, which bumps the descriptor's version. When a descriptor's version is bumped, all older cached versions are purged.

Fixes: #149167
Informs: #153138
Informs: #154229
Informs: #154304
Informs: #154931

Release note: None

155775: mixedversion: fix panic in refresh{Cluster,Binary}Version r=golgeek a=DarrylWong

A recent change #155713 fixed the indexing when nodes were unavailable. However, this change did not account for multi cluster tests where the roachprod node ID is not the same as the CRDB node ID, i.e. we can't assume the 4th VM is the same as the 4th node in the cluster.

Fixes: none
Epic: none
Release note: none

155794: logictest: finalize upgrade explicitly in mixed_version_upgrade_preserve_ttl r=rafiss a=rafiss

Replace retry-based waiting with explicit version finalization using SET CLUSTER SETTING version = crdb_internal.node_executable_version().

Resolves: #155429
Epic: None

Release note: None
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com>
Co-authored-by: DarrylWong <darryl@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>

Author: 4 people

pkg/ccl/changefeedccl/changefeed_test.go

@@ -7044,6 +7044,7 @@ func TestChangefeedSchemaTTL(t *testing.T) {
 		// Create the data table; it will only contain a single row with multiple
 		// versions.
 		sqlDB.Exec(t, `CREATE TABLE foo (a INT PRIMARY KEY, b STRING)`)
+		sqlDB.Exec(t, `CREATE USER joe`)
 
 		counter := 0
 		upsertRow := func() {
@@ -7074,6 +7075,15 @@ func TestChangefeedSchemaTTL(t *testing.T) {
 		// table to be deleted, with the middle version being lost to the changefeed.
 		forceTableGC(t, s.SystemServer, sqlDB, "system", "descriptor")
 
+		// Do an unnecessary version bump on the descriptor, which will purge old
+		// versions of the descriptor that the lease manager may have cached for
+		// historical queries. When schema_locked support is active, the act of
+		// detecting polling inside pauseOrResumePolling can cause old versions
+		// to be cached. The historical descriptors are cleared everytime a
+		// version bump occurs, otherwise this test can flake if the prior versions
+		// need to decode a row is already cached.
+		waitForSchemaChange(t, sqlDB, "ALTER TABLE foo OWNER TO joe")
+
 		// Resume our changefeed normally.
 		atomic.StoreInt32(&shouldWait, 0)
 		resume <- struct{}{}

pkg/cmd/roachtest/option/node_list_option.go

@@ -42,6 +42,15 @@ func (n NodeListOption) Equals(o NodeListOption) bool {
 	return true
 }
 
+func (n NodeListOption) Contains(node int) bool {
+	for _, n := range n {
+		if n == node {
+			return true
+		}
+	}
+	return false
+}
+
 // Option implements Option.
 func (n NodeListOption) Option() {}
 

pkg/cmd/roachtest/roachtestutil/mixedversion/runner.go

@@ -525,16 +525,26 @@ func (tr *testRunner) refreshBinaryVersions(ctx context.Context, service *servic
 	defer cancel()
 
 	group := ctxgroup.WithContext(connectionCtx)
-	for _, node := range tr.getAvailableNodes(service.descriptor) {
+	// We still attempt to refresh binary versions on nodes we expect to be unavailable:
+	// 	1. Some failure injections are overly conservative in marking nodes as
+	//	   unavailable out of caution (e.g. network partitions).
+	//	2. The monitor returns the roachprod node ID, which may be offset compared
+	//		 to what the mixed-version test expects, e.g. a multi cluster test where
+	//		 node 1 may be the 5th VM in the roachprod cluster.
+	availableNodes := tr.getAvailableNodes(service.descriptor)
+	for j, node := range service.descriptor.Nodes {
 		group.GoCtx(func(ctx context.Context) error {
 			bv, err := clusterupgrade.BinaryVersion(ctx, tr.conn(node, service.descriptor.Name))
 			if err != nil {
+				if !availableNodes.Contains(node) {
+					return nil
+				}
 				return fmt.Errorf(
 					"failed to get binary version for node %d (%s): %w",
 					node, service.descriptor.Name, err,
 				)
 			}
-			newBinaryVersions[node-1] = bv
+			newBinaryVersions[j] = bv
 			return nil
 		})
 	}
@@ -556,17 +566,28 @@ func (tr *testRunner) refreshClusterVersions(ctx context.Context, service *servi
 	defer cancel()
 
 	group := ctxgroup.WithContext(connectionCtx)
-	for _, node := range tr.getAvailableNodes(service.descriptor) {
+	// We still attempt to refresh cluster versions on nodes we expect to be unavailable:
+	// 	1. Some failure injections are overly conservative in marking nodes as
+	//	   unavailable out of caution (e.g. network partitions).
+	//	2. The monitor returns the roachprod node ID, which may be offset compared
+	//		 to what the mixed-version test expects, e.g. a multi cluster test where
+	//		 node 1 may be the 5th VM in the roachprod cluster.
+	availableNodes := tr.getAvailableNodes(service.descriptor)
+	for j, node := range service.descriptor.Nodes {
 		group.GoCtx(func(ctx context.Context) error {
 			cv, err := clusterupgrade.ClusterVersion(ctx, tr.conn(node, service.descriptor.Name))
 			if err != nil {
+				if !availableNodes.Contains(node) {
+					return nil
+				}
+
 				return fmt.Errorf(
 					"failed to get cluster version for node %d (%s): %w",
 					node, service.descriptor.Name, err,
 				)
 			}
 
-			newClusterVersions[node-1] = cv
+			newClusterVersions[j] = cv
 			return nil
 		})
 	}

pkg/sql/logictest/testdata/logic_test/mixed_version_upgrade_preserve_ttl

@@ -13,10 +13,12 @@ CREATE TABLE tbl (
 
 upgrade all
 
-# Verify that the cluster version upgrades have begun by asserting we're no
-# longer on the previous version. Note that the first cluster upgrade is the
-# one that repairs all descriptors.
-query B retry retry_duration 90s
+statement ok
+SET CLUSTER SETTING version = crdb_internal.node_executable_version()
+
+# Verify that the cluster version has been upgraded and is no longer on the
+# previous version.
+query B
 SELECT version != '$initial_version' FROM [SHOW CLUSTER SETTING version]
 ----
 true