From cce6a17019ad95d5f8aea5381592022b7475fed8 Mon Sep 17 00:00:00 2001 From: Xin Hao Zhang Date: Wed, 16 Aug 2023 00:36:43 +0100 Subject: [PATCH] persistedsqlstats: add cluster setting to enable flush on row limit reached This commit introduces the cluster setting `sql.stats.limit_table_size.enabled` which controls whether or not we enforce the row limit set by `sql.stats.persited_rows.max` in the system.{statement,transaction} statistics tables. Previously, when the sql stats tables reach the row limit set by `sql.stats.persisted_rows.max`, we disable flushing any more stats to the tables. This restriction involved issuing a query during the flush process to check the number of rows currently in the tables. This query can often contend with the compaction job queries and so we should allow a method to bypass this check to alleviate the contention. Furhtermore, in some cases we'd like to allow the system tables to grow in order to not have gaps in sql stats observability. In these cases we can attempt to modify the compaction settings to allow the job to catch up to the target number of rows. Fixes: #108771 Release note (sql change): This commit introduces the cluster setting `sql.stats.limit_table_size.enabled` which controls whether or not we enforce the row limit set by `sql.stats.persited_rows.max` in the system.{statement,transaction} statistics tables. --- .../settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- .../persistedsqlstats/cluster_settings.go | 15 +++++++++-- pkg/sql/sqlstats/persistedsqlstats/flush.go | 8 +++++- .../sqlstats/persistedsqlstats/flush_test.go | 27 +++++++++++++++---- 5 files changed, 44 insertions(+), 10 deletions(-) diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index eb954afb7d2b..8689ac8c921f 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -279,7 +279,7 @@ sql.stats.histogram_collection.enabled boolean true histogram collection mode te sql.stats.histogram_samples.count integer 10000 number of rows sampled for histogram construction during table statistics collection tenant-rw sql.stats.multi_column_collection.enabled boolean true multi-column statistics collection mode tenant-rw sql.stats.non_default_columns.min_retention_period duration 24h0m0s minimum retention period for table statistics collected on non-default columns tenant-rw -sql.stats.persisted_rows.max integer 1000000 maximum number of rows of statement and transaction statistics that will be persisted in the system tables tenant-rw +sql.stats.persisted_rows.max integer 1000000 maximum number of rows of statement and transaction statistics that will be persisted in the system tables before compaction begins tenant-rw sql.stats.post_events.enabled boolean false if set, an event is logged for every CREATE STATISTICS job tenant-rw sql.stats.response.max integer 20000 the maximum number of statements and transaction stats returned in a CombinedStatements request tenant-rw sql.stats.response.show_internal.enabled boolean false controls if statistics for internal executions should be returned by the CombinedStatements and if internal sessions should be returned by the ListSessions endpoints. These endpoints are used to display statistics on the SQL Activity pages tenant-rw diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index ecc6ef2f0611..9a1595c67e31 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -233,7 +233,7 @@
sql.stats.histogram_samples.count
integer10000number of rows sampled for histogram construction during table statistics collectionServerless/Dedicated/Self-Hosted
sql.stats.multi_column_collection.enabled
booleantruemulti-column statistics collection modeServerless/Dedicated/Self-Hosted
sql.stats.non_default_columns.min_retention_period
duration24h0m0sminimum retention period for table statistics collected on non-default columnsServerless/Dedicated/Self-Hosted -
sql.stats.persisted_rows.max
integer1000000maximum number of rows of statement and transaction statistics that will be persisted in the system tablesServerless/Dedicated/Self-Hosted +
sql.stats.persisted_rows.max
integer1000000maximum number of rows of statement and transaction statistics that will be persisted in the system tables before compaction beginsServerless/Dedicated/Self-Hosted
sql.stats.post_events.enabled
booleanfalseif set, an event is logged for every CREATE STATISTICS jobServerless/Dedicated/Self-Hosted
sql.stats.response.max
integer20000the maximum number of statements and transaction stats returned in a CombinedStatements requestServerless/Dedicated/Self-Hosted
sql.stats.response.show_internal.enabled
booleanfalsecontrols if statistics for internal executions should be returned by the CombinedStatements and if internal sessions should be returned by the ListSessions endpoints. These endpoints are used to display statistics on the SQL Activity pagesServerless/Dedicated/Self-Hosted diff --git a/pkg/sql/sqlstats/persistedsqlstats/cluster_settings.go b/pkg/sql/sqlstats/persistedsqlstats/cluster_settings.go index 61c7db7794b7..273b3b209318 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/cluster_settings.go +++ b/pkg/sql/sqlstats/persistedsqlstats/cluster_settings.go @@ -88,8 +88,8 @@ var SQLStatsFlushJitter = settings.RegisterFloatSetting( var SQLStatsMaxPersistedRows = settings.RegisterIntSetting( settings.TenantWritable, "sql.stats.persisted_rows.max", - "maximum number of rows of statement and transaction"+ - " statistics that will be persisted in the system tables", + "maximum number of rows of statement and transaction statistics that "+ + "will be persisted in the system tables before compaction begins", 1000000, /* defaultValue */ ).WithPublic() @@ -129,3 +129,14 @@ var CompactionJobRowsToDeletePerTxn = settings.RegisterIntSetting( 10000, settings.NonNegativeInt, ) + +// sqlStatsLimitTableSizeEnabled is the cluster setting that enables the +// sql stats system tables to grow past the number of rows set by +// sql.stats.persisted_row.max. +var sqlStatsLimitTableSizeEnabled = settings.RegisterBoolSetting( + settings.TenantWritable, + "sql.stats.limit_table_size.enabled", + "controls whether we allow statement and transaction statistics tables "+ + "to grow past sql.stats.persisted_rows.max", + true, +) diff --git a/pkg/sql/sqlstats/persistedsqlstats/flush.go b/pkg/sql/sqlstats/persistedsqlstats/flush.go index 6ba6b8e955f0..17b729a80a4e 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/flush.go +++ b/pkg/sql/sqlstats/persistedsqlstats/flush.go @@ -73,7 +73,13 @@ func (s *PersistedSQLStats) Flush(ctx context.Context) { aggregatedTs := s.ComputeAggregatedTs() // We only check the statement count as there should always be at least as many statements as transactions. - limitReached, err := s.StmtsLimitSizeReached(ctx) + limitReached := false + + var err error + if sqlStatsLimitTableSizeEnabled.Get(&s.SQLStats.GetClusterSettings().SV) { + limitReached, err = s.StmtsLimitSizeReached(ctx) + } + if err != nil { log.Errorf(ctx, "encountered an error at flush, checking for statement statistics size limit: %v", err) } diff --git a/pkg/sql/sqlstats/persistedsqlstats/flush_test.go b/pkg/sql/sqlstats/persistedsqlstats/flush_test.go index 7cf4210c7f21..ae0bb1d2737e 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/flush_test.go +++ b/pkg/sql/sqlstats/persistedsqlstats/flush_test.go @@ -15,6 +15,7 @@ import ( gosql "database/sql" "fmt" "math" + "strconv" "testing" "time" @@ -499,12 +500,28 @@ func TestSQLStatsPersistedLimitReached(t *testing.T) { sqlConn.Exec(t, "SELECT 1, 2, 3, 4") sqlConn.Exec(t, "SELECT 1, 2, 3, 4, 5") sqlConn.Exec(t, "SELECT 1, 2, 3, 4, 6, 7") - pss.Flush(ctx) - stmtStatsCountFlush3, txnStatsCountFlush3 := countStats(t, sqlConn) - // 5. Assert that neither table has grown in length. - require.Equal(t, stmtStatsCountFlush3, stmtStatsCountFlush2) - require.Equal(t, txnStatsCountFlush3, txnStatsCountFlush2) + for _, enforceLimitEnabled := range []bool{true, false} { + boolStr := strconv.FormatBool(enforceLimitEnabled) + t.Run("enforce-limit-"+boolStr, func(t *testing.T) { + + sqlConn.Exec(t, "SET CLUSTER SETTING sql.stats.limit_table_size.enabled = "+boolStr) + + pss.Flush(ctx) + + stmtStatsCountFlush3, txnStatsCountFlush3 := countStats(t, sqlConn) + + if enforceLimitEnabled { + // Assert that neither table has grown in length. + require.Equal(t, stmtStatsCountFlush3, stmtStatsCountFlush2) + require.Equal(t, txnStatsCountFlush3, txnStatsCountFlush2) + } else { + // Assert that tables were allowed to grow. + require.Greater(t, stmtStatsCountFlush3, stmtStatsCountFlush2) + require.Greater(t, txnStatsCountFlush3, txnStatsCountFlush2) + } + }) + } } func TestSQLStatsReadLimitSizeOnLockedTable(t *testing.T) {