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.

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

release-22.1: sql: turn off plan sampling by default #88420

Merged
merged 1 commit into from
Sep 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ sql.metrics.max_mem_stmt_fingerprints integer 100000 the maximum number of state
sql.metrics.max_mem_txn_fingerprints integer 100000 the maximum number of transaction fingerprints stored in memory
sql.metrics.statement_details.dump_to_logs boolean false dump collected statement statistics to node logs when periodically cleared
sql.metrics.statement_details.enabled boolean true collect per-statement query statistics
sql.metrics.statement_details.plan_collection.enabled boolean true periodically save a logical plan for each fingerprint
sql.metrics.statement_details.plan_collection.enabled boolean false periodically save a logical plan for each fingerprint
sql.metrics.statement_details.plan_collection.period duration 5m0s the time until a new logical plan is collected
sql.metrics.statement_details.threshold duration 0s minimum execution time to cause statement statistics to be collected. If configured, no transaction stats are collected.
sql.metrics.transaction_details.enabled boolean true collect per-application transaction statistics
Expand Down
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
<tr><td><code>sql.metrics.max_mem_txn_fingerprints</code></td><td>integer</td><td><code>100000</code></td><td>the maximum number of transaction fingerprints stored in memory</td></tr>
<tr><td><code>sql.metrics.statement_details.dump_to_logs</code></td><td>boolean</td><td><code>false</code></td><td>dump collected statement statistics to node logs when periodically cleared</td></tr>
<tr><td><code>sql.metrics.statement_details.enabled</code></td><td>boolean</td><td><code>true</code></td><td>collect per-statement query statistics</td></tr>
<tr><td><code>sql.metrics.statement_details.plan_collection.enabled</code></td><td>boolean</td><td><code>true</code></td><td>periodically save a logical plan for each fingerprint</td></tr>
<tr><td><code>sql.metrics.statement_details.plan_collection.enabled</code></td><td>boolean</td><td><code>false</code></td><td>periodically save a logical plan for each fingerprint</td></tr>
<tr><td><code>sql.metrics.statement_details.plan_collection.period</code></td><td>duration</td><td><code>5m0s</code></td><td>the time until a new logical plan is collected</td></tr>
<tr><td><code>sql.metrics.statement_details.threshold</code></td><td>duration</td><td><code>0s</code></td><td>minimum execution time to cause statement statistics to be collected. If configured, no transaction stats are collected.</td></tr>
<tr><td><code>sql.metrics.transaction_details.enabled</code></td><td>boolean</td><td><code>true</code></td><td>collect per-application transaction statistics</td></tr>
Expand Down
4 changes: 4 additions & 0 deletions pkg/ccl/telemetryccl/telemetry_logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ func TestTelemetryLogRegions(t *testing.T) {
sqlDB.Exec(t, `ALTER TABLE three_regions SPLIT AT SELECT generate_series(1, 3)`)
sqlDB.Exec(t, "ALTER TABLE three_regions EXPERIMENTAL_RELOCATE VALUES (ARRAY[1], 1), (ARRAY[2], 2), (ARRAY[3], 3)")

// Enable the sampling of all statements so that execution statistics
// (including the regions information) is collected.
sqlDB.Exec(t, `SET CLUSTER SETTING sql.txn_stats.sample_rate = 1.0`)

// Enable the telemetry logging and increase the sampling frequency so that
// all statements are captured.
sqlDB.Exec(t, `SET CLUSTER SETTING sql.telemetry.query_sampling.enabled = true;`)
Expand Down
9 changes: 0 additions & 9 deletions pkg/server/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1945,10 +1945,6 @@ func TestStatusAPIStatements(t *testing.T) {
// Ignore the ALTER USER ... VIEWACTIVITY statement.
continue
}
if len(respStatement.Stats.SensitiveInfo.MostRecentPlanDescription.Name) == 0 {
// Ensure that we populate the explain plan.
t.Fatal("expected MostRecentPlanDescription to be populated")
}
statementsInResponse = append(statementsInResponse, respStatement.Key.KeyData.Query)
}

