upgrades: skip known userfile-related corruption in desc validation

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

@@ -300,4 +300,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	1000022.1-74	set the active cluster version in the format '<major>.<minor>'
+version	version	1000022.1-76	set the active cluster version in the format '<major>.<minor>'

docs/generated/settings/settings.html

@@ -234,6 +234,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>1000022.1-74</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>1000022.1-76</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
 </tbody>
 </table>

pkg/clusterversion/cockroach_versions.go

@@ -302,6 +302,10 @@ const (
 	// supported in cloud storage and KMS.
 	SupportAssumeRoleAuth
 
+	// FixUserfileRelatedDescriptorCorruption adds a migration which uses
+	// heuristics to identify invalid table descriptors for userfile-related
+	// descriptors.
+	FixUserfileRelatedDescriptorCorruption
 	// *************************************************
 	// Step (1): Add new versions here.
 	// Do not add new versions to a patch release.
@@ -488,7 +492,10 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     SupportAssumeRoleAuth,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 74},
 	},
-
+	{
+		Key:     FixUserfileRelatedDescriptorCorruption,
+		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 76},
+	},
 	// *************************************************
 	// Step (2): Add new versions here.
 	// Do not add new versions to a patch release.

pkg/server/server_sql.go

@@ -1023,7 +1023,7 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
 
 		knobs, _ := cfg.TestingKnobs.UpgradeManager.(*upgrade.TestingKnobs)
 		migrationMgr := upgrademanager.NewManager(
-			systemDeps, leaseMgr, cfg.circularInternalExecutor, jobRegistry, codec,
+			systemDeps, leaseMgr, cfg.circularInternalExecutor, cfg.internalExecutorFactory, jobRegistry, codec,
 			cfg.Settings, knobs,
 		)
 		execCfg.UpgradeJobDeps = migrationMgr

pkg/upgrade/upgrademanager/BUILD.bazel

