Merge #76154 #86503 #86506 #86535

76154: migrations: add migration to wait on in-flight schema changes r=dt a=stevendanna

Eventually, we want to stop accepting non-MVCC AddSSTables. To do
this, we need all CREATE INDEX schema changes to use the new
mvcc-compatible process. To make this more tenable, we want to drain
in-flight schema changes during a migration.

Release note: Finalizing an upgrade to 22.2 requires that all
in-flight schema changes enter a terminal state. This may mean that
finalization takes as long as the longest-running schema change.

Release justification: Critical migration to ensure that 22.2's assumptions
about non-MVCC operations are true.

86503: ui: create new cell types for different insights r=maryliag a=maryliag

Adding the new types for high retry, suboptimal plan
and failed to the insights table component.
That can be used on Workload Statement Details page.

Release justification: low risk, high benefit change
Release note: None

86506: ui: create format function from hex to int64 r=maryliag a=maryliag

Creation of a function that converts from hex
to int64 in string format, so we don't have the
problems of js conversions.

Release justification: low risk change
Release note: None

86535: ui: general fixes on transaction insights page r=maryliag a=maryliag

This commit fixes a few bugs on the Workload Insights
page - transactions:
- Proper selection of the message when empty results
- Adjust some styling

Release justification: bug fixes
Release note: None

Co-authored-by: Steven Danna <danna@cockroachlabs.com>
Co-authored-by: Marylia Gutierrez <marylia@cockroachlabs.com>

docs/generated/settings/settings-for-tenants.txt

@@ -292,4 +292,4 @@ trace.jaeger.agent	string		the address of a Jaeger agent to receive traces using
 trace.opentelemetry.collector	string		address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.
 trace.span_registry.enabled	boolean	true	if set, ongoing traces can be seen at https://<ui>/#/debug/tracez
 trace.zipkin.collector	string		the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
-version	version	22.1-60	set the active cluster version in the format '<major>.<minor>'
+version	version	22.1-62	set the active cluster version in the format '<major>.<minor>'

docs/generated/settings/settings.html

@@ -223,6 +223,6 @@
 <tr><td><code>trace.opentelemetry.collector</code></td><td>string</td><td><code></code></td><td>address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.</td></tr>
 <tr><td><code>trace.span_registry.enabled</code></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://<ui>/#/debug/tracez</td></tr>
 <tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.</td></tr>
-<tr><td><code>version</code></td><td>version</td><td><code>22.1-60</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
+<tr><td><code>version</code></td><td>version</td><td><code>22.1-62</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
 </tbody>
 </table>

pkg/ccl/backupccl/BUILD.bazel

@@ -273,6 +273,7 @@ go_test(
         "//pkg/util/syncutil",
         "//pkg/util/timeutil",
         "//pkg/util/uuid",
+        "//pkg/util/version",
         "//pkg/workload/bank",
         "//pkg/workload/workloadsql",
         "@com_github_aws_aws_sdk_go//aws/credentials",

pkg/ccl/backupccl/restore_job.go

@@ -1901,7 +1901,7 @@ func (r *restoreResumer) publishDescriptors(
 			newIdx := found.IndexDescDeepCopy()
 			mutTable.RemovePublicNonPrimaryIndex(found.Ordinal())
 			if err := mutTable.AddIndexMutationMaybeWithTempIndex(
-				ctx, &newIdx, descpb.DescriptorMutation_ADD, r.settings,
+				&newIdx, descpb.DescriptorMutation_ADD,
 			); err != nil {
 				return err
 			}

pkg/ccl/backupccl/restore_mid_schema_change_test.go

@@ -27,9 +27,15 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/version"
 	"github.com/stretchr/testify/require"
 )
 
+var (
+	clusterRestoreVersion = version.MustParse("v20.1.0")
+	noNonMVCCAddSSTable   = version.MustParse("v22.1.0")
+)
+
 // TestRestoreMidSchemaChanges attempts to RESTORE several BACKUPs that are
 // already constructed and store in
 // ccl/backupccl/testdata/restore_mid_schema_change. These backups were taken on
@@ -79,10 +85,11 @@ func TestRestoreMidSchemaChange(t *testing.T) {
 					t.Run(blockLocation, func(t *testing.T) {
 						versionDirs, err := ioutil.ReadDir(filepath.Join(exportDirs, blockLocation))
 						require.NoError(t, err)
-
 						for _, clusterVersionDir := range versionDirs {
-							if clusterVersionDir.Name() == "19.2" && isClusterRestore {
-								// 19.2 does not support cluster backups.
+							clusterVersion, err := parseMajorVersion(clusterVersionDir.Name())
+							require.NoError(t, err)
+
+							if !clusterVersion.AtLeast(clusterRestoreVersion) && isClusterRestore {
 								continue
 							}
 
@@ -101,7 +108,7 @@ func TestRestoreMidSchemaChange(t *testing.T) {
 									fullBackupDir, err := filepath.Abs(filepath.Join(fullClusterVersionDir, backupDir.Name()))
 									require.NoError(t, err)
 									t.Run(backupDir.Name(), restoreMidSchemaChange(fullBackupDir, backupDir.Name(),
-										isClusterRestore, isSchemaOnly))
+										isClusterRestore, isSchemaOnly, clusterVersion))
 								}
 							})
 						}
@@ -112,9 +119,15 @@ func TestRestoreMidSchemaChange(t *testing.T) {
 	}
 }
 
