Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

release-22.2: more details about desc validity errors during upgrades #104048

Merged
merged 1 commit into from Jun 1, 2023
Merged

release-22.2: more details about desc validity errors during upgrades #104048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion pkg/sql/crdb_internal.go
View file
Open in desktop
Expand Up @@ -84,6 +84,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/tracing/collector"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/redact"
)

// CrdbInternalName is the name of the crdb_internal schema.
Expand Down Expand Up @@ -5011,7 +5012,8 @@ CREATE TABLE crdb_internal.invalid_objects (
database_name STRING,
schema_name STRING,
obj_name STRING,
error STRING
error STRING,
error_redactable STRING NOT VISIBLE
)`,
populate: func(
ctx context.Context, p *planner, dbContext catalog.DatabaseDescriptor, addRow func(...tree.Datum) error,
Expand Down Expand Up @@ -5061,6 +5063,7 @@ CREATE TABLE crdb_internal.invalid_objects (
tree.NewDString(scName),
tree.NewDString(objName),
tree.NewDString(validationError.Error()),
tree.NewDString(string(redact.Sprint(validationError))),
)
}

Expand Down
22 changes: 12 additions & 10 deletions pkg/upgrade/upgrades/precondition_before_starting_an_upgrade.go
View file
Open in desktop
Expand Up @@ -12,8 +12,6 @@ package upgrades

import (
"context"
"fmt"
"strings"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/kv"
Expand All @@ -25,6 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sqlutil"
"github.com/cockroachdb/cockroach/pkg/upgrade"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/redact"
)

func preconditionBeforeStartingAnUpgrade(
Expand All @@ -48,10 +47,10 @@ func preconditionBeforeStartingAnUpgrade(
func preconditionNoInvalidDescriptorsBeforeUpgrading(
ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps,
) error {
var errMsg strings.Builder
var errMsg redact.StringBuilder
err := d.InternalExecutorFactory.DescsTxnWithExecutor(ctx, d.DB, d.SessionData,
func(ctx context.Context, txn *kv.Txn, descriptors *descs.Collection, ie sqlutil.InternalExecutor) error {
query := `SELECT * FROM crdb_internal.invalid_objects`
query := `SELECT id, database_name, schema_name, obj_name, error_redactable FROM crdb_internal.invalid_objects`
rows, err := ie.QueryIterator(
ctx, "check-if-there-are-any-invalid-descriptors", txn /* txn */, query)
if err != nil {
Expand All @@ -68,13 +67,16 @@ func preconditionNoInvalidDescriptorsBeforeUpgrading(
tree.Name(tree.MustBeDString(row[3])),
)
tableID := descpb.ID(tree.MustBeDInt(row[0]))
errString := string(tree.MustBeDString(row[4]))
// Note: the cast from the `error` column to RedactableString here is only valid because
// the `error` column is populated via a RedactableString too. Conversion from string
// to RedactableString is not safe in general.
errString := redact.RedactableString(tree.MustBeDString(row[4]))
// TODO(ssd): Remove in 23.1 once we are sure that the migration which fixes this corruption has run.
if veryLikelyKnownUserfileBreakage(ctx, txn, descriptors, tableID, errString) {
log.Infof(ctx, "ignoring invalid descriptor %v (%v) with error %q because it looks like known userfile-related corruption",
descName.String(), tableID, errString)
if veryLikelyKnownUserfileBreakage(ctx, txn, descriptors, tableID, errString.StripMarkers()) {
log.Infof(ctx, "ignoring invalid descriptor %v (%v) with error because it looks like known userfile-related corruption: %v",
&descName, tableID, errString)
} else {
errMsg.WriteString(fmt.Sprintf("invalid descriptor: %v (%v) because %v\n", descName.String(), row[0], row[4]))
errMsg.Printf("invalid descriptor: %v (%d) because %v\n", &descName, tableID, errString)
}
}
return err
Expand All @@ -87,5 +89,5 @@ func preconditionNoInvalidDescriptorsBeforeUpgrading(
}
return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
"there exists invalid descriptors as listed below; fix these descriptors "+
"before attempting to upgrade again:\n%v", errMsg.String())
"before attempting to upgrade again:\n%v", errMsg)
}
4 changes: 2 additions & 2 deletions pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go
View file
Open in desktop
Expand Up @@ -119,8 +119,8 @@ func TestPreconditionBeforeStartingAnUpgrade(t *testing.T) {
require.Error(t, err, "upgrade should be refused because precondition is violated.")
require.Equal(t, "pq: verifying precondition for version 22.1-2: "+
"there exists invalid descriptors as listed below; fix these descriptors before attempting to upgrade again:\n"+
"invalid descriptor: defaultdb.public.t (104) because 'relation \"t\" (104): invalid depended-on-by relation back reference: referenced descriptor ID 53: referenced descriptor not found'\n"+
"invalid descriptor: defaultdb.public.temp_tbl (104) because 'no matching name info found in non-dropped relation \"t\"'",
"invalid descriptor: defaultdb.public.t (104) because relation \"t\" (104): invalid depended-on-by relation back reference: referenced descriptor ID 53: referenced descriptor not found\n"+
"invalid descriptor: defaultdb.public.temp_tbl (104) because no matching name info found in non-dropped relation \"t\"",
strings.ReplaceAll(err.Error(), "1000022", "22"))
// The cluster version should remain at `v0`.
ts.tdb.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{v0.String()}})
Expand Down