Merge #53759

53759: sql: create and fill `crdb_internal.node_transaction_statistics` table r=solongordon a=arulajmani

Previously there was no way to query the transaction level metrics
collected by individual nodes. This patch adds this capability by
exposing a new internal table called
`crdb_internal.node_transaction_statistics` which is analagous to
`crdb_internal.node_statement_statistics` albeit for transactions.

Closes #53504

Release justification: low risk update to new functionality
Release note (sql change): A new crdb_internal table called
`node_transaction_statistics` is exposed as part of this change,
which allows users to query transaction metrics collected on a
particular node.

Co-authored-by: arulajmani <arulajmani@gmail.com>

pkg/cli/testdata/zip/partial1

@@ -40,6 +40,7 @@ retrieving SQL data for crdb_internal.node_queries... writing: debug/nodes/1/crd
 retrieving SQL data for crdb_internal.node_runtime_info... writing: debug/nodes/1/crdb_internal.node_runtime_info.txt
 retrieving SQL data for crdb_internal.node_sessions... writing: debug/nodes/1/crdb_internal.node_sessions.txt
 retrieving SQL data for crdb_internal.node_statement_statistics... writing: debug/nodes/1/crdb_internal.node_statement_statistics.txt
+retrieving SQL data for crdb_internal.node_transaction_statistics... writing: debug/nodes/1/crdb_internal.node_transaction_statistics.txt
 retrieving SQL data for crdb_internal.node_transactions... writing: debug/nodes/1/crdb_internal.node_transactions.txt
 retrieving SQL data for crdb_internal.node_txn_stats... writing: debug/nodes/1/crdb_internal.node_txn_stats.txt
 requesting data for debug/nodes/1/details... writing: debug/nodes/1/details.json
@@ -128,6 +129,9 @@ writing: debug/nodes/2/crdb_internal.node_sessions.txt.err.txt
 retrieving SQL data for crdb_internal.node_statement_statistics... writing: debug/nodes/2/crdb_internal.node_statement_statistics.txt
 writing: debug/nodes/2/crdb_internal.node_statement_statistics.txt.err.txt
   ^- resulted in ...
+retrieving SQL data for crdb_internal.node_transaction_statistics... writing: debug/nodes/2/crdb_internal.node_transaction_statistics.txt
+writing: debug/nodes/2/crdb_internal.node_transaction_statistics.txt.err.txt
+  ^- resulted in ...
 retrieving SQL data for crdb_internal.node_transactions... writing: debug/nodes/2/crdb_internal.node_transactions.txt
 writing: debug/nodes/2/crdb_internal.node_transactions.txt.err.txt
   ^- resulted in ...
@@ -168,6 +172,7 @@ retrieving SQL data for crdb_internal.node_queries... writing: debug/nodes/3/crd
 retrieving SQL data for crdb_internal.node_runtime_info... writing: debug/nodes/3/crdb_internal.node_runtime_info.txt
 retrieving SQL data for crdb_internal.node_sessions... writing: debug/nodes/3/crdb_internal.node_sessions.txt
 retrieving SQL data for crdb_internal.node_statement_statistics... writing: debug/nodes/3/crdb_internal.node_statement_statistics.txt
+retrieving SQL data for crdb_internal.node_transaction_statistics... writing: debug/nodes/3/crdb_internal.node_transaction_statistics.txt
 retrieving SQL data for crdb_internal.node_transactions... writing: debug/nodes/3/crdb_internal.node_transactions.txt
 retrieving SQL data for crdb_internal.node_txn_stats... writing: debug/nodes/3/crdb_internal.node_txn_stats.txt
 requesting data for debug/nodes/3/details... writing: debug/nodes/3/details.json

pkg/cli/testdata/zip/partial1_excluded

@@ -40,6 +40,7 @@ retrieving SQL data for crdb_internal.node_queries... writing: debug/nodes/1/crd
 retrieving SQL data for crdb_internal.node_runtime_info... writing: debug/nodes/1/crdb_internal.node_runtime_info.txt
 retrieving SQL data for crdb_internal.node_sessions... writing: debug/nodes/1/crdb_internal.node_sessions.txt
 retrieving SQL data for crdb_internal.node_statement_statistics... writing: debug/nodes/1/crdb_internal.node_statement_statistics.txt
+retrieving SQL data for crdb_internal.node_transaction_statistics... writing: debug/nodes/1/crdb_internal.node_transaction_statistics.txt
 retrieving SQL data for crdb_internal.node_transactions... writing: debug/nodes/1/crdb_internal.node_transactions.txt
 retrieving SQL data for crdb_internal.node_txn_stats... writing: debug/nodes/1/crdb_internal.node_txn_stats.txt
 requesting data for debug/nodes/1/details... writing: debug/nodes/1/details.json