Expand Down Expand Up @@ -2061,11 +2057,6 @@ func TestStatusAPICombinedStatements(t *testing.T) {
continue
}

if len(respStatement.Stats.SensitiveInfo.MostRecentPlanDescription.Name) == 0 {
// Ensure that we populate the explain plan.
t.Fatal("expected MostRecentPlanDescription to be populated")
}

statementsInResponse = append(statementsInResponse, respStatement.Key.KeyData.Query)
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/sql/instrumentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,13 @@ func (ih *instrumentationHelper) Setup(
ih.savePlanForStats =
statsCollector.ShouldSaveLogicalPlanDesc(fingerprint, implicitTxn, p.SessionData().Database)

if ih.ShouldBuildExplainPlan() {
// Populate traceMetadata early in case we short-circuit the execution
// before reaching the bottom of this method.
ih.traceMetadata = make(execNodeTraceMetadata)
}
defer func() {
if ih.ShouldBuildExplainPlan() {
// Populate traceMetadata at the end once we have all properties of
// the helper setup.
ih.traceMetadata = make(execNodeTraceMetadata)
}
}()

if sp := tracing.SpanFromContext(ctx); sp != nil {
if sp.IsVerbose() {
Expand Down Expand Up @@ -270,9 +272,6 @@ func (ih *instrumentationHelper) Setup(
}

ih.collectExecStats = true
if ih.traceMetadata == nil {
ih.traceMetadata = make(execNodeTraceMetadata)
}
ih.evalCtx = p.EvalContext()
newCtx, ih.sp = tracing.EnsureChildSpan(ctx, cfg.AmbientCtx.Tracer, "traced statement", tracing.WithRecording(tracing.RecordingVerbose))
ih.shouldFinishSpan = true
Expand Down Expand Up @@ -424,7 +423,8 @@ func (ih *instrumentationHelper) ShouldUseJobForCreateStats() bool {
// ShouldBuildExplainPlan returns true if we should build an explain plan and
// call RecordExplainPlan.
func (ih *instrumentationHelper) ShouldBuildExplainPlan() bool {
return ih.collectBundle || ih.savePlanForStats || ih.outputMode == explainAnalyzePlanOutput ||
return ih.collectBundle || ih.collectExecStats || ih.savePlanForStats ||
ih.outputMode == explainAnalyzePlanOutput ||
ih.outputMode == explainAnalyzeDistSQLOutput
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/sqlstats/cluster_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ var SampleLogicalPlans = settings.RegisterBoolSetting(
settings.TenantWritable,
"sql.metrics.statement_details.plan_collection.enabled",
"periodically save a logical plan for each fingerprint",
true,
false,
).WithPublic()

// LogicalPlanCollectionPeriod specifies the interval between collections of
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/sqlstats/persistedsqlstats/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ func TestSQLStatsDataDriven(t *testing.T) {
observerConn := cluster.ServerConn(1 /* idx */)

observer := sqlutils.MakeSQLRunner(observerConn)
_, err := sqlConn.Exec(`SET CLUSTER SETTING sql.metrics.statement_details.plan_collection.enabled = true;`)
if err != nil {
t.Errorf("failed to enable plan collection due to %s", err.Error())
}

execDataDrivenTestCmd := func(t *testing.T, d *datadriven.TestData) string {
switch d.Cmd {
Expand Down
4 changes: 4 additions & 0 deletions pkg/util/log/logcrash/crash_reporting_packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ func TestInternalErrorReporting(t *testing.T) {

s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
defer s.Stopper().Stop(ctx)
_, err := sqlDB.Exec(`SET CLUSTER SETTING sql.metrics.statement_details.plan_collection.enabled = true;`)
if err != nil {
t.Errorf("failed to enable plan collection due to %s", err.Error())
}

if _, err := sqlDB.Exec("SELECT crdb_internal.force_assertion_error('woo')"); !testutils.IsError(err, "internal error") {
t.Fatalf("expected internal error, got %v", err)
Expand Down