Skip to content

Commit

Permalink
sql: stop writing trace to statement_diagnostics
Browse files Browse the repository at this point in the history
The trace JSON stored in the statement_diagnostics table can be
arbitrarily large which causes problems for very large queries. This
trace is not currently used, and is contained in the bundle itself.
We stop writing it to the table in this commit. We can remove the
column altogether in a separate change (it will require a migration).

Fixes #61874.

Release note (bug fix): Fixed "command is too large" errors in some
cases when using EXPLAIN ANALYZE (DEBUG) or statement diagnostics on
complex queries.
  • Loading branch information
RaduBerinde committed Mar 12, 2021
1 parent cd96b86 commit 2d05a54
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 293 deletions.
1 change: 0 additions & 1 deletion docs/generated/http/full.md
Original file line number Diff line number Diff line change
Expand Up @@ -2710,7 +2710,6 @@ Support status: [reserved](#support-status)
| id | [int64](#cockroach.server.serverpb.StatementDiagnosticsResponse-int64) | | | [reserved](#support-status) |
| statement_fingerprint | [string](#cockroach.server.serverpb.StatementDiagnosticsResponse-string) | | | [reserved](#support-status) |
| collected_at | [google.protobuf.Timestamp](#cockroach.server.serverpb.StatementDiagnosticsResponse-google.protobuf.Timestamp) | | | [reserved](#support-status) |
| trace | [string](#cockroach.server.serverpb.StatementDiagnosticsResponse-string) | | | [reserved](#support-status) |



Expand Down
423 changes: 191 additions & 232 deletions pkg/server/serverpb/status.pb.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/server/serverpb/status.proto
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,7 @@ message StatementDiagnostics {
string statement_fingerprint = 2;
google.protobuf.Timestamp collected_at = 3
[ (gogoproto.nullable) = false, (gogoproto.stdtime) = true ];
string trace = 4;
reserved 4;
}

message StatementDiagnosticsRequest {
Expand Down
20 changes: 1 addition & 19 deletions pkg/server/statement_diagnostics_requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
package server

import (
"bytes"
"context"
"encoding/json"
"time"

"github.com/cockroachdb/cockroach/pkg/security"
Expand All @@ -34,7 +32,6 @@ type stmtDiagnosticsRequest struct {
type stmtDiagnostics struct {
ID int
StatementFingerprint string
Trace string
CollectedAt time.Time
}

Expand All @@ -54,7 +51,6 @@ func (diagnostics *stmtDiagnostics) toProto() serverpb.StatementDiagnostics {
Id: int64(diagnostics.ID),
StatementFingerprint: diagnostics.StatementFingerprint,
CollectedAt: diagnostics.CollectedAt,
Trace: diagnostics.Trace,
}
return resp
}
Expand Down Expand Up @@ -179,8 +175,7 @@ func (s *statusServer) StatementDiagnostics(
`SELECT
id,
statement_fingerprint,
collected_at,
trace
collected_at
FROM
system.statement_diagnostics
WHERE
Expand All @@ -207,19 +202,6 @@ func (s *statusServer) StatementDiagnostics(
diagnostics.CollectedAt = collectedAt.Time
}

if traceJSON, ok := row[3].(*tree.DJSON); ok {
traceJSONString, err := traceJSON.AsText()
if err != nil {
return nil, err
}
var prettyJSON bytes.Buffer
if err := json.Indent(
&prettyJSON, []byte(*traceJSONString), "" /* prefix */, "\t" /* indent */); err != nil {
return nil, errors.Wrap(err, "failed to parse JSON")
}
diagnostics.Trace = prettyJSON.String()
}

diagnosticsProto := diagnostics.toProto()
response := &serverpb.StatementDiagnosticsResponse{
Diagnostics: &diagnosticsProto,
Expand Down
7 changes: 0 additions & 7 deletions pkg/server/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2070,13 +2070,6 @@ func TestStatementDiagnosticsCompleted(t *testing.T) {
if err := getStatusJSONProto(s, diagPath, &diagRespGet); err != nil {
t.Fatal(err)
}

json := diagRespGet.Diagnostics.Trace
if json == "" ||
!strings.Contains(json, "traced statement") ||
!strings.Contains(json, "statement execution committed the txn") {
t.Fatal("statement diagnostics did not capture a trace")
}
}

func TestJobStatusResponse(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/catalog/systemschema/system.go
Original file line number Diff line number Diff line change
Expand Up @@ -1593,6 +1593,7 @@ var (
{Name: "statement_fingerprint", ID: 2, Type: types.String, Nullable: false},
{Name: "statement", ID: 3, Type: types.String, Nullable: false},
{Name: "collected_at", ID: 4, Type: types.TimestampTZ, Nullable: false},
// TODO(radu): remove this column; it is no longer used.
{Name: "trace", ID: 5, Type: types.Jsonb, Nullable: true},
{Name: "bundle_chunks", ID: 6, Type: types.IntArray, Nullable: true},
{Name: "error", ID: 7, Type: types.String, Nullable: true},
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/stmtdiagnostics/statement_diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,9 @@ func (r *Registry) InsertStatementDiagnostics(
ctx, "stmt-diag-insert", txn,
sessiondata.InternalExecutorOverride{User: security.RootUserName()},
"INSERT INTO system.statement_diagnostics "+
"(statement_fingerprint, statement, collected_at, trace, bundle_chunks, error) "+
"VALUES ($1, $2, $3, $4, $5, $6) RETURNING id",
stmtFingerprint, stmt, collectionTime, traceJSON, bundleChunksVal, errorVal,
"(statement_fingerprint, statement, collected_at, bundle_chunks, error) "+
"VALUES ($1, $2, $3, $4, $5) RETURNING id",
stmtFingerprint, stmt, collectionTime, bundleChunksVal, errorVal,
)
if err != nil {
return err
Expand Down
14 changes: 0 additions & 14 deletions pkg/sql/stmtdiagnostics/statement_diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,6 @@ func TestDiagnosticsRequest(t *testing.T) {

checkCompleted(reqID)

// Check the trace.
row := db.QueryRow("SELECT jsonb_pretty(trace) FROM system.statement_diagnostics WHERE ID = $1", traceID.Int64)
var json string
require.NoError(t, row.Scan(&json))
require.Contains(t, json, "traced statement")
require.Contains(t, json, "statement execution committed the txn")

// Verify that we can handle multiple requests at the same time.
id1, err := registry.InsertRequestInternal(ctx, "INSERT INTO test VALUES (_)")
require.NoError(t, err)
Expand Down Expand Up @@ -139,13 +132,6 @@ func TestDiagnosticsRequestDifferentNode(t *testing.T) {
return nil
})
require.True(t, traceID.Valid)

// Check the trace.
row := db0.QueryRow(`SELECT jsonb_pretty(trace) FROM system.statement_diagnostics
WHERE ID = $1`, traceID.Int64)
var json string
require.NoError(t, row.Scan(&json))
require.Contains(t, json, "traced statement")
}
runUntilTraced("INSERT INTO test VALUES (1)", reqID)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@ import { Helmet } from "react-helmet";
import { connect } from "react-redux";
import moment from "moment";
import { Action, Dispatch } from "redux";
import Long from "long";
import { Link } from "react-router-dom";
import { isUndefined } from "lodash";

import { Anchor, Button, Text, TextTypes, Tooltip } from "src/components";
import HeaderSection from "src/views/shared/components/headerSection";
import { AdminUIState } from "src/redux/state";
import { getStatementDiagnostics } from "src/util/api";
import { trustIcon } from "src/util/trust";
import DownloadIcon from "!!raw-loader!assets/download.svg";
import {
Expand All @@ -36,7 +34,6 @@ import { DiagnosticStatusBadge } from "src/views/statements/diagnostics/diagnost
import "./statementDiagnosticsHistoryView.styl";
import { cockroach } from "src/js/protos";
import IStatementDiagnosticsReport = cockroach.server.serverpb.IStatementDiagnosticsReport;
import StatementDiagnosticsRequest = cockroach.server.serverpb.StatementDiagnosticsRequest;
import {
SortedTable,
ColumnDescriptor,
Expand Down Expand Up @@ -205,19 +202,6 @@ class StatementDiagnosticsHistoryView extends React.Component<
);
};

getStatementDiagnostics = async (diagnosticsId: Long) => {
const request = new StatementDiagnosticsRequest({
statement_diagnostics_id: diagnosticsId,
});
const response = await getStatementDiagnostics(request);
const trace = response.diagnostics?.trace;
this.downloadRef.current?.download(
"statement-diagnostics.json",
"application/json",
trace,
);
};

changeSortSetting = (ss: SortSetting) => {
this.setState({
sortSetting: ss,
Expand Down

0 comments on commit 2d05a54

Please sign in to comment.