@@ -105,6 +106,7 @@ retrieving SQL data for crdb_internal.node_queries... writing: debug/nodes/3/crd
 retrieving SQL data for crdb_internal.node_runtime_info... writing: debug/nodes/3/crdb_internal.node_runtime_info.txt
 retrieving SQL data for crdb_internal.node_sessions... writing: debug/nodes/3/crdb_internal.node_sessions.txt
 retrieving SQL data for crdb_internal.node_statement_statistics... writing: debug/nodes/3/crdb_internal.node_statement_statistics.txt
+retrieving SQL data for crdb_internal.node_transaction_statistics... writing: debug/nodes/3/crdb_internal.node_transaction_statistics.txt
 retrieving SQL data for crdb_internal.node_transactions... writing: debug/nodes/3/crdb_internal.node_transactions.txt
 retrieving SQL data for crdb_internal.node_txn_stats... writing: debug/nodes/3/crdb_internal.node_txn_stats.txt
 requesting data for debug/nodes/3/details... writing: debug/nodes/3/details.json

pkg/cli/testdata/zip/partial2

@@ -40,6 +40,7 @@ retrieving SQL data for crdb_internal.node_queries... writing: debug/nodes/1/crd
 retrieving SQL data for crdb_internal.node_runtime_info... writing: debug/nodes/1/crdb_internal.node_runtime_info.txt
 retrieving SQL data for crdb_internal.node_sessions... writing: debug/nodes/1/crdb_internal.node_sessions.txt
 retrieving SQL data for crdb_internal.node_statement_statistics... writing: debug/nodes/1/crdb_internal.node_statement_statistics.txt
+retrieving SQL data for crdb_internal.node_transaction_statistics... writing: debug/nodes/1/crdb_internal.node_transaction_statistics.txt
 retrieving SQL data for crdb_internal.node_transactions... writing: debug/nodes/1/crdb_internal.node_transactions.txt
 retrieving SQL data for crdb_internal.node_txn_stats... writing: debug/nodes/1/crdb_internal.node_txn_stats.txt
 requesting data for debug/nodes/1/details... writing: debug/nodes/1/details.json
@@ -104,6 +105,7 @@ retrieving SQL data for crdb_internal.node_queries... writing: debug/nodes/3/crd
 retrieving SQL data for crdb_internal.node_runtime_info... writing: debug/nodes/3/crdb_internal.node_runtime_info.txt
 retrieving SQL data for crdb_internal.node_sessions... writing: debug/nodes/3/crdb_internal.node_sessions.txt
 retrieving SQL data for crdb_internal.node_statement_statistics... writing: debug/nodes/3/crdb_internal.node_statement_statistics.txt
+retrieving SQL data for crdb_internal.node_transaction_statistics... writing: debug/nodes/3/crdb_internal.node_transaction_statistics.txt
 retrieving SQL data for crdb_internal.node_transactions... writing: debug/nodes/3/crdb_internal.node_transactions.txt
 retrieving SQL data for crdb_internal.node_txn_stats... writing: debug/nodes/3/crdb_internal.node_txn_stats.txt
 requesting data for debug/nodes/3/details... writing: debug/nodes/3/details.json

pkg/cli/testdata/zip/testzip

@@ -42,6 +42,7 @@ retrieving SQL data for crdb_internal.node_queries... writing: debug/nodes/1/crd
 retrieving SQL data for crdb_internal.node_runtime_info... writing: debug/nodes/1/crdb_internal.node_runtime_info.txt
 retrieving SQL data for crdb_internal.node_sessions... writing: debug/nodes/1/crdb_internal.node_sessions.txt
 retrieving SQL data for crdb_internal.node_statement_statistics... writing: debug/nodes/1/crdb_internal.node_statement_statistics.txt
+retrieving SQL data for crdb_internal.node_transaction_statistics... writing: debug/nodes/1/crdb_internal.node_transaction_statistics.txt
 retrieving SQL data for crdb_internal.node_transactions... writing: debug/nodes/1/crdb_internal.node_transactions.txt
 retrieving SQL data for crdb_internal.node_txn_stats... writing: debug/nodes/1/crdb_internal.node_txn_stats.txt
 requesting data for debug/nodes/1/details... writing: debug/nodes/1/details.json

pkg/cli/zip.go

@@ -96,6 +96,7 @@ var debugZipTablesPerNode = []string{
 	"crdb_internal.node_runtime_info",
 	"crdb_internal.node_sessions",
 	"crdb_internal.node_statement_statistics",
+	"crdb_internal.node_transaction_statistics",
 	"crdb_internal.node_transactions",
 	"crdb_internal.node_txn_stats",
 }

pkg/sql/app_stats.go

