Skip to content

Commit

Permalink
upgrades: delete userfile corruption version gate
Browse files Browse the repository at this point in the history
The userfile descriptor corruption version gate can
be safely deleted as all clusters upgrading to 23.1+
are guaranteed to have been upgraded to 22.2 prior to
that.

Informs: cockroachdb#100552
Release note: None
  • Loading branch information
adityamaru committed Apr 5, 2023
1 parent 7c1df10 commit a570e41
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 181 deletions.
8 changes: 0 additions & 8 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,6 @@ const (
// TODODelete_V22_2SupportAssumeRoleAuth is the version where assume role authorization is
// supported in cloud storage and KMS.
TODODelete_V22_2SupportAssumeRoleAuth
// TODODelete_V22_2FixUserfileRelatedDescriptorCorruption adds a migration which uses
// heuristics to identify invalid table descriptors for userfile-related
// descriptors.
TODODelete_V22_2FixUserfileRelatedDescriptorCorruption

// V22_2 is CockroachDB v22.2. It's used for all v22.2.x patch releases.
V22_2
Expand Down Expand Up @@ -708,10 +704,6 @@ var rawVersionsSingleton = keyedVersions{
Key: TODODelete_V22_2SupportAssumeRoleAuth,
Version: roachpb.Version{Major: 22, Minor: 1, Internal: 74},
},
{
Key: TODODelete_V22_2FixUserfileRelatedDescriptorCorruption,
Version: roachpb.Version{Major: 22, Minor: 1, Internal: 76},
},
{
Key: V22_2,
Version: roachpb.Version{Major: 22, Minor: 2, Internal: 0},
Expand Down
1 change: 0 additions & 1 deletion pkg/upgrade/upgrades/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ go_test(
"descriptor_utils_test.go",
"ensure_sql_schema_telemetry_schedule_test.go",
"external_connections_table_user_id_migration_test.go",
"fix_userfile_descriptor_corruption_test.go",
"helpers_test.go",
"key_visualizer_migration_test.go",
"main_test.go",
Expand Down
57 changes: 0 additions & 57 deletions pkg/upgrade/upgrades/fix_userfile_descriptor_corruption.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,70 +14,13 @@ import (
"context"
"strings"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"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/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,
) error {
return d.DB.DescsTxn(ctx, func(
ctx context.Context, txn descs.Txn,
) error {
query := `SELECT * FROM crdb_internal.invalid_objects`
rows, err := txn.QueryIterator(ctx, "find-invalid-descriptors", txn.KV(), 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.KV(), txn.Descriptors(), tableID, errString) {
log.Infof(ctx, "attempting to fix invalid table descriptor %d assuming it is a userfile-related table", tableID)
mutTableDesc, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, tableID)
if err != nil {
return err
}
mutTableDesc.Mutations = nil
mutTableDesc.MutationJobs = nil
if err := txn.Descriptors().WriteDesc(ctx, false, mutTableDesc, txn.KV()); 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.
Expand Down
110 changes: 0 additions & 110 deletions pkg/upgrade/upgrades/fix_userfile_descriptor_corruption_test.go

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,38 @@ import (
"github.com/stretchr/testify/require"
)

// The hex for the descriptor to inject was created by running the following
// commands in a 22.1 binary.
//
// In 22.1 and prior, userfile creation had a bug that produced a foreign
// key constraint mutation without an associated mutation job.
//
// CREATE DATABASE to_backup;
// BACKUP DATABASE to_backup INTO 'userfile://defaultdb.test/data';
//
// SELECT encode(descriptor, 'hex')
// FROM system.descriptor
// WHERE id = (
// SELECT id
// FROM system.namespace
// WHERE name = 'test_upload_payload'
// );
// SELECT encode(descriptor, 'hex')
// FROM system.descriptor
// WHERE id = (
// SELECT id
// FROM system.namespace
// WHERE name = 'test_upload_files'
// );
//
// The files table is not broken, but should probably be inserted for completeness.
//
// These are shared with the tests for the upgrade preconditions.
const (
brokenUserfilePayloadTable = "0ac4040a13746573745f75706c6f61645f7061796c6f6164186b206428033a00422d0a0766696c655f696410011a0d080e10001800300050861760002000300068007000780080010088010098010042300a0b627974655f6f666673657410021a0c08011040180030005014600020003000680070007800800100880100980100422c0a077061796c6f616410031a0c0808100018003000501160002001300068007000780080010088010098010048045290010a18746573745f75706c6f61645f7061796c6f61645f706b657910011801220766696c655f6964220b627974655f6f66667365742a077061796c6f616430013002400040004a10080010001a00200028003000380040005a0070037a0408002000800100880100900104980101a20106080012001800a80100b20100ba0100c00100c80188a5978797a6b68c17d0010160026a210a0b0a0561646d696e100218020a0a0a04726f6f74100218021204726f6f74180272541801200128013800424a0801120a66696c655f69645f666b1a0c0a0012001800300038004003221e086b10011802206a2a0a66696c655f69645f666b3002380040004800700330003a0a08001a0020002a003003800102880103980100b201320a077072696d61727910001a0766696c655f69641a0b627974655f6f66667365741a077061796c6f61642001200220032803b80101c20100da010c080110818088ef93a5db8d0be80100f2010408001200f801008002009202009a020a0888a5978797a6b68c17b20200b80200c00265c80200e00200800300880304"
brokenUserfileFilesTable = "0a91060a11746573745f75706c6f61645f66696c6573186a206428033a00422d0a0866696c656e616d6510011a0c0807100018003000501960002000300068007000780080010088010098010042400a0766696c655f696410021a0d080e100018003000508617600020002a1167656e5f72616e646f6d5f7575696428293000680070007800800100880100980100422e0a0966696c655f73697a6510031a0c08011040180030005014600020003000680070007800800100880100980100422d0a08757365726e616d6510041a0c0807100018003000501960002000300068007000780080010088010098010042440a0b75706c6f61645f74696d6510051a0d080510001800300050da08600020012a116e6f7728293a3a3a54494d455354414d503000680070007800800100880100980100480652a6010a16746573745f75706c6f61645f66696c65735f706b657910011801220866696c656e616d652a0766696c655f69642a0966696c655f73697a652a08757365726e616d652a0b75706c6f61645f74696d65300140004a10080010001a00200028003000380040005a0070027003700470057a0408002000800100880100900104980101a20106080012001800a80100b20100ba0100c00100c80188a5978797a6b68c17d001025a7b0a1d746573745f75706c6f61645f66696c65735f66696c655f69645f6b657910021801220766696c655f69643002380140004a10080010001a00200028003000380040005a007a0408002000800100880100900103980100a20106080012001800a80100b20100ba0100c00100c80188a5978797a6b68c17d0010160036a210a0b0a0561646d696e100218020a0a0a04726f6f74100218021204726f6f741802800101880103980100b2014c0a077072696d61727910001a0866696c656e616d651a0766696c655f69641a0966696c655f73697a651a08757365726e616d651a0b75706c6f61645f74696d65200120022003200420052800b80101c20100e80100f2010408001200f801008002009202009a020a0888a5978797a6b68c17b20200b80200c00265c80200e00200800300880303"
)

// TestPreconditionBeforeStartingAnUpgrade tests that all defined preconditions
// must be met before starting an upgrade.
func TestPreconditionBeforeStartingAnUpgrade(t *testing.T) {
Expand Down
5 changes: 0 additions & 5 deletions pkg/upgrade/upgrades/upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,6 @@ var upgrades = []upgradebase.Upgrade{
ensureSQLSchemaTelemetrySchedule,
"add default SQL schema telemetry schedule",
),
upgrade.NewTenantUpgrade("fix corrupt user-file related table descriptors",
toCV(clusterversion.TODODelete_V22_2FixUserfileRelatedDescriptorCorruption),
upgrade.NoPrecondition,
fixInvalidObjectsThatLookLikeBadUserfileConstraint,
),
upgrade.NewTenantUpgrade("add columns to system.tenants and populate a system tenant entry",
toCV(clusterversion.V23_1TenantNamesStateAndServiceMode),
upgrade.NoPrecondition,
Expand Down

0 comments on commit a570e41

Please sign in to comment.