@@ -16,6 +16,7 @@ go_library(
         "//pkg/server/serverpb",
         "//pkg/settings/cluster",
         "//pkg/sql",
+        "//pkg/sql/catalog/descs",
         "//pkg/sql/catalog/lease",
         "//pkg/sql/protoreflect",
         "//pkg/sql/sem/tree",

pkg/upgrade/upgrademanager/manager.go

@@ -25,6 +25,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/lease"
 	"github.com/cockroachdb/cockroach/pkg/sql/protoreflect"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -44,6 +45,7 @@ type Manager struct {
 	deps     upgrade.SystemDeps
 	lm       *lease.Manager
 	ie       sqlutil.InternalExecutor
+	ief      descs.TxnManager
 	jr       *jobs.Registry
 	codec    keys.SQLCodec
 	settings *cluster.Settings
@@ -72,6 +74,7 @@ func NewManager(
 	deps upgrade.SystemDeps,
 	lm *lease.Manager,
 	ie sqlutil.InternalExecutor,
+	ief descs.TxnManager,
 	jr *jobs.Registry,
 	codec keys.SQLCodec,
 	settings *cluster.Settings,
@@ -85,6 +88,7 @@ func NewManager(
 		deps:     deps,
 		lm:       lm,
 		ie:       ie,
+		ief:      ief,
 		jr:       jr,
 		codec:    codec,
 		settings: settings,
@@ -411,12 +415,13 @@ func (m *Manager) checkPreconditions(
 			continue
 		}
 		if err := tm.Precondition(ctx, v, upgrade.TenantDeps{
-			DB:               m.deps.DB,
-			Codec:            m.codec,
-			Settings:         m.settings,
-			LeaseManager:     m.lm,
-			InternalExecutor: m.ie,
-			JobRegistry:      m.jr,
+			DB:                      m.deps.DB,
+			Codec:                   m.codec,
+			Settings:                m.settings,
+			LeaseManager:            m.lm,
+			InternalExecutor:        m.ie,
+			InternalExecutorFactory: m.ief,
+			JobRegistry:             m.jr,
 		}); err != nil {
 			return errors.Wrapf(
 				err,

pkg/upgrade/upgrades/BUILD.bazel

@@ -8,6 +8,7 @@ go_library(
         "alter_statement_statistics_index_recommendations.go",
         "descriptor_utils.go",
         "ensure_sql_schema_telemetry_schedule.go",
+        "fix_userfile_descriptor_corruption.go",
         "precondition_before_starting_an_upgrade.go",
         "remove_grant_migration.go",
         "role_id_sequence_migration.go",
@@ -50,6 +51,7 @@ go_library(
         "//pkg/sql/sem/tree",
         "//pkg/sql/sessiondata",
         "//pkg/sql/sqlutil",
+        "//pkg/sql/types",
         "//pkg/storage",
         "//pkg/upgrade",
         "//pkg/util/hlc",
@@ -71,6 +73,7 @@ go_test(
         "builtins_test.go",
         "descriptor_utils_test.go",
         "ensure_sql_schema_telemetry_schedule_test.go",
+        "fix_userfile_descriptor_corruption_test.go",
         "helpers_test.go",
         "main_test.go",
         "precondition_before_starting_an_upgrade_external_test.go",

pkg/upgrade/upgrades/fix_userfile_descriptor_corruption.go

@@ -0,0 +1,162 @@
+// Copyright 2022 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package upgrades
+
+import (
+	"context"
+	"strings"
+
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/jobs"
+	"github.com/cockroachdb/cockroach/pkg/kv"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqlutil"
+	"github.com/cockroachdb/cockroach/pkg/sql/types"
+	"github.com/cockroachdb/cockroach/pkg/upgrade"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+)
+
+// fixInvalidObjectsThatLookLikeBadUserfileConstraint attempts to remove a
+// dangling table descriptor mutation for foreign key constraints on
+// usefile-related upload_payload tables.
+//
+// It only proceeds with the fix if
+//
+//   - the fk mutation it the _only_ mutation on the table,
+//   - the table name, column names, and column type all look like a userfile table, and
+//   - the fk is referencing a table whose name, column names, and column types all look like a userfile table.
+//
+// To fix the table, we remove the mutation as the FK constraint is unnecessary
+// in the current implementation.
+func fixInvalidObjectsThatLookLikeBadUserfileConstraint(
+	ctx context.Context, _ clusterversion.ClusterVersion, d upgrade.TenantDeps, _ *jobs.Job,
+) error {
+	return d.InternalExecutorFactory.DescsTxnWithExecutor(ctx, d.DB, nil,
+		func(ctx context.Context, txn *kv.Txn, descriptors *descs.Collection, ie sqlutil.InternalExecutor) error {
+			query := `SELECT * FROM crdb_internal.invalid_objects`
+			rows, err := ie.QueryIterator(ctx, "find-invalid-descriptors", txn, query)
+			if err != nil {
+				return err
+			}
+			defer func() { _ = rows.Close() }()
+
+			var hasNext bool
+			for hasNext, err = rows.Next(ctx); hasNext && err == nil; hasNext, err = rows.Next(ctx) {
+				// crdb_internal.invalid_objects has five columns: id, database name, schema name, table name, error.
+				row := rows.Cur()
+				tableID := descpb.ID(tree.MustBeDInt(row[0]))
+				errString := string(tree.MustBeDString(row[4]))
+				if veryLikelyKnownUserfileBreakage(ctx, txn, descriptors, tableID, errString) {
+					log.Infof(ctx, "attempting to fix invalid table descriptor %d assuming it is a userfile-related table", tableID)
+					mutTableDesc, err := descriptors.GetMutableTableByID(ctx, txn, tableID, tree.ObjectLookupFlags{
+						CommonLookupFlags: tree.CommonLookupFlags{
+							IncludeOffline: true,
+							IncludeDropped: true,
+						},
+					})
+					if err != nil {
+						return err
+					}
+					mutTableDesc.Mutations = nil
+					mutTableDesc.MutationJobs = nil
+					if err := descriptors.WriteDesc(ctx, false, mutTableDesc, txn); err != nil {
+						return err
+					}
+				}
+			}
+			if err != nil {
+				// TODO(ssd): We always return a nil error here because I'm not sure that this
+				// would be worth failing an upgrade for.
+				log.Warningf(ctx, "could not fix broken userfile: %v", err)
+			}
+			return nil
+		})
+}
+
+// veryLikelyKnownUserfileBreakage returns true if the given descriptor id and
+// error message from crdb_internal.invalid_objects is likely related to a known
+// userfile-related table corruption.
+func veryLikelyKnownUserfileBreakage(
+	ctx context.Context, txn *kv.Txn, descriptors *descs.Collection, id descpb.ID, errMsg string,
+) bool {
+	if !strings.HasSuffix(errMsg, "job not found") {
+		return false
+	}
+
+	tableDesc, err := descriptors.GetImmutableTableByID(ctx, txn, id, tree.ObjectLookupFlags{
+		CommonLookupFlags: tree.CommonLookupFlags{
+			IncludeOffline: true,
+			IncludeDropped: true,
+		},
+	})
+	if err != nil {
+		return false
+	}
+	return tableLooksLikeUserfilePayloadTable(tableDesc) && mutationsLookLikeuserfilePayloadCorruption(ctx, txn, tableDesc, descriptors)
+}
+
+func tableLooksLikeUserfilePayloadTable(tableDesc catalog.TableDescriptor) bool {
+	columns := tableDesc.PublicColumns()
+	return strings.HasSuffix(tableDesc.GetName(), "_upload_payload") &&
+		len(columns) == 3 &&
+		(columns[0].ColName() == "file_id" && columns[0].GetType().Oid() == types.Uuid.Oid()) &&
+		(columns[1].ColName() == "byte_offset" && columns[1].GetType().Oid() == types.Int.Oid()) &&
+		(columns[2].ColName() == "payload" && columns[2].GetType().Oid() == types.Bytes.Oid())
+}
+
+func tableLooksLikeUserfileFileTable(tableDesc catalog.TableDescriptor) bool {
+	columns := tableDesc.PublicColumns()
+	return strings.HasSuffix(tableDesc.GetName(), "_upload_files") &&
+		len(columns) == 5 &&
+		(columns[0].ColName() == "filename" && columns[0].GetType().Oid() == types.String.Oid()) &&
+		(columns[1].ColName() == "file_id" && columns[1].GetType().Oid() == types.Uuid.Oid()) &&
+		(columns[2].ColName() == "file_size" && columns[2].GetType().Oid() == types.Int.Oid()) &&
+		(columns[3].ColName() == "username" && columns[3].GetType().Oid() == types.String.Oid()) &&
+		(columns[4].ColName() == "upload_time" && columns[4].GetType().Oid() == types.Timestamp.Oid())
+}
+
+func mutationsLookLikeuserfilePayloadCorruption(
+	ctx context.Context,
+	txn *kv.Txn,
+	tableDesc catalog.TableDescriptor,
+	descriptors *descs.Collection,
+) bool {
+	if len(tableDesc.GetMutationJobs()) != 1 {
+		return false
+	}
+	if len(tableDesc.AllMutations()) != 1 {
+		return false
+	}
+	mutation := tableDesc.AllMutations()[0]
+	if mutation.Adding() && mutation.DeleteOnly() {
+		if constraintMutation := mutation.AsConstraint(); constraintMutation != nil {
+			if constraintMutation.IsForeignKey() {
+				fkConstraint := constraintMutation.ForeignKey()
+				targetTableDesc, err := descriptors.GetImmutableTableByID(ctx, txn, fkConstraint.ReferencedTableID, tree.ObjectLookupFlags{
+					CommonLookupFlags: tree.CommonLookupFlags{
+						IncludeOffline: true,
+						IncludeDropped: true,
+					},
+				})
+				if err != nil {
+					return false
+				}
+				if tableLooksLikeUserfileFileTable(targetTableDesc) {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}