Skip to content

Commit

Permalink
cluster-ui: transaction insights bug fixes
Browse files Browse the repository at this point in the history
Fixes cockroachdb#91914
Fixes cockroachdb#91915
Fixes cockroachdb#91927

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.
  • Loading branch information
xinhaoz committed Nov 15, 2022
1 parent 5a24e11 commit 9296682
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 48 deletions.
9 changes: 2 additions & 7 deletions pkg/ui/workspaces/cluster-ui/src/api/insightsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from "./sqlApi";
import {
BlockedContentionDetails,
dedupInsights,
FlattenedStmtInsightEvent,
getInsightFromCause,
getInsightsFromProblemsAndCauses,
Expand Down Expand Up @@ -617,13 +618,7 @@ function organizeExecutionInsightsResponseIntoTxns(

txnByExecID.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) => {
Expand Down
15 changes: 14 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/insights/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,9 @@ export function mergeTxnContentionAndStmtInsights(
...txn,
contention: existingContentionEvent.contention,
startTime: existingContentionEvent.startTime,
insights: txn.insights.concat(existingContentionEvent.insights),
insights: dedupInsights(
txn.insights.concat(existingContentionEvent.insights),
),
};
return; // Continue
}
Expand Down Expand Up @@ -490,3 +492,14 @@ export function getTxnInsightRecommendations(

return recs;
}

export function dedupInsights(insights: Insight[]): Insight[] {
// De-duplicate top-level txn insights.
const insightsSeen = new Set<string>();
return insights.reduce((deduped, i) => {
if (insightsSeen.has(i.name)) return deduped;
insightsSeen.add(i.name);
deduped.push(i);
return deduped;
}, []);
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ export const TransactionInsightDetailsOverviewTab: React.FC<Props> = ({

const insightQueries =
insightDetails?.queries?.join("") || "Insight not found.";
const insightsColumns = makeInsightsColumns(isCockroachCloud);
const insightsColumns = makeInsightsColumns(
isCockroachCloud,
insightDetails.queries?.length > 1,
true,
);

const blockingExecutions: ContentionEvent[] =
insightDetails?.blockingContentionDetails?.map(x => {
Expand Down
89 changes: 51 additions & 38 deletions pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { Anchor } from "../anchor";
import { Link } from "react-router-dom";
import { performanceTuningRecipes } from "../util";
import { InsightRecommendation, insightType } from "../insights";
import { limitText } from "src/util";

const cx = classNames.bind(styles);

Expand All @@ -45,7 +44,7 @@ type InsightsTableTitleType = {
};

export const insightsTableTitles: InsightsTableTitleType = {
query: () => <span>insightColumnLabels.query</span>,
query: () => <span>{insightColumnLabels.query}</span>,
insights: () => {
return (
<Tooltip
Expand Down Expand Up @@ -79,7 +78,6 @@ function typeCell(value: string): React.ReactElement {

function descriptionCell(
insightRec: InsightRecommendation,
disableStmtLink?: boolean,
): React.ReactElement {
const clusterSettingsLink = (
<>
Expand All @@ -90,38 +88,12 @@ function descriptionCell(
{"."}
</>
);
const summary = computeOrUseStmtSummary(
insightRec.execution?.statement,
insightRec.execution?.summary,
);
switch (insightRec.type) {
case "CreateIndex":
case "ReplaceIndex":
case "AlterIndex":
return (
<>
<div className={cx("description-item")}>
<span className={cx("label-bold")}>Statement Fingerprint: </span>{" "}
{disableStmtLink && (
<div className={cx("inline")}>
<Tooltip
placement="bottom"
content={insightRec.execution.statement}
>
{summary}
</Tooltip>
</div>
)}
{!disableStmtLink && (
<StatementLink
statementFingerprintID={insightRec.execution.fingerprintID}
statement={insightRec.execution.statement}
statementSummary={insightRec.execution.summary}
implicitTxn={insightRec.execution.implicit}
className={"inline"}
/>
)}
</div>
<div className={cx("description-item")}>
<span className={cx("label-bold")}>Recommendation: </span>{" "}
{insightRec.query}
Expand Down Expand Up @@ -284,8 +256,50 @@ function actionCell(
return <></>;
}

const isIndexRec = (rec: InsightRecommendation) => {
switch (rec.type) {
case "AlterIndex":
case "CreateIndex":
case "DropIndex":
return true;
default:
return false;
}
};

const StatementExecution = ({
rec,
disableLink,
}: {
rec: InsightRecommendation;
disableLink: boolean;
}) => (
<div className={cx("description-item")}>
<span className={cx("label-bold")}>Statement: </span>{" "}
{disableLink ? (
<div className={cx("inline")}>
<Tooltip placement="bottom" content={rec.execution.statement}>
{computeOrUseStmtSummary(
rec.execution?.statement,
rec.execution?.summary,
)}
</Tooltip>
</div>
) : (
<StatementLink
statementFingerprintID={rec.execution.fingerprintID}
statement={rec.execution.statement}
statementSummary={rec.execution.summary}
implicitTxn={rec.execution.implicit}
className="inline"
/>
)}
</div>
);

export function makeInsightsColumns(
hideAction: boolean,
showQuery?: boolean,
disableStmtLink?: boolean,
): ColumnDescriptor<InsightRecommendation>[] {
return [
Expand All @@ -298,17 +312,16 @@ export function makeInsightsColumns(
{
name: "details",
title: insightsTableTitles.details(),
cell: (item: InsightRecommendation) =>
descriptionCell(item, disableStmtLink),
cell: (item: InsightRecommendation) => (
<>
{(showQuery || isIndexRec(item)) && (
<StatementExecution rec={item} disableLink={disableStmtLink} />
)}
{descriptionCell(item)}
</>
),
sort: (item: InsightRecommendation) => item.type,
},
{
name: "query",
title: insightsTableTitles.query(),
cell: (item: InsightRecommendation) =>
limitText(item.execution?.statement, 100) ?? "N/A",
sort: (item: InsightRecommendation) => item.execution?.statement,
},
{
name: "action",
title: insightsTableTitles.actions(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ export function Insights({
onChangeSortSetting,
}: InsightsProps): React.ReactElement {
const hideAction = useContext(CockroachCloudContext) && database?.length == 0;
const insightsColumns = makeInsightsColumns(hideAction, true);
const insightsColumns = makeInsightsColumns(hideAction, false, true);
const data = formatIdxRecommendations(
idxRecommendations,
database,
Expand Down

0 comments on commit 9296682

Please sign in to comment.