From fa488360f18d9d6ad612d0c5c9c51e4fa3e96c19 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Thu, 12 Jan 2023 11:11:59 -0500 Subject: [PATCH] sql/schemachanger: downgrade log warning when statement is not supported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this change, we'd log a warning with a scary file path when building any opaque statement which is not supported by the declarative schema changer: ``` I230112 15:56:38.184048 1797 sql/schemachanger/scbuild/build.go:39 ⋮ [n1,client=127.0.0.1:50565,user=root] 86 building declarative schema change targets for EXPERIMENTAL CHANGEFEED W230112 15:56:38.184280 1797 runtime/panic.go:884 ⋮ [n1,client=127.0.0.1:50565,user=root] 87 failed building declarative schema change targets for EXPERIMENTAL CHANGEFEED with error: ‹*tree.CreateChangefeed not implemented in the new schema changer› ``` Epic: None Relates to #95053 Release note: None --- pkg/sql/schemachanger/scerrors/errors.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/sql/schemachanger/scerrors/errors.go b/pkg/sql/schemachanger/scerrors/errors.go index a3c183cd160a..db4c49a251e8 100644 --- a/pkg/sql/schemachanger/scerrors/errors.go +++ b/pkg/sql/schemachanger/scerrors/errors.go @@ -66,15 +66,21 @@ func (el EventLogger) HandlePanicAndLogError(ctx context.Context, err *error) { if errors.Is(*err, context.Canceled) { return } - if *err == nil { + // We use a depth of 2 because this function is generally called with defer; + // using a depth of 1 would show that the caller was runtime/panic.go. + const depth = 2 + switch { + case *err == nil: if log.ExpensiveLogEnabled(ctx, 2) { - log.InfofDepth(ctx, 1, "done %s in %s", el.msg, redact.Safe(timeutil.Since(el.start))) + log.InfofDepth(ctx, depth, "done %s in %s", el.msg, redact.Safe(timeutil.Since(el.start))) } - return - } - log.WarningfDepth(ctx, 1, "failed %s with error: %v", el.msg, *err) - if errors.HasAssertionFailure(*err) { + case HasNotImplemented(*err): + log.InfofDepth(ctx, depth, "failed %s with error: %v", el.msg, *err) + case errors.HasAssertionFailure(*err): *err = errors.Wrapf(*err, "%s", el.msg) + fallthrough + default: + log.WarningfDepth(ctx, depth, "failed %s with error: %v", el.msg, *err) } }