Merge cockroachdb#108835

108835: persistedsqlstats: add cluster setting to enable flush on row limit r… r=xinhaoz a=xinhaoz

…eached

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: cockroachdb#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. 

Co-authored-by: Xin Hao Zhang <[email protected]>

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

docs/generated/settings/settings.html

@@ -233,7 +233,7 @@
 <tr><td><div id="setting-sql-stats-histogram-samples-count" class="anchored"><code>sql.stats.histogram_samples.count</code></div></td><td>integer</td><td><code>10000</code></td><td>number of rows sampled for histogram construction during table statistics collection</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-multi-column-collection-enabled" class="anchored"><code>sql.stats.multi_column_collection.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>multi-column statistics collection mode</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-non-default-columns-min-retention-period" class="anchored"><code>sql.stats.non_default_columns.min_retention_period</code></div></td><td>duration</td><td><code>24h0m0s</code></td><td>minimum retention period for table statistics collected on non-default columns</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-sql-stats-persisted-rows-max" class="anchored"><code>sql.stats.persisted_rows.max</code></div></td><td>integer</td><td><code>1000000</code></td><td>maximum number of rows of statement and transaction statistics that will be persisted in the system tables</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-sql-stats-persisted-rows-max" class="anchored"><code>sql.stats.persisted_rows.max</code></div></td><td>integer</td><td><code>1000000</code></td><td>maximum number of rows of statement and transaction statistics that will be persisted in the system tables before compaction begins</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-post-events-enabled" class="anchored"><code>sql.stats.post_events.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if set, an event is logged for every CREATE STATISTICS job</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-response-max" class="anchored"><code>sql.stats.response.max</code></div></td><td>integer</td><td><code>20000</code></td><td>the maximum number of statements and transaction stats returned in a CombinedStatements request</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-response-show-internal-enabled" class="anchored"><code>sql.stats.response.show_internal.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>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</td><td>Serverless/Dedicated/Self-Hosted</td></tr>

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,
+)

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)
 	}

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) {