Merge #65575

65575: sql: instantiate metrics for internal sql stats r=Azhng a=Azhng Previously, we did not instantiate metrics for internal SQL stats. However, this can cause issues when connExecutor is being created by InternalExecutor. In order to address this, we also instantiate metrics for internal stats. Release note: None Closes #65523 Co-authored-by: Azhng <[email protected]>
cockroachdb · May 22, 2021 · 17517f3 · 17517f3
2 parents b4d8f7b + 0955915
commit 17517f3
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 23 deletions.
diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go
@@ -324,28 +324,6 @@ func NewServer(cfg *ExecutorConfig, pool *mon.BytesMonitor) *Server {
 }
 
 func makeMetrics(cfg *ExecutorConfig, internal bool) Metrics {
-	var statsMetrics StatsMetrics
-	if !internal {
-		statsMetrics = StatsMetrics{
-			SQLStatsMemoryMaxBytesHist: metric.NewHistogram(
-				getMetricMeta(MetaSQLStatsMemMaxBytes, internal),
-				cfg.HistogramWindowInterval,
-				log10int64times1000,
-				3, /* sigFigs */
-			),
-			SQLStatsMemoryCurBytesCount: metric.NewGauge(
-				getMetricMeta(MetaSQLStatsMemCurBytes, internal)),
-			ReportedSQLStatsMemoryMaxBytesHist: metric.NewHistogram(
-				getMetricMeta(MetaReportedSQLStatsMemMaxBytes, internal),
-				cfg.HistogramWindowInterval,
-				log10int64times1000,
-				3, /* sigFigs */
-			),
-			ReportedSQLStatsMemoryCurBytesCount: metric.NewGauge(
-				getMetricMeta(MetaReportedSQLStatsMemCurBytes, internal)),
-			DiscardedStatsCount: metric.NewCounter(getMetricMeta(MetaDiscardedSQLStats, internal)),
-		}
-	}
 	return Metrics{
 		EngineMetrics: EngineMetrics{
 			DistSQLSelectCount:    metric.NewCounter(getMetricMeta(MetaDistSQLSelect, internal)),
@@ -371,7 +349,25 @@ func makeMetrics(cfg *ExecutorConfig, internal bool) Metrics {
 		},
 		StartedStatementCounters:  makeStartedStatementCounters(internal),
 		ExecutedStatementCounters: makeExecutedStatementCounters(internal),
-		StatsMetrics:              statsMetrics,
+		StatsMetrics: StatsMetrics{
+			SQLStatsMemoryMaxBytesHist: metric.NewHistogram(
+				getMetricMeta(MetaSQLStatsMemMaxBytes, internal),
+				cfg.HistogramWindowInterval,
+				log10int64times1000,
+				3, /* sigFigs */
+			),
+			SQLStatsMemoryCurBytesCount: metric.NewGauge(
+				getMetricMeta(MetaSQLStatsMemCurBytes, internal)),
+			ReportedSQLStatsMemoryMaxBytesHist: metric.NewHistogram(
+				getMetricMeta(MetaReportedSQLStatsMemMaxBytes, internal),
+				cfg.HistogramWindowInterval,
+				log10int64times1000,
+				3, /* sigFigs */
+			),
+			ReportedSQLStatsMemoryCurBytesCount: metric.NewGauge(
+				getMetricMeta(MetaReportedSQLStatsMemCurBytes, internal)),
+			DiscardedStatsCount: metric.NewCounter(getMetricMeta(MetaDiscardedSQLStats, internal)),
+		},
 	}
 }
 

diff --git a/pkg/sql/internal_test.go b/pkg/sql/internal_test.go
@@ -173,6 +173,33 @@ func TestInternalFullTableScan(t *testing.T) {
 	require.NoError(t, err)
 }
 
+// Test for regression https://github.com/cockroachdb/cockroach/issues/65523
+func TestInternalStmtFingerprintLimit(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	ctx := context.Background()
+	params, _ := tests.CreateTestServerParams()
+	s, db, _ := serverutils.StartServer(t, params)
+	defer s.Stopper().Stop(ctx)
+
+	_, err := db.Exec("SET CLUSTER SETTING sql.metrics.max_mem_app_txn_fingerprints = 0;")
+	require.NoError(t, err)
+
+	_, err = db.Exec("SET CLUSTER SETTING sql.metrics.max_mem_app_stmt_fingerprints = 0;")
+	require.NoError(t, err)
+
+	ie := sql.MakeInternalExecutor(
+		ctx,
+		s.(*server.TestServer).Server.PGServer().SQLServer,
+		sql.MemoryMetrics{},
+		s.ExecutorConfig().(sql.ExecutorConfig).Settings,
+	)
+
+	_, err = ie.Exec(ctx, "stmt-exceeds-fingerprint-limit", nil, "SELECT 1")
+	require.NoError(t, err)
+}
+
 func TestQueryIsAdminWithNoTxn(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)

diff --git a/pkg/sql/pgwire/server.go b/pkg/sql/pgwire/server.go
@@ -351,6 +351,7 @@ func (s *Server) Metrics() (res []interface{}) {
 		&s.SQLServer.InternalMetrics.StartedStatementCounters,
 		&s.SQLServer.InternalMetrics.ExecutedStatementCounters,
 		&s.SQLServer.InternalMetrics.EngineMetrics,
+		&s.SQLServer.InternalMetrics.StatsMetrics,
 	}
 }
 

diff --git a/pkg/ts/catalog/chart_catalog.go b/pkg/ts/catalog/chart_catalog.go
@@ -1714,6 +1714,26 @@ var charts = []sectionDescription{
 				Title:   "Number of fingerprint statistics being discarded",
 				Metrics: []string{"sql.stats.discarded.current"},
 			},
+			{
+				Title:   "Memory usage for internal fingerprint storage",
+				Metrics: []string{"sql.stats.mem.max.internal"},
+			},
+			{
+				Title:   "Current memory usage for internal fingerprint storage",
+				Metrics: []string{"sql.stats.mem.current.internal"},
+			},
+			{
+				Title:   "Memory usage for internal reported fingerprint storage",
+				Metrics: []string{"sql.stats.reported.mem.max.internal"},
+			},
+			{
+				Title:   "Current memory usage for internal reported fingerprint storage",
+				Metrics: []string{"sql.stats.reported.mem.current.internal"},
+			},
+			{
+				Title:   "Number of internal fingerprint statistics being discarded",
+				Metrics: []string{"sql.stats.discarded.current.internal"},
+			},
 		},
 	},
 	{