+// parseMajorVersion parses our major-versioned directory names as if they were
+// full crdb versions.
+func parseMajorVersion(verStr string) (*version.Version, error) {
+	return version.Parse(fmt.Sprintf("v%s.0", verStr))
+}
+
 // expectedSCJobCount returns the expected number of schema change jobs
 // we expect to find.
-func expectedSCJobCount(scName string) int {
+func expectedSCJobCount(scName string, ver *version.Version) int {
 	// The number of schema change under test. These will be the ones that are
 	// synthesized in database restore.
 	var expNumSCJobs int
@@ -126,8 +139,13 @@ func expectedSCJobCount(scName string) int {
 	case "midmultitable":
 		expNumSCJobs = 2 // this test perform a schema change for each table
 	case "midprimarykeyswap":
-		// PK change + PK cleanup
-		expNumSCJobs = 2
+		if ver.AtLeast(noNonMVCCAddSSTable) {
+			// PK change and PK cleanup
+			expNumSCJobs = 2
+		} else {
+			// This will fail so we expect no cleanup job.
+			expNumSCJobs = 1
+		}
 	case "midprimarykeyswapcleanup":
 		expNumSCJobs = 1
 	default:
@@ -179,12 +197,17 @@ func getTablesInTest(scName string) (tableNames []string) {
 }
 
 func verifyMidSchemaChange(
-	t *testing.T, scName string, kvDB *kv.DB, sqlDB *sqlutils.SQLRunner, isSchemaOnly bool,
+	t *testing.T,
+	scName string,
+	kvDB *kv.DB,
+	sqlDB *sqlutils.SQLRunner,
+	isSchemaOnly bool,
+	majorVer *version.Version,
 ) {
 	tables := getTablesInTest(scName)
 
 	// Check that we are left with the expected number of schema change jobs.
-	expNumSchemaChangeJobs := expectedSCJobCount(scName)
+	expNumSchemaChangeJobs := expectedSCJobCount(scName, majorVer)
 
 	synthesizedSchemaChangeJobs := sqlDB.QueryStr(t,
 		"SELECT description FROM crdb_internal.jobs WHERE job_type = 'SCHEMA CHANGE' AND description LIKE '%RESTORING%'")
@@ -201,7 +224,10 @@ func verifyMidSchemaChange(
 }
 
 func restoreMidSchemaChange(
-	backupDir, schemaChangeName string, isClusterRestore bool, isSchemaOnly bool,
+	backupDir, schemaChangeName string,
+	isClusterRestore bool,
+	isSchemaOnly bool,
+	majorVer *version.Version,
 ) func(t *testing.T) {
 	return func(t *testing.T) {
 		ctx := context.Background()
@@ -243,7 +269,7 @@ func restoreMidSchemaChange(
 		// Wait for all jobs to terminate. Some may fail since we don't restore
 		// adding spans.
 		sqlDB.CheckQueryResultsRetry(t, "SELECT * FROM crdb_internal.jobs WHERE job_type = 'SCHEMA CHANGE' AND NOT (status = 'succeeded' OR status = 'failed')", [][]string{})
-		verifyMidSchemaChange(t, schemaChangeName, kvDB, sqlDB, isSchemaOnly)
+		verifyMidSchemaChange(t, schemaChangeName, kvDB, sqlDB, isSchemaOnly, majorVer)
 
 		// Because crdb_internal.invalid_objects is a virtual table, by default, the
 		// query will take a lease on the database sqlDB is connected to and only run

pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/tenant/indexes

@@ -96,12 +96,7 @@ translate database=db table=t
 block-gc-jobs
 ----
 
-# Create an index and create a zone configuration in the same transaction
-# with the mvcc-index-backfiller.
-exec-sql
-SET CLUSTER SETTING sql.mvcc_compliant_index_creation.enabled = true;
-----
-
+# Create an index and create a zone configuration in the same transaction.
 exec-sql
 CREATE INDEX idx2 ON db.t (j);
 ALTER INDEX db.t@idx2 CONFIGURE ZONE USING gc.ttlseconds = 1;
@@ -133,34 +128,8 @@ translate database=db table=t
 /Tenant/10/Table/106/{4-5}                 range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=1 num_replicas=7
 /Tenant/10/Table/10{6/5-7}                 range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=3600 num_replicas=7
 
-# Create an index and create a zone configuration in the same transaction
-# with the non-mvcc-index-backfiller.
-exec-sql
-SET CLUSTER SETTING sql.mvcc_compliant_index_creation.enabled = false;
-----
-
-exec-sql
-CREATE INDEX idx3 ON db.t (j);
-ALTER INDEX db.t@idx3 CONFIGURE ZONE USING gc.ttlseconds = 1;
-----
-
-translate database=db table=t
-----
-/Tenant/10/Table/106{-/2}                  range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=3600 num_replicas=7
-/Tenant/10/Table/106/{2-3}                 range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=25 num_replicas=7 num_voters=5
-/Tenant/10/Table/106/{3-4}                 range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=3600 num_replicas=7
-/Tenant/10/Table/106/{4-5}                 range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=1 num_replicas=7
-/Tenant/10/Table/106/{5-6}                 range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=3600 num_replicas=7
-/Tenant/10/Table/106/{6-7}                 range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=1 num_replicas=7
-/Tenant/10/Table/10{6/7-7}                 range_max_bytes=100000 range_min_bytes=1000 ttl_seconds=3600 num_replicas=7
-
-
 # Create and drop an index inside the same transaction. The related
 # zone configuration should also be cleaned up.
-exec-sql
-SET CLUSTER SETTING sql.mvcc_compliant_index_creation.enabled = true;
-----
-
 exec-sql
 CREATE TABLE db.t2(i INT PRIMARY KEY, j INT);
 CREATE INDEX idx ON db.t2 (j);
@@ -171,18 +140,3 @@ DROP INDEX db.t2@idx
 translate database=db table=t2
 ----
 /Tenant/10/Table/10{7-8}                   ttl_seconds=3600 num_replicas=7
-
-exec-sql
-SET CLUSTER SETTING sql.mvcc_compliant_index_creation.enabled = false;
-----
-
-exec-sql
-CREATE TABLE db.t3(i INT PRIMARY KEY, j INT);
-CREATE INDEX idx ON db.t3 (j);
-ALTER INDEX db.t3@idx CONFIGURE ZONE USING gc.ttlseconds = 1;
-DROP INDEX db.t3@idx
-----
-
-translate database=db table=t3
-----
-/Tenant/10/Table/10{8-9}                   ttl_seconds=3600 num_replicas=7

pkg/clusterversion/cockroach_versions.go

@@ -292,6 +292,10 @@ const (
 	WaitedForDelRangeInGCJob
 	// RangefeedUseOneStreamPerNode changes rangefeed implementation to use 1 RPC stream per node.
 	RangefeedUseOneStreamPerNode
+	// NoNonMVCCAddSSTable adds a migration which waits for all
+	// schema changes to complete. After this point, no non-MVCC
+	// AddSSTable calls will be used outside of tenant streaming.
+	NoNonMVCCAddSSTable
 
 	// *************************************************
 	// Step (1): Add new versions here.
@@ -486,7 +490,10 @@ var versionsSingleton = keyedVersions{
 		Key:     RangefeedUseOneStreamPerNode,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 60},
 	},
-
+	{
+		Key:     NoNonMVCCAddSSTable,
+		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 62},
+	},
 	// *************************************************
 	// Step (2): Add new versions here.
 	// Do not add new versions to a patch release.

pkg/cmd/roachtest/tests/BUILD.bazel

@@ -56,7 +56,6 @@ go_library(
         "hotspotsplits.go",
         "import.go",
         "inconsistency.go",
-        "indexbackfiller.go",
         "indexes.go",
         "inverted_index.go",
         "jasyncsql.go",