From e811eb6a51669179abb97259f2d8f47b1b308755 Mon Sep 17 00:00:00 2001 From: Mark Sirek Date: Mon, 2 May 2022 14:02:33 -0700 Subject: [PATCH] stats: enable auto stats collection on system tables Fixes #80123 Previously, mutations to system tables did not trigger automatic collection of statistics to influence optimizer costs and plan selection. This was inadequate because system tables are being used in increasingly sophisticated ways in queries, most notably around driving subsystems in CRDB, requiring avoidance of full table scans. Manual collection of stats on system tables is not sufficient to meet requirements as system tables are driven by automatic processes/jobs. To address this, this patch enables auto stats collection on system tables by default, which can be disabled by setting new cluster setting `sql.stats.system_tables_autostats.enabled` to false. Auto stats are always disabled on `system.lease`, `system.table_statistics`, `system.jobs` and `system.scheduled_jobs`, no matter the value of the cluster setting. Autostats on the first two tables could potentially cause hangs, and autostats on the last two tables could potentially impact system performance. Release note: none --- .../settings/settings-for-tenants.txt | 1 + docs/generated/settings/settings.html | 1 + pkg/sql/catalog/catpb/catalog.go | 4 + pkg/sql/create_stats.go | 12 ++ .../testdata/logic_test/distsql_event_log | 6 +- .../testdata/logic_test/distsql_stats | 8 + pkg/sql/logictest/testdata/logic_test/jobs | 5 + .../logictest/testdata/logic_test/show_source | 166 +++++++++--------- pkg/sql/logictest/testdata/logic_test/system | 153 ++++++++-------- pkg/sql/opt/exec/execbuilder/testdata/explain | 39 ++-- pkg/sql/stats/automatic_stats.go | 16 +- pkg/sql/stats/automatic_stats_test.go | 122 +++++++++++++ pkg/sql/stats/stats_cache.go | 35 +++- 13 files changed, 382 insertions(+), 186 deletions(-) diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index 698fce6cfcc1..d27c9fa67e3b 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -260,6 +260,7 @@ sql.stats.persisted_rows.max integer 1000000 maximum number of rows of statement sql.stats.post_events.enabled boolean false if set, an event is logged for every CREATE STATISTICS job sql.stats.response.max integer 20000 the maximum number of statements and transaction stats returned in a CombinedStatements request sql.stats.system_tables.enabled boolean true when true, enables use of statistics on system tables by the query optimizer +sql.stats.system_tables_autostats.enabled boolean true when true, enables automatic collection of statistics on system tables sql.telemetry.query_sampling.enabled boolean false when set to true, executed queries will emit an event on the telemetry logging channel sql.temp_object_cleaner.cleanup_interval duration 30m0s how often to clean up orphaned temporary objects sql.temp_object_cleaner.wait_interval duration 30m0s how long after creation a temporary object will be cleaned up diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 19c6f5648370..2232bd9b3a14 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -191,6 +191,7 @@ sql.stats.post_events.enabledbooleanfalseif set, an event is logged for every CREATE STATISTICS job sql.stats.response.maxinteger20000the maximum number of statements and transaction stats returned in a CombinedStatements request sql.stats.system_tables.enabledbooleantruewhen true, enables use of statistics on system tables by the query optimizer +sql.stats.system_tables_autostats.enabledbooleantruewhen true, enables automatic collection of statistics on system tables sql.telemetry.query_sampling.enabledbooleanfalsewhen set to true, executed queries will emit an event on the telemetry logging channel sql.temp_object_cleaner.cleanup_intervalduration30m0show often to clean up orphaned temporary objects sql.temp_object_cleaner.wait_intervalduration30m0show long after creation a temporary object will be cleaned up diff --git a/pkg/sql/catalog/catpb/catalog.go b/pkg/sql/catalog/catpb/catalog.go index d506b48ccb71..07a3e71bba83 100644 --- a/pkg/sql/catalog/catpb/catalog.go +++ b/pkg/sql/catalog/catpb/catalog.go @@ -38,6 +38,10 @@ const ( // cluster setting. UseStatsOnSystemTables = "sql.stats.system_tables.enabled" + // AutoStatsOnSystemTables is the name of the autostats on system tables + // cluster setting. + AutoStatsOnSystemTables = "sql.stats.system_tables_autostats.enabled" + // AutoStatsMinStaleTableSettingName is the name of the automatic stats collection // min stale rows table setting. AutoStatsMinStaleTableSettingName = "sql_stats_automatic_collection_min_stale_rows" diff --git a/pkg/sql/create_stats.go b/pkg/sql/create_stats.go index 5965a813890e..aad8df17085f 100644 --- a/pkg/sql/create_stats.go +++ b/pkg/sql/create_stats.go @@ -243,6 +243,18 @@ func (n *createStatsNode) makeJobRecord(ctx context.Context) (*jobs.Record, erro ) } + if tableDesc.GetID() == keys.JobsTableID { + return nil, pgerror.New( + pgcode.WrongObjectType, "cannot create statistics on system.jobs", + ) + } + + if tableDesc.GetID() == keys.ScheduledJobsTableID { + return nil, pgerror.New( + pgcode.WrongObjectType, "cannot create statistics on system.scheduled_jobs", + ) + } + if err := n.p.CheckPrivilege(ctx, tableDesc, privilege.SELECT); err != nil { return nil, err } diff --git a/pkg/sql/logictest/testdata/logic_test/distsql_event_log b/pkg/sql/logictest/testdata/logic_test/distsql_event_log index 7423d1d99245..caaffcd09120 100644 --- a/pkg/sql/logictest/testdata/logic_test/distsql_event_log +++ b/pkg/sql/logictest/testdata/logic_test/distsql_event_log @@ -2,6 +2,10 @@ # CREATE STATISTICS ################### +# Keep auto stats jobs on system tables from disrupting tests in this file. +statement ok +SET CLUSTER SETTING sql.stats.system_tables_autostats.enabled = FALSE + # This test verifies that events are posted for table statistics creation. statement ok SET CLUSTER SETTING sql.stats.post_events.enabled = TRUE @@ -18,7 +22,7 @@ CREATE STATISTICS __auto__ FROM a query IIT SELECT "targetID", "reportingID", "info"::JSONB - 'Timestamp' - 'DescriptorID' FROM system.eventlog -WHERE "eventType" = 'create_statistics' +WHERE "eventType" = 'create_statistics' AND "targetID" <> 12 ORDER BY "timestamp", info ---- 106 1 {"EventType": "create_statistics", "Statement": "CREATE STATISTICS s1 ON id FROM test.public.a", "TableName": "test.public.a", "Tag": "CREATE STATISTICS", "User": "root"} diff --git a/pkg/sql/logictest/testdata/logic_test/distsql_stats b/pkg/sql/logictest/testdata/logic_test/distsql_stats index 869a41935836..978e116d02c9 100644 --- a/pkg/sql/logictest/testdata/logic_test/distsql_stats +++ b/pkg/sql/logictest/testdata/logic_test/distsql_stats @@ -1367,3 +1367,11 @@ ANALYZE system.lease # Collecting stats on system.table_statistics is disallowed. statement error pq: cannot create statistics on system.table_statistics ANALYZE system.table_statistics + +# Collecting stats on system.jobs is disallowed. +statement error pq: cannot create statistics on system.jobs +ANALYZE system.jobs + +# Collecting stats on system.scheduled_jobs is disallowed. +statement error pq: cannot create statistics on system.scheduled_jobs +ANALYZE system.scheduled_jobs diff --git a/pkg/sql/logictest/testdata/logic_test/jobs b/pkg/sql/logictest/testdata/logic_test/jobs index 9c5f8dee0385..b88a951e1892 100644 --- a/pkg/sql/logictest/testdata/logic_test/jobs +++ b/pkg/sql/logictest/testdata/logic_test/jobs @@ -24,6 +24,7 @@ CREATE INDEX ON t(x) query TTT SELECT job_type, description, user_name FROM [SHOW JOBS] WHERE user_name = 'root' +AND job_type LIKE 'SCHEMA CHANGE%' ---- SCHEMA CHANGE updating version for users table root SCHEMA CHANGE updating version for role options table root @@ -33,6 +34,7 @@ SCHEMA CHANGE GC GC for temporary index used during index backfill root query TTT SELECT job_type, description, user_name FROM crdb_internal.jobs WHERE user_name = 'root' +AND job_type LIKE 'SCHEMA CHANGE%' ---- SCHEMA CHANGE updating version for users table root SCHEMA CHANGE updating version for role options table root @@ -42,6 +44,7 @@ SCHEMA CHANGE GC GC for temporary index used during index backfi query TTT SELECT job_type, description, user_name FROM crdb_internal.jobs WHERE user_name = 'node' +AND job_type LIKE 'AUTO SPAN%' ---- AUTO SPAN CONFIG RECONCILIATION reconciling span configurations node @@ -84,6 +87,7 @@ user root query TTT SELECT job_type, description, user_name FROM [SHOW JOBS] WHERE user_name IN ('root', 'testuser', 'node') +AND job_type LIKE 'SCHEMA CHANGE%' ---- SCHEMA CHANGE updating version for users table root SCHEMA CHANGE updating version for role options table root @@ -95,6 +99,7 @@ SCHEMA CHANGE GC GC for temporary index used during index backfill testuser query TTT SELECT job_type, description, user_name FROM crdb_internal.jobs WHERE user_name IN ('root', 'testuser', 'node') +AND (job_type LIKE 'AUTO SPAN%' OR job_type LIKE 'SCHEMA CHANGE%') ---- AUTO SPAN CONFIG RECONCILIATION reconciling span configurations node SCHEMA CHANGE updating version for users table root diff --git a/pkg/sql/logictest/testdata/logic_test/show_source b/pkg/sql/logictest/testdata/logic_test/show_source index d3ff5995942c..5f237313d711 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_source +++ b/pkg/sql/logictest/testdata/logic_test/show_source @@ -236,89 +236,89 @@ SELECT * FROM [SHOW SEQUENCES FROM system] ---- sequence_schema sequence_name -query TTTTIT colnames,rowsort -SELECT * FROM [SHOW TABLES FROM system] ----- -schema_name table_name type owner estimated_row_count locality -public descriptor table NULL 0 NULL -public tenant_settings table NULL 0 NULL -public span_configurations table NULL 0 NULL -public sql_instances table NULL 0 NULL -public tenant_usage table NULL 0 NULL -public database_role_settings table NULL 0 NULL -public transaction_statistics table NULL 0 NULL -public statement_statistics table NULL 0 NULL -public join_tokens table NULL 0 NULL -public migrations table NULL 0 NULL -public sqlliveness table NULL 0 NULL -public scheduled_jobs table NULL 0 NULL -public statement_diagnostics table NULL 0 NULL -public statement_diagnostics_requests table NULL 0 NULL -public statement_bundle_chunks table NULL 0 NULL -public role_options table NULL 0 NULL -public protected_ts_records table NULL 0 NULL -public protected_ts_meta table NULL 0 NULL -public namespace table NULL 0 NULL -public reports_meta table NULL 0 NULL -public replication_stats table NULL 0 NULL -public replication_critical_localities table NULL 0 NULL -public replication_constraint_stats table NULL 0 NULL -public comments table NULL 0 NULL -public role_members table NULL 0 NULL -public locations table NULL 0 NULL -public table_statistics table NULL 0 NULL -public web_sessions table NULL 0 NULL -public jobs table NULL 0 NULL -public ui table NULL 0 NULL -public rangelog table NULL 0 NULL -public eventlog table NULL 0 NULL -public lease table NULL 0 NULL -public tenants table NULL 0 NULL -public settings table NULL 0 NULL -public zones table NULL 0 NULL -public users table NULL 0 NULL - -query TTTTITT colnames,rowsort -SELECT * FROM [SHOW TABLES FROM system WITH COMMENT] ----- -schema_name table_name type owner estimated_row_count locality comment -public descriptor table NULL 0 NULL · -public tenant_settings table NULL 0 NULL · -public span_configurations table NULL 0 NULL · -public sql_instances table NULL 0 NULL · -public tenant_usage table NULL 0 NULL · -public database_role_settings table NULL 0 NULL · -public transaction_statistics table NULL 0 NULL · -public statement_statistics table NULL 0 NULL · -public join_tokens table NULL 0 NULL · -public migrations table NULL 0 NULL · -public sqlliveness table NULL 0 NULL · -public scheduled_jobs table NULL 0 NULL · -public statement_diagnostics table NULL 0 NULL · -public statement_diagnostics_requests table NULL 0 NULL · -public statement_bundle_chunks table NULL 0 NULL · -public role_options table NULL 0 NULL · -public protected_ts_records table NULL 0 NULL · -public protected_ts_meta table NULL 0 NULL · -public namespace table NULL 0 NULL · -public reports_meta table NULL 0 NULL · -public replication_stats table NULL 0 NULL · -public replication_critical_localities table NULL 0 NULL · -public replication_constraint_stats table NULL 0 NULL · -public comments table NULL 0 NULL · -public role_members table NULL 0 NULL · -public locations table NULL 0 NULL · -public table_statistics table NULL 0 NULL · -public web_sessions table NULL 0 NULL · -public jobs table NULL 0 NULL · -public ui table NULL 0 NULL · -public rangelog table NULL 0 NULL · -public eventlog table NULL 0 NULL · -public lease table NULL 0 NULL · -public tenants table NULL 0 NULL · -public settings table NULL 0 NULL · -public zones table NULL 0 NULL · -public users table NULL 0 NULL · +query TTTTT colnames,rowsort +SELECT schema_name, table_name, type, owner, locality FROM [SHOW TABLES FROM system] +---- +schema_name table_name type owner locality +public comments table NULL NULL +public database_role_settings table NULL NULL +public descriptor table NULL NULL +public eventlog table NULL NULL +public jobs table NULL NULL +public join_tokens table NULL NULL +public lease table NULL NULL +public locations table NULL NULL +public migrations table NULL NULL +public namespace table NULL NULL +public protected_ts_meta table NULL NULL +public protected_ts_records table NULL NULL +public rangelog table NULL NULL +public replication_constraint_stats table NULL NULL +public replication_critical_localities table NULL NULL +public replication_stats table NULL NULL +public reports_meta table NULL NULL +public role_members table NULL NULL +public role_options table NULL NULL +public scheduled_jobs table NULL NULL +public settings table NULL NULL +public span_configurations table NULL NULL +public sql_instances table NULL NULL +public sqlliveness table NULL NULL +public statement_bundle_chunks table NULL NULL +public statement_diagnostics table NULL NULL +public statement_diagnostics_requests table NULL NULL +public statement_statistics table NULL NULL +public table_statistics table NULL NULL +public tenant_settings table NULL NULL +public tenant_usage table NULL NULL +public tenants table NULL NULL +public transaction_statistics table NULL NULL +public ui table NULL NULL +public users table NULL NULL +public web_sessions table NULL NULL +public zones table NULL NULL + +query TTTTTT colnames,rowsort +SELECT schema_name, table_name, type, owner, locality, comment FROM [SHOW TABLES FROM system WITH COMMENT] +---- +schema_name table_name type owner locality comment +public descriptor table NULL NULL · +public tenant_settings table NULL NULL · +public span_configurations table NULL NULL · +public sql_instances table NULL NULL · +public tenant_usage table NULL NULL · +public database_role_settings table NULL NULL · +public transaction_statistics table NULL NULL · +public statement_statistics table NULL NULL · +public join_tokens table NULL NULL · +public migrations table NULL NULL · +public sqlliveness table NULL NULL · +public scheduled_jobs table NULL NULL · +public statement_diagnostics table NULL NULL · +public statement_diagnostics_requests table NULL NULL · +public statement_bundle_chunks table NULL NULL · +public role_options table NULL NULL · +public protected_ts_records table NULL NULL · +public protected_ts_meta table NULL NULL · +public namespace table NULL NULL · +public reports_meta table NULL NULL · +public replication_stats table NULL NULL · +public replication_critical_localities table NULL NULL · +public replication_constraint_stats table NULL NULL · +public comments table NULL NULL · +public role_members table NULL NULL · +public locations table NULL NULL · +public table_statistics table NULL NULL · +public web_sessions table NULL NULL · +public jobs table NULL NULL · +public ui table NULL NULL · +public rangelog table NULL NULL · +public eventlog table NULL NULL · +public lease table NULL NULL · +public tenants table NULL NULL · +public settings table NULL NULL · +public zones table NULL NULL · +public users table NULL NULL · query ITTT colnames SELECT node_id, user_name, application_name, active_queries diff --git a/pkg/sql/logictest/testdata/logic_test/system b/pkg/sql/logictest/testdata/logic_test/system index c390850fafb7..320efcc3e705 100644 --- a/pkg/sql/logictest/testdata/logic_test/system +++ b/pkg/sql/logictest/testdata/logic_test/system @@ -9,86 +9,87 @@ test root NULL {} NULL # The test expectations are different on tenants because of # descriptor_id_sq, tenant, tenant_usage, and span_configurations. skipif config 3node-tenant -query TTTTIT -SHOW TABLES FROM system +query TTTTT +SELECT schema_name, table_name, type, owner, locality FROM [SHOW TABLES FROM system] ORDER BY 2 ---- -public comments table NULL 0 NULL -public database_role_settings table NULL 0 NULL -public descriptor table NULL 0 NULL -public eventlog table NULL 0 NULL -public jobs table NULL 0 NULL -public join_tokens table NULL 0 NULL -public lease table NULL 0 NULL -public locations table NULL 0 NULL -public migrations table NULL 0 NULL -public namespace table NULL 0 NULL -public protected_ts_meta table NULL 0 NULL -public protected_ts_records table NULL 0 NULL -public rangelog table NULL 0 NULL -public replication_constraint_stats table NULL 0 NULL -public replication_critical_localities table NULL 0 NULL -public replication_stats table NULL 0 NULL -public reports_meta table NULL 0 NULL -public role_members table NULL 0 NULL -public role_options table NULL 0 NULL -public scheduled_jobs table NULL 0 NULL -public settings table NULL 0 NULL -public span_configurations table NULL 0 NULL -public sql_instances table NULL 0 NULL -public sqlliveness table NULL 0 NULL -public statement_bundle_chunks table NULL 0 NULL -public statement_diagnostics table NULL 0 NULL -public statement_diagnostics_requests table NULL 0 NULL -public statement_statistics table NULL 0 NULL -public table_statistics table NULL 0 NULL -public tenant_settings table NULL 0 NULL -public tenant_usage table NULL 0 NULL -public tenants table NULL 0 NULL -public transaction_statistics table NULL 0 NULL -public ui table NULL 0 NULL -public users table NULL 0 NULL -public web_sessions table NULL 0 NULL -public zones table NULL 0 NULL +public comments table NULL NULL +public database_role_settings table NULL NULL +public descriptor table NULL NULL +public eventlog table NULL NULL +public jobs table NULL NULL +public join_tokens table NULL NULL +public lease table NULL NULL +public locations table NULL NULL +public migrations table NULL NULL +public namespace table NULL NULL +public protected_ts_meta table NULL NULL +public protected_ts_records table NULL NULL +public rangelog table NULL NULL +public replication_constraint_stats table NULL NULL +public replication_critical_localities table NULL NULL +public replication_stats table NULL NULL +public reports_meta table NULL NULL +public role_members table NULL NULL +public role_options table NULL NULL +public scheduled_jobs table NULL NULL +public settings table NULL NULL +public span_configurations table NULL NULL +public sql_instances table NULL NULL +public sqlliveness table NULL NULL +public statement_bundle_chunks table NULL NULL +public statement_diagnostics table NULL NULL +public statement_diagnostics_requests table NULL NULL +public statement_statistics table NULL NULL +public table_statistics table NULL NULL +public tenant_settings table NULL NULL +public tenant_usage table NULL NULL +public tenants table NULL NULL +public transaction_statistics table NULL NULL +public ui table NULL NULL +public users table NULL NULL +public web_sessions table NULL NULL +public zones table NULL NULL onlyif config 3node-tenant -query TTTTIT -SHOW TABLES FROM system +query TTTTT +SELECT schema_name, table_name, type, owner, locality FROM [SHOW TABLES FROM system] ORDER BY 2 ---- -public comments table NULL 0 NULL -public database_role_settings table NULL 0 NULL -public descriptor table NULL 0 NULL -public descriptor_id_seq sequence NULL 0 NULL -public eventlog table NULL 0 NULL -public jobs table NULL 0 NULL -public join_tokens table NULL 0 NULL -public lease table NULL 0 NULL -public locations table NULL 0 NULL -public migrations table NULL 0 NULL -public namespace table NULL 0 NULL -public protected_ts_meta table NULL 0 NULL -public protected_ts_records table NULL 0 NULL -public rangelog table NULL 0 NULL -public replication_constraint_stats table NULL 0 NULL -public replication_critical_localities table NULL 0 NULL -public replication_stats table NULL 0 NULL -public reports_meta table NULL 0 NULL -public role_members table NULL 0 NULL -public role_options table NULL 0 NULL -public scheduled_jobs table NULL 0 NULL -public settings table NULL 0 NULL -public span_count table NULL 0 NULL -public sql_instances table NULL 0 NULL -public sqlliveness table NULL 0 NULL -public statement_bundle_chunks table NULL 0 NULL -public statement_diagnostics table NULL 0 NULL -public statement_diagnostics_requests table NULL 0 NULL -public statement_statistics table NULL 0 NULL -public table_statistics table NULL 0 NULL -public transaction_statistics table NULL 0 NULL -public ui table NULL 0 NULL -public users table NULL 0 NULL -public web_sessions table NULL 0 NULL -public zones table NULL 0 NULL +public comments table NULL NULL +public database_role_settings table NULL NULL +public descriptor table NULL NULL +public descriptor_id_seq sequence NULL NULL +public eventlog table NULL NULL +public jobs table NULL NULL +public join_tokens table NULL NULL +public lease table NULL NULL +public locations table NULL NULL +public migrations table NULL NULL +public namespace table NULL NULL +public protected_ts_meta table NULL NULL +public protected_ts_records table NULL NULL +public rangelog table NULL NULL +public replication_constraint_stats table NULL NULL +public replication_critical_localities table NULL NULL +public replication_stats table NULL NULL +public reports_meta table NULL NULL +public role_members table NULL NULL +public role_options table NULL NULL +public scheduled_jobs table NULL NULL +public settings table NULL NULL +public span_count table NULL NULL +public sql_instances table NULL NULL +public sqlliveness table NULL NULL +public statement_bundle_chunks table NULL NULL +public statement_diagnostics table NULL NULL +public statement_diagnostics_requests table NULL NULL +public statement_statistics table NULL NULL +public table_statistics table NULL NULL +public transaction_statistics table NULL NULL +public ui table NULL NULL +public users table NULL NULL +public web_sessions table NULL NULL +public zones table NULL NULL + # The test expectations are different on tenants because of # descriptor_id_sq, tenant, tenant_usage, and span_configurations. diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain b/pkg/sql/opt/exec/execbuilder/testdata/explain index 7b2bb5e3ad10..17fdb50280f7 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/explain +++ b/pkg/sql/opt/exec/execbuilder/testdata/explain @@ -16,19 +16,6 @@ vectorized: true · • norows -query T -EXPLAIN (PLAN) SELECT 1 FROM system.jobs WHERE TRUE ----- -distribution: local -vectorized: true -· -• render -│ -└── • scan - missing stats - table: jobs@jobs_status_created_idx - spans: FULL SCAN - query T EXPLAIN (PLAN, VERBOSE) SELECT 1 a ---- @@ -666,50 +653,68 @@ vectorized: true └── • virtual table table: pg_namespace@primary -query T +statement OK +ANALYZE system.users + +statement OK +ANALYZE system.role_options + +statement OK +ANALYZE system.role_members + +query T retry EXPLAIN SHOW USERS ---- distribution: local vectorized: true · • sort +│ estimated row count: 3 │ order: +username │ └── • render + │ estimated row count: 3 │ └── • group (hash) + │ estimated row count: 3 │ group by: username │ └── • sort + │ estimated row count: 3 │ order: +"role" │ └── • hash join (left outer) + │ estimated row count: 3 │ equality: (username) = (member) │ left cols are key │ ├── • group (hash) + │ │ estimated row count: 3 │ │ group by: username │ │ │ └── • window + │ │ estimated row count: 3 │ │ │ └── • render + │ │ estimated row count: 3 │ │ │ └── • merge join (left outer) + │ │ estimated row count: 3 │ │ equality: (username) = (username) │ │ left cols are key │ │ │ ├── • scan - │ │ missing stats + │ │ estimated row count: 3 (100% of the table; stats collected ago) │ │ table: users@primary │ │ spans: FULL SCAN │ │ │ └── • scan - │ missing stats + │ estimated row count: 1 (100% of the table; stats collected ago) │ table: role_options@primary │ spans: FULL SCAN │ └── • scan - missing stats + estimated row count: 1 (100% of the table; stats collected ago) table: role_members@role_members_role_idx spans: FULL SCAN diff --git a/pkg/sql/stats/automatic_stats.go b/pkg/sql/stats/automatic_stats.go index bad7aad1f625..82651bc793a3 100644 --- a/pkg/sql/stats/automatic_stats.go +++ b/pkg/sql/stats/automatic_stats.go @@ -57,6 +57,17 @@ var UseStatisticsOnSystemTables = settings.RegisterBoolSetting( true, ).WithPublic() +// AutomaticStatisticsOnSystemTables controls the cluster setting for enabling +// automatic statistics collection on system tables. Auto stats must be enabled +// via a true setting of sql.stats.automatic_collection.enabled for this flag to +// have any effect. +var AutomaticStatisticsOnSystemTables = settings.RegisterBoolSetting( + settings.TenantWritable, + catpb.AutoStatsOnSystemTables, + "when true, enables automatic collection of statistics on system tables", + true, +).WithPublic() + // MultiColumnStatisticsClusterMode controls the cluster setting for enabling // automatic collection of multi-column statistics. var MultiColumnStatisticsClusterMode = settings.RegisterBoolSetting( @@ -607,8 +618,9 @@ func (r *Refresher) NotifyMutation(table catalog.TableDescriptor, rowsAffected i if !r.autoStatsEnabled(table) { return } - if !autostatsCollectionAllowed(table) { - // Don't collect stats for this kind of table: system, virtual, view, etc. + if !autostatsCollectionAllowed(table, r.st) { + // Don't collect stats for virtual tables or views. System tables may be + // allowed if enabled in cluster settings. return } diff --git a/pkg/sql/stats/automatic_stats_test.go b/pkg/sql/stats/automatic_stats_test.go index 059962b1bc75..55d8196c2a0e 100644 --- a/pkg/sql/stats/automatic_stats_test.go +++ b/pkg/sql/stats/automatic_stats_test.go @@ -132,6 +132,37 @@ func TestMaybeRefreshStats(t *testing.T) { t.Fatal(err) } + // Auto stats collection on any system table except system.lease and + // system.table_statistics should succeed. + descRoleOptions := + desctestutils.TestingGetPublicTableDescriptor(s.DB(), keys.SystemSQLCodec, "system", "role_options") + refresher.maybeRefreshStats( + ctx, descRoleOptions.GetID(), nil /* explicitSettings */, 10000 /* rowsAffected */, time.Microsecond, /* asOf */ + ) + if err := checkStatsCount(ctx, cache, descRoleOptions, 4 /* expected */); err != nil { + t.Fatal(err) + } + + // Auto stats collection on system.lease should fail (no stats should be collected). + descLease := + desctestutils.TestingGetPublicTableDescriptor(s.DB(), keys.SystemSQLCodec, "system", "lease") + refresher.maybeRefreshStats( + ctx, descLease.GetID(), nil /* explicitSettings */, 10000 /* rowsAffected */, time.Microsecond, /* asOf */ + ) + if err := checkStatsCount(ctx, cache, descLease, 0 /* expected */); err != nil { + t.Fatal(err) + } + + // Auto stats collection on system.table_statistics should fail (no stats should be collected). + descTableStats := + desctestutils.TestingGetPublicTableDescriptor(s.DB(), keys.SystemSQLCodec, "system", "table_statistics") + refresher.maybeRefreshStats( + ctx, descTableStats.GetID(), nil /* explicitSettings */, 10000 /* rowsAffected */, time.Microsecond, /* asOf */ + ) + if err := checkStatsCount(ctx, cache, descTableStats, 0 /* expected */); err != nil { + t.Fatal(err) + } + // Ensure that attempt to refresh stats on view does not result in re- // enqueuing the attempt. // TODO(rytaft): Should not enqueue views to begin with. @@ -669,6 +700,70 @@ func TestDefaultColumns(t *testing.T) { }) } +func TestAnalyzeSystemTables(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + ctx := context.Background() + + s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) + defer s.Stopper().Stop(ctx) + + st := cluster.MakeTestingClusterSettings() + AutomaticStatisticsClusterMode.Override(ctx, &st.SV, false) + evalCtx := eval.NewTestingEvalContext(st) + defer evalCtx.Stop(ctx) + executor := s.InternalExecutor().(sqlutil.InternalExecutor) + cache := NewTableStatisticsCache( + ctx, + 10, /* cacheSize */ + kvDB, + executor, + keys.SystemSQLCodec, + s.ClusterSettings(), + s.RangeFeedFactory().(*rangefeed.Factory), + s.CollectionFactory().(*descs.CollectionFactory), + ) + + var tableNames []string + tableNames = make([]string, 0, 40) + + it, err := executor.QueryIterator( + ctx, + "get-system-tables", + nil, /* txn */ + "SELECT table_name FROM [SHOW TABLES FROM SYSTEM]", + ) + if err != nil { + t.Fatal(err) + } + + var ok bool + for ok, err = it.Next(ctx); ok; ok, err = it.Next(ctx) { + if err != nil { + t.Fatal(err) + } + row := it.Cur() + tableName := string(*row[0].(*tree.DOidWrapper).Wrapped.(*tree.DString)) + tableNames = append(tableNames, tableName) + } + sqlRun := sqlutils.MakeSQLRunner(sqlDB) + expectZeroRows := false + for _, tableName := range tableNames { + // Stats may not be collected on system.lease and system.table_statistics. + if tableName == "lease" || tableName == "table_statistics" || + tableName == "jobs" || tableName == "scheduled_jobs" { + continue + } + sql := fmt.Sprintf("ANALYZE system.%s", tableName) + sqlRun.Exec(t, sql) + // We're testing that ANALYZE on every system table except the above two + // doesn't error out, and populates system.table_statistics. + if err := compareStatsCountWithZero(ctx, cache, tableName, s, expectZeroRows); err != nil { + t.Fatal(err) + } + } +} + func checkStatsCount( ctx context.Context, cache *TableStatisticsCache, table catalog.TableDescriptor, expected int, ) error { @@ -683,3 +778,30 @@ func checkStatsCount( return nil }) } + +func compareStatsCountWithZero( + ctx context.Context, + cache *TableStatisticsCache, + tableName string, + s serverutils.TestServerInterface, + expectZeroRows bool, +) error { + desc := + desctestutils.TestingGetPublicTableDescriptor(s.DB(), keys.SystemSQLCodec, "system", tableName) + return testutils.SucceedsSoonError(func() error { + stats, err := cache.GetTableStats(ctx, desc) + if err != nil { + return err + } + if expectZeroRows { + if len(stats) != 0 { + return fmt.Errorf("expected no stats but found %d stats rows", len(stats)) + } + } else { + if len(stats) == 0 { + return fmt.Errorf("expected stats but found no stats rows") + } + } + return nil + }) +} diff --git a/pkg/sql/stats/stats_cache.go b/pkg/sql/stats/stats_cache.go index 6fcb55777d43..313fc9606456 100644 --- a/pkg/sql/stats/stats_cache.go +++ b/pkg/sql/stats/stats_cache.go @@ -220,15 +220,24 @@ func (sc *TableStatisticsCache) GetTableStats( return sc.getTableStatsFromCache(ctx, table.GetID()) } +func statsDisallowedSystemTable(tableID descpb.ID) bool { + switch tableID { + case keys.TableStatisticsTableID, keys.LeaseTableID, keys.JobsTableID, keys.ScheduledJobsTableID: + return true + } + return false +} + // statsUsageAllowed returns true if statistics on `table` are allowed to be // used by the query optimizer. func statsUsageAllowed(table catalog.TableDescriptor, clusterSettings *cluster.Settings) bool { if catalog.IsSystemDescriptor(table) { // Disable stats usage on system.table_statistics and system.lease. Looking // up stats on system.lease is known to cause hangs, and the same could - // happen with system.table_statistics. - if table.GetID() == keys.TableStatisticsTableID || - table.GetID() == keys.LeaseTableID { + // happen with system.table_statistics. Stats on system.jobs and + // system.scheduled_jobs are also disallowed because autostats are disabled + // on them. + if statsDisallowedSystemTable(table.GetID()) { return false } // Return whether the optimizer is allowed to use stats on system tables. @@ -239,11 +248,23 @@ func statsUsageAllowed(table catalog.TableDescriptor, clusterSettings *cluster.S // autostatsCollectionAllowed returns true if statistics are allowed to be // automatically collected on the table. -func autostatsCollectionAllowed(table catalog.TableDescriptor) bool { +func autostatsCollectionAllowed( + table catalog.TableDescriptor, clusterSettings *cluster.Settings, +) bool { if catalog.IsSystemDescriptor(table) { - // Don't try to get statistics for system tables (most importantly, - // for table_statistics itself). - return false + // Disable autostats on system.table_statistics and system.lease. Looking + // up stats on system.lease is known to cause hangs, and the same could + // happen with system.table_statistics. No need to collect stats if we + // cannot use them. Stats on system.jobs and system.scheduled_jobs + // are also disallowed because they are mutated too frequently and would + // trigger too many stats collections. The potential benefit is not worth + // the potential performance hit. + if statsDisallowedSystemTable(table.GetID()) { + return false + } + // Return whether autostats collection is allowed on system tables, + // according to the cluster settings. + return AutomaticStatisticsOnSystemTables.Get(&clusterSettings.SV) } return tableTypeCanHaveStats(table) }