Merge pull request cockroachdb#101475 from xinhaoz/backport22.2-99719

docs/generated/http/full.md

@@ -4000,6 +4000,7 @@ tenant pods.
 | key | [StatementsResponse.ExtendedStatementStatisticsKey](#cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.ExtendedStatementStatisticsKey) |  |  | [reserved](#support-status) |
 | id | [uint64](#cockroach.server.serverpb.StatementsResponse-uint64) |  |  | [reserved](#support-status) |
 | stats | [cockroach.sql.StatementStatistics](#cockroach.server.serverpb.StatementsResponse-cockroach.sql.StatementStatistics) |  |  | [reserved](#support-status) |
+| txn_fingerprint_ids | [uint64](#cockroach.server.serverpb.StatementsResponse-uint64) | repeated | In 23.1 we expect the response to only group on fingerprint_id and app_name in the overview page. We now return the aggregated list of unique txn fingerprint ids, leaving the txn_fingerprint_id field in the key empty. | [reserved](#support-status) |
 
 
 
@@ -4111,6 +4112,7 @@ Support status: [reserved](#support-status)
 | key | [StatementsResponse.ExtendedStatementStatisticsKey](#cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.ExtendedStatementStatisticsKey) |  |  | [reserved](#support-status) |
 | id | [uint64](#cockroach.server.serverpb.StatementsResponse-uint64) |  |  | [reserved](#support-status) |
 | stats | [cockroach.sql.StatementStatistics](#cockroach.server.serverpb.StatementsResponse-cockroach.sql.StatementStatistics) |  |  | [reserved](#support-status) |
+| txn_fingerprint_ids | [uint64](#cockroach.server.serverpb.StatementsResponse-uint64) | repeated | In 23.1 we expect the response to only group on fingerprint_id and app_name in the overview page. We now return the aggregated list of unique txn fingerprint ids, leaving the txn_fingerprint_id field in the key empty. | [reserved](#support-status) |
 
 
 

pkg/server/combined_statement_stats.go

@@ -96,15 +96,13 @@ func getCombinedStatementStats(
 	}
 
 	if req.FetchMode != nil && req.FetchMode.StatsType == serverpb.CombinedStatementsStatsRequest_TxnStatsOnly {
-		// Change the whereClause for the statements to those matching the txn_fingerprint_ids in the
-		// transactions response that are within the desired interval. We also don't need the order and
-		// limit anymore.
-		orderAndLimit = ""
-		whereClause, args = buildWhereClauseForStmtsByTxn(req, transactions, testingKnobs)
+		// If we're fetching for txns, the client still expects statement stats for
+		// stmts in the txns response.
+		statements, err = collectStmtsForTxns(ctx, ie, req, transactions, testingKnobs)
+	} else {
+		statements, err = collectCombinedStatements(ctx, ie, whereClause, args, orderAndLimit, settings, testingKnobs)
 	}
 
-	statements, err = collectCombinedStatements(ctx, ie, whereClause, args, orderAndLimit, settings, testingKnobs)
-
 	if err != nil {
 		return nil, serverError(ctx, err)
 	}
@@ -401,17 +399,15 @@ func collectCombinedStatements(
 SELECT * FROM (
 SELECT
     fingerprint_id,
-    transaction_fingerprint_id,
+    array_agg(distinct transaction_fingerprint_id),
     app_name,
     max(aggregated_ts) as aggregated_ts,
-    metadata,
+    crdb_internal.merge_stats_metadata(array_agg(metadata)) as metadata,
     crdb_internal.merge_statement_stats(array_agg(statistics)) AS statistics
 FROM %s %s
 GROUP BY
     fingerprint_id,
-    transaction_fingerprint_id,
-    app_name,
-    metadata
+    app_name
 ) %s
 %s`
 
@@ -469,12 +465,18 @@ GROUP BY
 			return nil, serverError(ctx, err)
 		}
 
-		var transactionFingerprintID uint64
-		if transactionFingerprintID, err = sqlstatsutil.DatumToUint64(row[1]); err != nil {
-			return nil, serverError(ctx, err)
+		var txnFingerprintID uint64
+		txnFingerprintDatums := tree.MustBeDArray(row[1])
+		txnFingerprintIDs := make([]roachpb.TransactionFingerprintID, 0, txnFingerprintDatums.Array.Len())
+		for _, idDatum := range txnFingerprintDatums.Array {
+			if txnFingerprintID, err = sqlstatsutil.DatumToUint64(idDatum); err != nil {
+				return nil, serverError(ctx, err)
+			}
+			txnFingerprintIDs = append(txnFingerprintIDs, roachpb.TransactionFingerprintID(txnFingerprintID))
 		}
 
 		app := string(tree.MustBeDString(row[2]))
+
 		aggregatedTs := tree.MustBeDTimestampTZ(row[3]).Time
 
 		var metadata roachpb.CollectedStatementStatistics
@@ -484,8 +486,6 @@ GROUP BY
 		}
 
 		metadata.Key.App = app
-		metadata.Key.TransactionFingerprintID =
-			roachpb.TransactionFingerprintID(transactionFingerprintID)
 
 		statsJSON := tree.MustBeDJSON(row[5]).JSON
 		if err = sqlstatsutil.DecodeStmtStatsStatisticsJSON(statsJSON, &metadata.Stats); err != nil {
@@ -497,8 +497,9 @@ GROUP BY
 				KeyData:      metadata.Key,
 				AggregatedTs: aggregatedTs,
 			},
-			ID:    roachpb.StmtFingerprintID(statementFingerprintID),
-			Stats: metadata.Stats,
+			ID:                roachpb.StmtFingerprintID(statementFingerprintID),
+			Stats:             metadata.Stats,
+			TxnFingerprintIDs: txnFingerprintIDs,
 		}
 
 		statements = append(statements, stmt)
@@ -528,13 +529,12 @@ SELECT
     app_name,
     max(aggregated_ts) as aggregated_ts,
     fingerprint_id,
-    metadata,
+    max(metadata),
     crdb_internal.merge_transaction_stats(array_agg(statistics)) AS statistics
 FROM %s %s
 GROUP BY
     app_name,
-    fingerprint_id,
-    metadata
+    fingerprint_id
 ) %s
 %s`
 
@@ -589,6 +589,7 @@ GROUP BY
 		}
 
 		app := string(tree.MustBeDString(row[0]))
+
 		aggregatedTs := tree.MustBeDTimestampTZ(row[1]).Time
 		fingerprintID, err := sqlstatsutil.DatumToUint64(row[2])
 		if err != nil {
@@ -626,6 +627,120 @@ GROUP BY
 	return transactions, nil
 }
 
+func collectStmtsForTxns(
+	ctx context.Context,
+	ie *sql.InternalExecutor,
+	req *serverpb.CombinedStatementsStatsRequest,
+	transactions []serverpb.StatementsResponse_ExtendedCollectedTransactionStatistics,
+	testingKnobs *sqlstats.TestingKnobs,
+) ([]serverpb.StatementsResponse_CollectedStatementStatistics, error) {
+
+	whereClause, args := buildWhereClauseForStmtsByTxn(req, transactions, testingKnobs)
+
+	queryFormat := `
+SELECT
+    fingerprint_id,
+    transaction_fingerprint_id,
+    crdb_internal.merge_stats_metadata(array_agg(metadata)) AS metadata,
+    crdb_internal.merge_statement_stats(array_agg(statistics)) AS statistics,
+    app_name
+FROM %s %s
+GROUP BY
+    fingerprint_id,
+    transaction_fingerprint_id,
+    app_name
+`
+
+	const expectedNumDatums = 5
+
+	query := fmt.Sprintf(queryFormat, "crdb_internal.statement_statistics_persisted", whereClause)
+
+	it, err := ie.QueryIteratorEx(ctx, "stmts-for-txn", nil,
+		sessiondata.NodeUserSessionDataOverride, query, args...)
+
+	if err != nil {
+		return nil, serverError(ctx, err)
+	}
+
+	// If there are no results from the persisted table, retrieve the data from the combined view
+	// with data in-memory.
+	if !it.HasResults() {
+		err = closeIterator(it, err)
+
+		query = fmt.Sprintf(queryFormat, `crdb_internal.statement_statistics`, whereClause)
+
+		it, err = ie.QueryIteratorEx(ctx, "stmts-for-txn-with-memory", nil,
+			sessiondata.NodeUserSessionDataOverride, query, args...)
+
+		if err != nil {
+			return nil, serverError(ctx, err)
+		}
+	}
+
+	defer func() {
+		closeErr := it.Close()
+		if closeErr != nil {
+			err = errors.CombineErrors(err, closeErr)
+		}
+	}()
+
+	var statements []serverpb.StatementsResponse_CollectedStatementStatistics
+	var ok bool
+	for ok, err = it.Next(ctx); ok; ok, err = it.Next(ctx) {
+		var row tree.Datums
+		if row = it.Cur(); row == nil {
+			return nil, errors.New("unexpected null row on collectStmtsForTxns")
+		}
+
+		if row.Len() != expectedNumDatums {
+			return nil, errors.Newf("expected %d columns, received %d on collectStmtsForTxns", expectedNumDatums)
+		}
+
+		var statementFingerprintID uint64
+		if statementFingerprintID, err = sqlstatsutil.DatumToUint64(row[0]); err != nil {
+			return nil, serverError(ctx, err)
+		}
+
+		var txnFingerprintID uint64
+		if txnFingerprintID, err = sqlstatsutil.DatumToUint64(row[1]); err != nil {
+			return nil, serverError(ctx, err)
+		}
+
+		var metadata roachpb.CollectedStatementStatistics
+		metadataJSON := tree.MustBeDJSON(row[2]).JSON
+		if err = sqlstatsutil.DecodeStmtStatsMetadataJSON(metadataJSON, &metadata); err != nil {
+			return nil, serverError(ctx, err)
+		}
+
+		metadata.Key.TransactionFingerprintID = roachpb.TransactionFingerprintID(txnFingerprintID)
+
+		statsJSON := tree.MustBeDJSON(row[3]).JSON
+		if err = sqlstatsutil.DecodeStmtStatsStatisticsJSON(statsJSON, &metadata.Stats); err != nil {
+			return nil, serverError(ctx, err)
+		}
+
+		app := string(tree.MustBeDString(row[4]))
+		metadata.Key.App = app
+
+		stmt := serverpb.StatementsResponse_CollectedStatementStatistics{
+			Key: serverpb.StatementsResponse_ExtendedStatementStatisticsKey{
+				KeyData: metadata.Key,
+			},
+			ID:    roachpb.StmtFingerprintID(statementFingerprintID),
+			Stats: metadata.Stats,
+		}
+
+		statements = append(statements, stmt)
+
+	}
+
+	if err != nil {
+		return nil, serverError(ctx, err)
+	}
+
+	return statements, nil
+}
+
 func (s *statusServer) StatementDetails(
 	ctx context.Context, req *serverpb.StatementDetailsRequest,
 ) (*serverpb.StatementDetailsResponse, error) {

pkg/server/serverpb/status.proto

@@ -1,4 +1,4 @@
-// Copyright 2016 The Cockroach Authors.
+ // Copyright 2016 The Cockroach Authors.
 //
 // Use of this software is governed by the Business Source License
 // included in the file licenses/BSL.txt.
@@ -1542,6 +1542,11 @@ message StatementsResponse {
     uint64 id = 3 [(gogoproto.customname) = "ID",
       (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/roachpb.StmtFingerprintID"];
     cockroach.sql.StatementStatistics stats = 2 [(gogoproto.nullable) = false];
+    // In 23.1 we expect the response to only group on fingerprint_id and app_name
+    // in the overview page. We now return the aggregated list of unique txn fingerprint ids,
+    // leaving the txn_fingerprint_id field in the key empty.
+   repeated uint64 txn_fingerprint_ids = 4 [(gogoproto.customname) = "TxnFingerprintIDs",
+     (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/roachpb.TransactionFingerprintID"];
   }
 
   repeated CollectedStatementStatistics statements = 1 [(gogoproto.nullable) = false];

pkg/server/status_test.go

@@ -1932,7 +1932,9 @@ func TestStatusAPICombinedStatements(t *testing.T) {
 			}
 
 			statementsInResponse = append(statementsInResponse, respStatement.Key.KeyData.Query)
-			expectedTxnFingerprints[respStatement.Key.KeyData.TransactionFingerprintID] = struct{}{}
+			for _, txnFingerprintID := range respStatement.TxnFingerprintIDs {
+				expectedTxnFingerprints[txnFingerprintID] = struct{}{}
+			}
 		}
 
 		for _, respTxn := range resp.Transactions {
@@ -1947,6 +1949,8 @@ func TestStatusAPICombinedStatements(t *testing.T) {
 				expectedStmts, statementsInResponse, pretty.Sprint(resp), path)
 		}
 		if hasTxns {
+			// We expect that expectedTxnFingerprints is now empty since
+			// we should have removed them all.
 			assert.Empty(t, expectedTxnFingerprints)
 		} else {
 			assert.Empty(t, resp.Transactions)

pkg/ui/workspaces/cluster-ui/src/api/statementsApi.spec.ts

@@ -9,6 +9,7 @@
 // licenses/APL.txt.
 import Long from "long";
 import {
+  aggregateOnStmtFingerprintAndAppName,
   createCombinedStmtsRequest,
   getCombinedStatements,
   getFlushedTxnStatsApi,
@@ -433,3 +434,95 @@ describe("getFlushedTxnStatsApi", () => {
     },
   );
 });
+
+describe("aggregateOnStmtFingerprintAndAppName", () => {
+  it("should aggregate stmts on stmt fingerprint ID and app name only", () => {
+    const stmts = [
+      {
+        id: 1,
+        appName: "cockroach",
+        count: 3,
+      },
+      {
+        id: 1,
+        appName: "cockroach",
+        count: 1,
+      },
+      {
+        id: 1,
+        appName: "not_cockroach",
+        count: 1,
+      },
+      {
+        id: 2,
+        appName: "cockroach",
+        count: 2,
+      },
+      {
+        id: 2,
+        appName: "cockroach",
+        count: 1,
+      },
+      {
+        id: 2,
+        appName: "cockroach",
+        count: 1,
+      },
+      {
+        id: 3,
+        appName: "myApp",
+        count: 1,
+      },
+    ].map(stmt =>
+      mockStmtStats({
+        id: Long.fromInt(stmt.id),
+        key: {
+          key_data: {
+            app: stmt.appName,
+          },
+        },
+        stats: {
+          count: Long.fromInt(stmt.count),
+        },
+      }),
+    );
+
+    const aggregatedStmts = aggregateOnStmtFingerprintAndAppName(stmts)
+      .sort((stmtA, stmtB) => {
+        const comp = stmtA.id.compare(stmtB.id);
+        if (comp === 0)
+          return stmtA.key.key_data.app.localeCompare(stmtB.key.key_data.app);
+        return comp;
+      })
+      .map(stmt => ({
+        id: stmt.id.toInt(),
+        appName: stmt.key.key_data.app,
+        count: stmt.stats.count.toInt(),
+      }));
+
+    const expectedSortedRes = [
+      {
+        id: 1,
+        appName: "cockroach",
+        count: 4,
+      },
+      {
+        id: 1,
+        appName: "not_cockroach",
+        count: 1,
+      },
+      {
+        id: 2,
+        appName: "cockroach",
+        count: 4,
+      },
+      {
+        id: 3,
+        appName: "myApp",
+        count: 1,
+      },
+    ];
+
+    expect(aggregatedStmts).toEqual(expectedSortedRes);
+  });
+});