@@ -261,13 +261,15 @@ func (a *appStats) getStatsForStmtWithKey(
 	return s
 }
 
-func (a *appStats) getStatsForTxnWithKey(key txnKey, stmtIDs []roachpb.StmtID) *txnStats {
+func (a *appStats) getStatsForTxnWithKey(
+	key txnKey, stmtIDs []roachpb.StmtID, createIfNonexistent bool,
+) *txnStats {
 	a.Lock()
 	defer a.Unlock()
 	// Retrieve the per-transaction statistic object, and create it if it doesn't
 	// exist yet.
 	s, ok := a.txns[key]
-	if !ok {
+	if !ok && createIfNonexistent {
 		s = &txnStats{}
 		s.statementIDs = stmtIDs
 		a.txns[key] = s
@@ -325,7 +327,7 @@ func (a *appStats) Add(other *appStats) {
 
 	// Merge the txn stats
 	for k, v := range txnMap {
-		t := a.getStatsForTxnWithKey(k, v.statementIDs)
+		t := a.getStatsForTxnWithKey(k, v.statementIDs, true /* createIfNonExistent */)
 		t.mu.Lock()
 		t.mu.data.Add(&v.mu.data)
 		t.mu.Unlock()
@@ -406,7 +408,7 @@ func (a *appStats) recordTransaction(
 	}
 
 	// Get the statistics object.
-	s := a.getStatsForTxnWithKey(key, statementIDs)
+	s := a.getStatsForTxnWithKey(key, statementIDs, true /* createIfNonexistent */)
 
 	// Collect the per-transaction statistics.
 	s.mu.Lock()

pkg/sql/catalog/catconstants/constants.go

@@ -78,6 +78,7 @@ const (
 	CrdbInternalTableIndexesTableID
 	CrdbInternalTablesTableID
 	CrdbInternalTablesTableLastStatsID
+	CrdbInternalTransactionStatsTableID
 	CrdbInternalTxnStatsTableID
 	CrdbInternalZonesTableID
 	InformationSchemaID

pkg/sql/crdb_internal.go

@@ -44,6 +44,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
 	"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
@@ -115,6 +116,7 @@ var crdbInternal = virtualSchema{
 		catconstants.CrdbInternalTableIndexesTableID:         crdbInternalTableIndexesTable,
 		catconstants.CrdbInternalTablesTableLastStatsID:      crdbInternalTablesTableLastStats,
 		catconstants.CrdbInternalTablesTableID:               crdbInternalTablesTable,
+		catconstants.CrdbInternalTransactionStatsTableID:     crdbInternalTransactionStatisticsTable,
 		catconstants.CrdbInternalTxnStatsTableID:             crdbInternalTxnStatsTable,
 		catconstants.CrdbInternalZonesTableID:                crdbInternalZonesTable,
 	},
@@ -715,6 +717,20 @@ func (s stmtList) Less(i, j int) bool {
 	return s[i].anonymizedStmt < s[j].anonymizedStmt
 }
 
+type txnList []txnKey
+
+func (t txnList) Len() int {
+	return len(t)
+}
+
+func (t txnList) Swap(i, j int) {
+	t[i], t[j] = t[j], t[i]
+}
+
+func (t txnList) Less(i, j int) bool {
+	return t[i] < t[j]
+}
+
 var crdbInternalStmtStatsTable = virtualSchemaTable{
 	comment: `statement statistics (in-memory, not durable; local node only). ` +
 		`This table is wiped periodically (by default, at least every two hours)`,
@@ -748,9 +764,14 @@ CREATE TABLE crdb_internal.node_statement_statistics (
   implicit_txn        BOOL NOT NULL
 )`,
 	populate: func(ctx context.Context, p *planner, _ *dbdesc.Immutable, addRow func(...tree.Datum) error) error {
-		if err := p.RequireAdminRole(ctx, "access application statistics"); err != nil {
+		hasViewActivity, err := p.HasRoleOption(ctx, roleoption.VIEWACTIVITY)
+		if err != nil {
 			return err
 		}
+		if !hasViewActivity {
+			return pgerror.Newf(pgcode.InsufficientPrivilege,
+				"user %s does not have %s privilege", p.User(), roleoption.VIEWACTIVITY)
+		}
 
 		sqlStats := p.extendedEvalCtx.sqlStatsCollector.sqlStats
 		if sqlStats == nil {
@@ -845,6 +866,117 @@ CREATE TABLE crdb_internal.node_statement_statistics (
 	},
 }
 
+var crdbInternalTransactionStatisticsTable = virtualSchemaTable{
+	comment: `finer-grained transaction statistics (in-memory, not durable; local node only). ` +
+		`This table is wiped periodically (by default, at least every two hours)`,
+	schema: `
+CREATE TABLE crdb_internal.node_transaction_statistics (
+  node_id           INT NOT NULL,
+  application_name  STRING NOT NULL,
+  key               STRING,
+  statement_ids     STRING[],
+  count             INT,
+  max_retries       INT,
+  service_lat_avg   FLOAT NOT NULL,
+  service_lat_var   FLOAT NOT NULL,
+  retry_lat_avg     FLOAT NOT NULL,
+  retry_lat_var     FLOAT NOT NULL,
+  commit_lat_avg    FLOAT NOT NULL,
+  commit_lat_var    FLOAT NOT NULL,
+  rows_read_avg     FLOAT NOT NULL,
+  rows_read_var     FLOAT NOT NULL
+)
+`,
+	populate: func(ctx context.Context, p *planner, _ *dbdesc.Immutable, addRow func(...tree.Datum) error) error {
+		hasViewActivity, err := p.HasRoleOption(ctx, roleoption.VIEWACTIVITY)
+		if err != nil {
+			return err
+		}
+		if !hasViewActivity {
+			return pgerror.Newf(pgcode.InsufficientPrivilege,
+				"user %s does not have %s privilege", p.User(), roleoption.VIEWACTIVITY)
+		}
+		sqlStats := p.extendedEvalCtx.sqlStatsCollector.sqlStats
+		if sqlStats == nil {
+			return errors.AssertionFailedf(
+				"cannot access sql statistics from this context")
+		}
+
+		nodeID, _ := p.execCfg.NodeID.OptionalNodeID() // zero if not available
+
+		// Retrieve the application names and sort them to ensure the
+		// output is deterministic.
+		var appNames []string
+		sqlStats.Lock()
+
+		for n := range sqlStats.apps {
+			appNames = append(appNames, n)
+		}
+		sqlStats.Unlock()
+		sort.Strings(appNames)
+
+		for _, appName := range appNames {
+			appStats := sqlStats.getStatsForApplication(appName)
+
+			// Retrieve the statement keys and sort them to ensure the
+			// output is deterministic.
+			var txnKeys txnList
+			appStats.Lock()
+			for k := range appStats.txns {
+				txnKeys = append(txnKeys, k)
+			}
+			appStats.Unlock()
+			sort.Sort(txnKeys)
+
+			// Now retrieve the per-txn stats proper.
+			for _, txnKey := range txnKeys {
+				// We don't want to create the key if it doesn't exist, so it's okay to
+				// pass nil for the statementIDs, as they are only set when a key is
+				// constructed.
+				s := appStats.getStatsForTxnWithKey(txnKey, nil, false /* createIfNonexistent */)
+				// If the key is not found (and we expected to find it), the table must
+				// have been cleared between now and the time we read all the keys. In
+				// that case we simply skip this key as there are no metrics to report.
+				if s == nil {
+					continue
+				}
+				stmtIDsDatum := tree.NewDArray(types.String)
+				for _, stmtID := range s.statementIDs {
+					if err := stmtIDsDatum.Append(tree.NewDString(string(stmtID))); err != nil {
+						return err
+					}
+				}
+
+				s.mu.Lock()
+
+				err := addRow(
+					tree.NewDInt(tree.DInt(nodeID)),
+					tree.NewDString(appName),
+					tree.NewDString(string(txnKey)),
+					stmtIDsDatum,
+					tree.NewDInt(tree.DInt(s.mu.data.Count)),
+					tree.NewDInt(tree.DInt(s.mu.data.MaxRetries)),
+					tree.NewDFloat(tree.DFloat(s.mu.data.ServiceLat.Mean)),
+					tree.NewDFloat(tree.DFloat(s.mu.data.ServiceLat.GetVariance(s.mu.data.Count))),
+					tree.NewDFloat(tree.DFloat(s.mu.data.RetryLat.Mean)),
+					tree.NewDFloat(tree.DFloat(s.mu.data.RetryLat.GetVariance(s.mu.data.Count))),
+					tree.NewDFloat(tree.DFloat(s.mu.data.CommitLat.Mean)),
+					tree.NewDFloat(tree.DFloat(s.mu.data.CommitLat.GetVariance(s.mu.data.Count))),
+					tree.NewDFloat(tree.DFloat(s.mu.data.NumRows.Mean)),
+					tree.NewDFloat(tree.DFloat(s.mu.data.NumRows.GetVariance(s.mu.data.Count))),
+				)
+
+				s.mu.Unlock()
+				if err != nil {
+					return err
+				}
+			}
+
+		}
+		return nil
+	},
+}
+
 var crdbInternalTxnStatsTable = virtualSchemaTable{
 	comment: `per-application transaction statistics (in-memory, not durable; local node only). ` +
 		`This table is wiped periodically (by default, at least every two hours)`,