Skip to content

Commit

Permalink
sql: fall back to raw SQL for statement diagnostics bundle
Browse files Browse the repository at this point in the history
When gathering the statement for a statement diagnostics bundle, fall
back to using the raw SQL when we cannot pretty-print the statement AST,
which happens for statements that fail with certain errors.

(This also fixes a crash in addTrace, which was not handling these
error cases. It looks like this crash was partially fixed in #56768 but
not completely.)

Fixes: #94416 #56705

Epic: None

Release note (bug fix): Fix a crash that occurs on the gateway node when
collecting a statement diagnostics bundle (e.g. `EXPLAIN ANALYZE
(DEBUG)`) on a statement that fails with certain errors. This crash has
existed in various forms since the introduction of statement bundles in
v20.1.0.
  • Loading branch information
michae2 committed Jan 3, 2023
1 parent 1d7bd69 commit 4f66dcc
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 34 deletions.
66 changes: 34 additions & 32 deletions pkg/sql/explain_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ func buildStatementBundle(
ctx context.Context,
db *kv.DB,
ie *InternalExecutor,
stmtRawSQL string,
plan *planTop,
planString string,
trace tracingpb.Recording,
Expand All @@ -134,7 +135,7 @@ func buildStatementBundle(
if plan == nil {
return diagnosticsBundle{collectionErr: errors.AssertionFailedf("execution terminated early")}
}
b := makeStmtBundleBuilder(db, ie, plan, trace, placeholders)
b := makeStmtBundleBuilder(db, ie, stmtRawSQL, plan, trace, placeholders)

b.addStatement()
b.addOptPlans(ctx)
Expand Down Expand Up @@ -187,6 +188,7 @@ type stmtBundleBuilder struct {
db *kv.DB
ie *InternalExecutor

stmt string
plan *planTop
trace tracingpb.Recording
placeholders *tree.PlaceholderInfo
Expand All @@ -197,34 +199,43 @@ type stmtBundleBuilder struct {
func makeStmtBundleBuilder(
db *kv.DB,
ie *InternalExecutor,
stmt string,
plan *planTop,
trace tracingpb.Recording,
placeholders *tree.PlaceholderInfo,
) stmtBundleBuilder {
b := stmtBundleBuilder{db: db, ie: ie, plan: plan, trace: trace, placeholders: placeholders}
b := stmtBundleBuilder{
db: db, ie: ie, stmt: stmt, plan: plan, trace: trace, placeholders: placeholders,
}
b.buildPrettyStatement()
b.z.Init()
return b
}

// addStatement adds the pretty-printed statement as file statement.txt.
func (b *stmtBundleBuilder) addStatement() {
cfg := tree.DefaultPrettyCfg()
cfg.UseTabs = false
cfg.LineWidth = 100
cfg.TabWidth = 2
cfg.Simplify = true
cfg.Align = tree.PrettyNoAlign
cfg.JSONFmt = true
var output string
// If we hit an early error, stmt or stmt.AST might not be initialized yet.
switch {
case b.plan.stmt == nil:
output = "-- No Statement."
case b.plan.stmt.AST == nil:
output = "-- No AST."
default:
output = cfg.Pretty(b.plan.stmt.AST)
// buildPrettyStatement saves the pretty-printed statement (without any
// placeholder arguments).
func (b *stmtBundleBuilder) buildPrettyStatement() {
// If we hit an early error, stmt or stmt.AST might not be initialized yet. In
// this case use the original statement SQL already in the stmtBundleBuilder.
if b.plan.stmt != nil && b.plan.stmt.AST != nil {
cfg := tree.DefaultPrettyCfg()
cfg.UseTabs = false
cfg.LineWidth = 100
cfg.TabWidth = 2
cfg.Simplify = true
cfg.Align = tree.PrettyNoAlign
cfg.JSONFmt = true
b.stmt = cfg.Pretty(b.plan.stmt.AST)
}
if b.stmt == "" {
b.stmt = "-- no statement"
}
}

// addStatement adds the pretty-printed statement in b.stmt as file
// statement.txt.
func (b *stmtBundleBuilder) addStatement() {
output := b.stmt

if b.placeholders != nil && len(b.placeholders.Values) != 0 {
var buf bytes.Buffer
Expand Down Expand Up @@ -324,25 +335,16 @@ func (b *stmtBundleBuilder) addTrace() {
b.z.AddFile("trace.json", traceJSONStr)
}

cfg := tree.DefaultPrettyCfg()
cfg.UseTabs = false
cfg.LineWidth = 100
cfg.TabWidth = 2
cfg.Simplify = true
cfg.Align = tree.PrettyNoAlign
cfg.JSONFmt = true
stmt := cfg.Pretty(b.plan.stmt.AST)

// The JSON is not very human-readable, so we include another format too.
b.z.AddFile("trace.txt", fmt.Sprintf("%s\n\n\n\n%s", stmt, b.trace.String()))
b.z.AddFile("trace.txt", fmt.Sprintf("%s\n\n\n\n%s", b.stmt, b.trace.String()))

// Note that we're going to include the non-anonymized statement in the trace.
// But then again, nothing in the trace is anonymized.
comment := fmt.Sprintf(`This is a trace for SQL statement: %s
This trace can be imported into Jaeger for visualization. From the Jaeger Search screen, select the JSON File.
Jaeger can be started using docker with: docker run -d --name jaeger -p 16686:16686 jaegertracing/all-in-one:1.17
The UI can then be accessed at http://localhost:16686/search`, stmt)
jaegerJSON, err := b.trace.ToJaegerJSON(stmt, comment, "")
The UI can then be accessed at http://localhost:16686/search`, b.stmt)
jaegerJSON, err := b.trace.ToJaegerJSON(b.stmt, comment, "")
if err != nil {
b.z.AddFile("trace-jaeger.txt", err.Error())
} else {
Expand Down
7 changes: 5 additions & 2 deletions pkg/sql/instrumentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,12 @@ func (ih *instrumentationHelper) Finish(
)
warnings = ob.GetWarnings()
bundle = buildStatementBundle(
ctx, cfg.DB, ie.(*InternalExecutor), &p.curPlan, ob.BuildString(), trace, placeholders,
ctx, cfg.DB, ie.(*InternalExecutor), stmtRawSQL, &p.curPlan, ob.BuildString(), trace,
placeholders,
)
bundle.insert(
ctx, ih.fingerprint, ast, cfg.StmtDiagnosticsRecorder, ih.diagRequestID, ih.diagRequest,
)
bundle.insert(ctx, ih.fingerprint, ast, cfg.StmtDiagnosticsRecorder, ih.diagRequestID, ih.diagRequest)
telemetry.Inc(sqltelemetry.StatementDiagnosticsCollectedCounter)
}
ih.stmtDiagnosticsRecorder.MaybeRemoveRequest(ih.diagRequestID, ih.diagRequest, execLatency)
Expand Down

0 comments on commit 4f66dcc

Please sign in to comment.