Merge cockroachdb#91955

91955: cluster-ui: transaction insights bug fixes r=xinhaoz a=xinhaoz

Fixes cockroachdb#91914
Fixes cockroachdb#91915
Fixes cockroachdb#91927

This commit fixes txn insights bugs and adds unit
tests for utils used by the insights pages.

Fixes for txn insights:
- De-duplicate txn contention insights labels in overview page for high contention
- Remove query column for statement details and single stmt transactions
- Include statement query for a txn insight recommendation if there are multiple stmts in the txn being reported

Release note (ui change): Query column in insight recs table removed. Instead, stmt is included in the description if the txn being reported has multiple stmts.

This txn has multiple stmts:
<img width="1678" alt="image" src="https://user-images.githubusercontent.com/20136951/202526318-601587ea-8b2c-4708-b058-b24cad66c71d.png">

Ths txn has 1 stmt:
<img width="1736" alt="image" src="https://user-images.githubusercontent.com/20136951/202040846-f02faecf-0c68-4b2c-bca3-ac813ac94a3a.png">

No change to stmts (but query col is removed everywhere):
<img width="1734" alt="image" src="https://user-images.githubusercontent.com/20136951/202040904-d21c1ed3-ed91-4cf7-b77e-bc74736f7d44.png">


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

pkg/ui/workspaces/cluster-ui/src/api/insightsApi.ts

@@ -19,6 +19,7 @@ import {
 } from "./sqlApi";
 import {
   BlockedContentionDetails,
+  dedupInsights,
   FlattenedStmtInsightEvent,
   getInsightFromCause,
   getInsightsFromProblemsAndCauses,
@@ -564,10 +565,14 @@ function organizeExecutionInsightsResponseIntoTxns(
     return [];
   }
 
-  const txnByExecID = new Map<string, TxnInsightEvent>();
+  // Map of Transaction  exec and fingerprint id -> txn.
+  const txnByIDs = new Map<string, TxnInsightEvent>();
+  const getTxnKey = (row: ExecutionInsightsResponseRow) =>
+    row.txn_id.concat(row.txn_fingerprint_id);
 
   response.execution.txn_results[0].rows.forEach(row => {
-    let txnInsight: TxnInsightEvent = txnByExecID.get(row.txn_id);
+    const rowKey = getTxnKey(row);
+    let txnInsight: TxnInsightEvent = txnByIDs.get(rowKey);
 
     if (!txnInsight) {
       txnInsight = {
@@ -587,7 +592,7 @@ function organizeExecutionInsightsResponseIntoTxns(
         insights: [],
         queries: [],
       };
-      txnByExecID.set(row.txn_id, txnInsight);
+      txnByIDs.set(rowKey, txnInsight);
     }
 
     const start = moment.utc(row.start_time);
@@ -630,15 +635,9 @@ function organizeExecutionInsightsResponseIntoTxns(
     );
   });
 
-  txnByExecID.forEach(txn => {
+  txnByIDs.forEach(txn => {
     // De-duplicate top-level txn insights.
-    const insightsSeen = new Set<string>();
-    txn.insights = txn.insights.reduce((insights, i) => {
-      if (insightsSeen.has(i.name)) return insights;
-      insightsSeen.add(i.name);
-      insights.push(i);
-      return insights;
-    }, []);
+    txn.insights = dedupInsights(txn.insights);
 
     // Sort stmt insights for each txn by start time.
     txn.statementInsights.sort((a, b) => {
@@ -648,7 +647,7 @@ function organizeExecutionInsightsResponseIntoTxns(
     });
   });
 
-  return Array.from(txnByExecID.values());
+  return Array.from(txnByIDs.values());
 }
 
 type InsightQuery<ResponseColumnType, State> = {

pkg/ui/workspaces/cluster-ui/src/insights/types.ts

@@ -179,7 +179,7 @@ export type ContentionEvent = {
   execType: InsightExecEnum;
 };
 
-const highContentionInsight = (
+export const highContentionInsight = (
   execType: InsightExecEnum,
   latencyThreshold?: number,
   contentionDuration?: number,
@@ -207,9 +207,9 @@ const highContentionInsight = (
   };
 };
 
-const slowExecutionInsight = (
+export const slowExecutionInsight = (
   execType: InsightExecEnum,
-  latencyThreshold: number,
+  latencyThreshold?: number,
 ): Insight => {
   let threshold = latencyThreshold + "ms";
   if (!latencyThreshold) {
@@ -226,7 +226,7 @@ const slowExecutionInsight = (
   };
 };
 
-const planRegressionInsight = (execType: InsightExecEnum): Insight => {
+export const planRegressionInsight = (execType: InsightExecEnum): Insight => {
   const description =
     `This ${execType} was slow because we picked the wrong plan, ` +
     `possibly due to outdated statistics, the statement using different literals or ` +
@@ -240,7 +240,7 @@ const planRegressionInsight = (execType: InsightExecEnum): Insight => {
   };
 };
 
-const suboptimalPlanInsight = (execType: InsightExecEnum): Insight => {
+export const suboptimalPlanInsight = (execType: InsightExecEnum): Insight => {
   const description =
     `This ${execType} was slow because a good plan was not available, whether ` +
     `due to outdated statistics or missing indexes.`;
@@ -253,7 +253,7 @@ const suboptimalPlanInsight = (execType: InsightExecEnum): Insight => {
   };
 };
 
-const highRetryCountInsight = (execType: InsightExecEnum): Insight => {
+export const highRetryCountInsight = (execType: InsightExecEnum): Insight => {
   const description =
     `This ${execType} has being retried more times than the value of the ` +
     `'sql.insights.high_retry_count.threshold' cluster setting.`;
@@ -266,7 +266,7 @@ const highRetryCountInsight = (execType: InsightExecEnum): Insight => {
   };
 };
 
-const failedExecutionInsight = (execType: InsightExecEnum): Insight => {
+export const failedExecutionInsight = (execType: InsightExecEnum): Insight => {
   const description =
     `This ${execType} execution failed completely, due to contention, resource ` +
     `saturation, or syntax errors.`;