Skip to content

Commit

Permalink
sql: turn off plan sampling by default
Browse files Browse the repository at this point in the history
Previously, we were sampling plans for fingerprints
and saving to statement_statistics tables. Now that
we are saving plan hash and plan gist (that allow us
to decode back to the plan) we are no longer using the
sampled plan anywhere. Since this is a heavy opperation,
we are turning it off by default, but changing the default
value of `sql.metrics.statement_details.plan_collection.enabled`
to `false`.
If we don't receive feedback about turning it back on,
we can remove this sampling entirely.

Partially addresses #77944

Release note (sql change): Change the default value of
`sql.metrics.statement_details.plan_collection.enabled`
to false, since we no longer use this information
anywhere.
  • Loading branch information
maryliag committed Sep 21, 2022
1 parent 7cde315 commit c1adb2d
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 12 deletions.
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 @@ -248,7 +248,7 @@ sql.metrics.statement_details.dump_to_logs boolean false dump collected statemen
sql.metrics.statement_details.enabled boolean true collect per-statement query statistics
sql.metrics.statement_details.index_recommendation_collection.enabled boolean true generate an index recommendation for each fingerprint ID
sql.metrics.statement_details.max_mem_reported_idx_recommendations integer 5000 the maximum number of reported index recommendation info stored in memory
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 @@ -182,7 +182,7 @@
<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.index_recommendation_collection.enabled</code></td><td>boolean</td><td><code>true</code></td><td>generate an index recommendation for each fingerprint ID</td></tr>
<tr><td><code>sql.metrics.statement_details.max_mem_reported_idx_recommendations</code></td><td>integer</td><td><code>5000</code></td><td>the maximum number of reported index recommendation info stored in memory</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
1 change: 1 addition & 0 deletions pkg/ccl/telemetryccl/telemetry_logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func TestTelemetryLogRegions(t *testing.T) {
// 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;`)
sqlDB.Exec(t, `SET CLUSTER SETTING sql.metrics.statement_details.plan_collection.enabled = true;`)
sqlDB.Exec(t, `SET CLUSTER SETTING sql.telemetry.query_sampling.max_event_frequency = 1000000`)

testData := []struct {
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 @@ -1818,10 +1818,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 @@ -1934,11 +1930,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
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

0 comments on commit c1adb2d

Please sign in to comment.