Merge #98491 #98504 #98508

98491: roachtest: maybe deflake `change-replicas/mixed-version` r=erikgrinaker a=erikgrinaker

Touches #98429.

Epic: none
Release note: None

98504: upgrade: Remove two to-be-deleted-V22_2 cluster versions r=Xiang-Gu a=Xiang-Gu

This commit removed the following two cluster versions and its associated upgrade logic and tests:
 - V22_2UpgradeSequenceToBeReferencedByID
 - V22_2UpdateInvalidColumnIDsInSequenceBackReferences

Informs: #96763, #96751
Release Note: None

98508: kv: deflake TestAbortCountConflictingWrites r=nvanbenschoten a=nvanbenschoten

Fixes #96839.

The test was made flaky by 5129578. See the comment in #96839 (comment) for an explanation.

This commit resolves that flakiness.

Release note: None

Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
Co-authored-by: Xiang Gu <xiang@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>

Author: 4 people

pkg/clusterversion/cockroach_versions.go

@@ -203,11 +203,6 @@ const (
 	TODODelete_V22_2RemoveGrantPrivilege
 	// TODODelete_V22_2MVCCRangeTombstones enables the use of MVCC range tombstones.
 	TODODelete_V22_2MVCCRangeTombstones
-	// TODODelete_V22_2UpgradeSequenceToBeReferencedByID ensures that sequences are referenced
-	// by IDs rather than by their names. For example, a column's DEFAULT (or
-	// ON UPDATE) expression can be defined to be 'nextval('s')'; we want to be
-	// able to refer to sequence 's' by its ID, since 's' might be later renamed.
-	TODODelete_V22_2UpgradeSequenceToBeReferencedByID
 	// TODODelete_V22_2SampledStmtDiagReqs enables installing statement diagnostic requests that
 	// probabilistically collects stmt bundles, controlled by the user provided
 	// sampling rate.
@@ -273,10 +268,6 @@ const (
 	// schema changes to complete. After this point, no non-MVCC
 	// AddSSTable calls will be used outside of tenant streaming.
 	TODODelete_V22_2NoNonMVCCAddSSTable
-	// TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences looks for invalid column
-	// ids in sequences' back references and attempts a best-effort-based matching
-	// to update those column IDs.
-	TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences
 	// TODODelete_V22_2TTLDistSQL uses DistSQL to distribute TTL SELECT/DELETE statements to
 	// leaseholder nodes.
 	TODODelete_V22_2TTLDistSQL
@@ -579,10 +570,6 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     TODODelete_V22_2MVCCRangeTombstones,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 16},
 	},
-	{
-		Key:     TODODelete_V22_2UpgradeSequenceToBeReferencedByID,
-		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 18},
-	},
 	{
 		Key:     TODODelete_V22_2SampledStmtDiagReqs,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 20},
@@ -655,10 +642,6 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     TODODelete_V22_2NoNonMVCCAddSSTable,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 62},
 	},
-	{
-		Key:     TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences,
-		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 66},
-	},
 	{
 		Key:     TODODelete_V22_2TTLDistSQL,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 68},

pkg/cmd/roachtest/tests/mixed_version_change_replicas.go

@@ -125,10 +125,11 @@ func runChangeReplicasMixedVersion(ctx context.Context, t test.Test, c cluster.C
 					InitialBackoff: 100 * time.Millisecond,
 					MaxBackoff:     5 * time.Second,
 					Multiplier:     2,
-					MaxRetries:     8,
+					MaxRetries:     12,
 				}
 				var rangeErrors map[int]string
 				for r := retry.StartWithCtx(ctx, retryOpts); r.Next(); {
+					setReplicateQueueEnabled(false)
 					if errCount := len(rangeErrors); errCount > 0 {
 						t.L().Printf("%d ranges failed, retrying", errCount)
 					}
@@ -153,6 +154,10 @@ func runChangeReplicasMixedVersion(ctx context.Context, t test.Test, c cluster.C
 					if len(rangeErrors) == 0 {
 						break
 					}
+					// The failure may be caused by conflicts with ongoing configuration
+					// changes by the replicate queue, so we re-enable it and let it run
+					// for a bit before the next retry.
+					setReplicateQueueEnabled(true)
 				}
 
 				if len(rangeErrors) > 0 {

pkg/testutils/storageutils/mocking.go

@@ -96,5 +96,8 @@ func (c *ReplayProtectionFilterWrapper) run(args kvserverbase.FilterArgs) *kvpb.
 	c.Unlock()
 
 	res := future.WaitForResult(args.Ctx)
+	if res.Err != nil {
+		return kvpb.NewError(res.Err)
+	}
 	return shallowCloneErrorWithTxn(res.Val.(*kvpb.Error))
 }

pkg/upgrade/upgrades/BUILD.bazel

@@ -29,8 +29,6 @@ go_library(
         "system_privileges_user_id_migration.go",
         "system_rbr_indexes.go",
         "tenant_table_migration.go",
-        "update_invalid_column_ids_in_sequence_back_references.go",
-        "upgrade_sequence_to_be_referenced_by_ID.go",
         "upgrades.go",
         "wait_for_del_range_in_gc_job.go",
         "wait_for_schema_changes.go",
@@ -50,21 +48,16 @@ go_library(
         "//pkg/roachpb",
         "//pkg/security/username",
         "//pkg/settings/cluster",
-        "//pkg/sql",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/catalogkeys",
-        "//pkg/sql/catalog/descbuilder",
         "//pkg/sql/catalog/descidgen",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/descs",
-        "//pkg/sql/catalog/resolver",
         "//pkg/sql/catalog/schematelemetry/schematelemetrycontroller",
-        "//pkg/sql/catalog/seqexpr",
         "//pkg/sql/catalog/systemschema",
         "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/enum",
         "//pkg/sql/isql",
-        "//pkg/sql/parser",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/schemachanger/scpb",
@@ -117,8 +110,6 @@ go_test(
         "system_privileges_user_id_migration_test.go",
         "system_rbr_indexes_test.go",
         "tenant_table_migration_test.go",
-        "update_invalid_column_ids_in_sequence_back_references_external_test.go",
-        "upgrade_sequence_to_be_referenced_by_ID_external_test.go",
         "wait_for_del_range_in_gc_job_test.go",
         "wait_for_schema_changes_test.go",
         "web_sessions_table_user_id_migration_test.go",