diff --git a/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx index 5b2ab2c28fc9..c365a129397a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx @@ -10,6 +10,7 @@ import React from "react"; import classNames from "classnames/bind"; +import { flatMap } from "lodash"; import { ISortedTablePagination, SortedTable, @@ -462,7 +463,9 @@ export class IndexDetailsPage extends React.Component< .map(n => Number(n)) .sort(); const regions = unique( - nodes.map(node => nodeRegions[node.toString()]), + isTenant + ? flatMap(statements, statement => statement.stats.regions) + : nodes.map(node => nodeRegions[node.toString()]), ).sort(); const filteredStmts = this.filteredStatements(); diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx index 750d97df64e4..a4b82c830437 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx @@ -102,8 +102,8 @@ export class SessionDetails extends React.Component { terminateQueryRef: React.RefObject; componentDidMount(): void { - this.props.refreshNodes(); if (!this.props.isTenant) { + this.props.refreshNodes(); this.props.refreshNodesLiveness(); } this.props.refreshSessions(); diff --git a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx index 80c132182394..5f0bb539cc4e 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx @@ -61,14 +61,7 @@ export function filteredStatementsData( // list if the list is not empty. statement => regions.length == 0 || - (statement.stats.nodes && - containAny( - statement.stats.nodes.map( - node => nodeRegions[node.toString()], - regions, - ), - regions, - )), + statement.stats.regions?.some(region => regions.includes(region)), ) .filter( // The statement must contain at least one value from the selected nodes diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx index 29413afb5570..0688b4d5cf13 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx @@ -316,8 +316,8 @@ export class StatementDetails extends React.Component< ); } this.props.refreshUserSQLRoles(); - this.props.refreshNodes(); if (!this.props.isTenant) { + this.props.refreshNodes(); this.props.refreshNodesLiveness(); if (!this.props.hasViewActivityRedactedRole) { this.props.refreshStatementDiagnosticsRequests(); @@ -335,8 +335,8 @@ export class StatementDetails extends React.Component< this.refreshStatementDetails(); } - this.props.refreshNodes(); if (!this.props.isTenant) { + this.props.refreshNodes(); this.props.refreshNodesLiveness(); if (!this.props.hasViewActivityRedactedRole) { this.props.refreshStatementDiagnosticsRequests(); @@ -571,9 +571,9 @@ export class StatementDetails extends React.Component< (stats.nodes || []).map(node => node.toString()), ).sort(); const regions = unique( - (stats.nodes || []) - .map(node => nodeRegions[node.toString()]) - .filter(r => r), // Remove undefined / unknown regions. + isTenant + ? stats.regions || [] + : nodes.map(node => nodeRegions[node]).filter(r => r), // Remove undefined / unknown regions. ).sort(); const lastExec = diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 10576973d9d5..17c5c8dc081f 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -10,7 +10,7 @@ import React from "react"; import { RouteComponentProps } from "react-router-dom"; -import { isNil, merge } from "lodash"; +import { flatMap, isNil, merge } from "lodash"; import classNames from "classnames/bind"; import { getValidErrorsList, Loading } from "src/loading"; import { Delayed } from "src/delayed"; @@ -352,9 +352,11 @@ export class StatementsPage extends React.Component< this.refreshDatabases(); this.props.refreshUserSQLRoles(); - this.props.refreshNodes(); - if (!this.props.isTenant && !this.props.hasViewActivityRedactedRole) { - this.props.refreshStatementDiagnosticsRequests(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + if (!this.props.hasViewActivityRedactedRole) { + this.props.refreshStatementDiagnosticsRequests(); + } } } @@ -392,9 +394,11 @@ export class StatementsPage extends React.Component< componentDidUpdate = (): void => { this.updateQueryParams(); - this.props.refreshNodes(); - if (!this.props.isTenant && !this.props.hasViewActivityRedactedRole) { - this.props.refreshStatementDiagnosticsRequests(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + if (!this.props.hasViewActivityRedactedRole) { + this.props.refreshStatementDiagnosticsRequests(); + } } }; @@ -603,6 +607,7 @@ export class StatementsPage extends React.Component< onDiagnosticsModalOpen, apps, databases, + statements, search, isTenant, nodeRegions, @@ -614,7 +619,9 @@ export class StatementsPage extends React.Component< .sort(); const regions = unique( - nodes.map(node => nodeRegions[node.toString()]), + isTenant + ? flatMap(statements, statement => statement.stats.regions) + : nodes.map(node => nodeRegions[node.toString()]), ).sort(); const { filters, activeFilters } = this.state; diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.spec.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.spec.tsx deleted file mode 100644 index e4e9de24803c..000000000000 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.spec.tsx +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2022 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -import { assert } from "chai"; -import Long from "long"; -import { - AggregateStatistics, - populateRegionNodeForStatements, -} from "./statementsTable"; - -describe("populateRegionNodeForStatements", () => { - function statementWithNodeIDs(...nodeIDs: number[]): AggregateStatistics { - return { - aggregatedFingerprintID: "", - aggregatedFingerprintHexID: "", - label: "", - summary: "", - aggregatedTs: 0, - implicitTxn: false, - fullScan: false, - database: "", - applicationName: "", - stats: { nodes: nodeIDs.map(id => Long.fromInt(id)) }, - }; - } - - it("maps nodes to regions, sorted", () => { - const statement = statementWithNodeIDs(1, 2); - populateRegionNodeForStatements([statement], { - "1": "gcp-us-west1", - "2": "gcp-us-east1", - }); - assert.deepEqual(["gcp-us-east1", "gcp-us-west1"], statement.regions); - }); - - it("handles statements without nodes", () => { - const statement = statementWithNodeIDs(); - populateRegionNodeForStatements([statement], { - "1": "gcp-us-west1", - "2": "gcp-us-east1", - }); - assert.deepEqual(statement.regions, []); - }); - - it("excludes nodes whose region we don't know", () => { - const statement = statementWithNodeIDs(1, 2); - populateRegionNodeForStatements([statement], { - "1": "gcp-us-west1", - }); - assert.deepEqual(statement.regions, ["gcp-us-west1"]); - }); -}); diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx index adb6adbf5e0c..8964473ce5c2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx @@ -73,7 +73,6 @@ export interface AggregateStatistics { diagnosticsReports?: StatementDiagnosticsReport[]; // totalWorkload is the sum of service latency of all statements listed on the table. totalWorkload?: Long; - regions?: string[]; regionNodes?: string[]; } @@ -356,9 +355,9 @@ function makeRegionsColumn( title: statisticsTableTitles.regions(statType), className: cx("statements-table__col-regions"), cell: (stmt: AggregateStatistics) => { - return longListWithTooltip(stmt.regions.sort().join(", "), 50); + return longListWithTooltip(stmt.stats.regions.sort().join(", "), 50); }, - sort: (stmt: AggregateStatistics) => stmt.regions.sort().join(", "), + sort: (stmt: AggregateStatistics) => stmt.stats.regions.sort().join(", "), }; } else { return { @@ -417,7 +416,6 @@ export function populateRegionNodeForStatements( ")", ); }); - stmt.regions = Object.keys(regions).sort(); stmt.regionNodes = regionNodes; }); } diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx index 5c2982033a45..3385d03c8e27 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx @@ -260,7 +260,9 @@ export class TransactionDetails extends React.Component< ); } this.props.refreshUserSQLRoles(); - this.props.refreshNodes(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } componentWillUnmount(): void { @@ -269,7 +271,9 @@ export class TransactionDetails extends React.Component< componentDidUpdate(prevProps: TransactionDetailsProps): void { this.getTransactionStateInfo(prevProps.transactionFingerprintId); - this.props.refreshNodes(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } onChangeSortSetting = (ss: SortSetting): void => { diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactions.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactions.fixture.ts index ec88fe77a466..96f096e82e43 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactions.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactions.fixture.ts @@ -88,6 +88,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(557), nodes: [Long.fromNumber(1), Long.fromNumber(2)], + regions: ["gcp-us-east1"], first_attempt_count: Long.fromInt(557), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -167,6 +168,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(70), nodes: [Long.fromNumber(1), Long.fromNumber(3)], + regions: ["gcp-us-east1", "gcp-us-west1"], first_attempt_count: Long.fromInt(70), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -233,6 +235,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(1), nodes: [Long.fromNumber(1), Long.fromNumber(3)], + regions: ["gcp-us-east1", "gcp-us-west1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -290,6 +293,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(280), nodes: [Long.fromNumber(3), Long.fromNumber(4)], + regions: ["gcp-us-west1", "gcp-europe-west1"], first_attempt_count: Long.fromInt(280), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -391,6 +395,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(1), nodes: [Long.fromNumber(2), Long.fromNumber(4)], + regions: ["gcp-us-east1", "gcp-europe-west1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -442,6 +447,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(1), nodes: [Long.fromNumber(1)], + regions: ["gcp-us-east1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -482,6 +488,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(1), nodes: [Long.fromNumber(3), Long.fromNumber(4)], + regions: ["gcp-us-west1", "gcp-europe-west1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -533,6 +540,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(24), nodes: [Long.fromNumber(2), Long.fromNumber(3)], + regions: ["gcp-us-east1", "gcp-us-west1"], first_attempt_count: Long.fromInt(24), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -622,6 +630,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(141), nodes: [Long.fromNumber(1), Long.fromNumber(2), Long.fromNumber(3)], + regions: ["gcp-us-east1", "gcp-us-west1"], first_attempt_count: Long.fromInt(141), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -740,6 +749,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { Long.fromNumber(3), Long.fromNumber(4), ], + regions: ["gcp-us-east1", "gcp-us-west1", "gcp-europe-west1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx index d5102531cea4..f1495ca0ffdc 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx @@ -36,7 +36,7 @@ import { filterTransactions, } from "./utils"; import Long from "long"; -import { merge } from "lodash"; +import { flatMap, merge } from "lodash"; import { unique, syncHistory } from "src/util"; import { EmptyTransactionsPlaceholder } from "./emptyTransactionsPlaceholder"; import { Loading } from "../loading"; @@ -251,7 +251,10 @@ export class TransactionsPage extends React.Component< ); } - this.props.refreshNodes(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } + this.props.refreshUserSQLRoles(); } @@ -293,7 +296,9 @@ export class TransactionsPage extends React.Component< componentDidUpdate(): void { this.updateQueryParams(); - this.props.refreshNodes(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } onChangeSortSetting = (ss: SortSetting): void => { @@ -425,7 +430,9 @@ export class TransactionsPage extends React.Component< .sort(); const regions = unique( - nodes.map(node => nodeRegions[node.toString()]), + isTenant + ? flatMap(statements, statement => statement.stats.regions) + : nodes.map(node => nodeRegions[node.toString()]), ).sort(); // We apply the search filters and app name filters prior to aggregating across Node IDs @@ -508,7 +515,7 @@ export class TransactionsPage extends React.Component< t => ({ stats_data: t.stats_data, node_id: t.node_id, - regions: generateRegion(t, statements, nodeRegions), + regions: generateRegion(t, statements), regionNodes: generateRegionNode(t, statements, nodeRegions), }), ); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts index 1056c9275dfd..b7a415707c89 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts @@ -293,9 +293,7 @@ SELECT _`, }); describe("generateRegion", () => { - function transactionWithStatementFingerprintIDs( - ...ids: number[] - ): Transaction { + function transaction(...ids: number[]): Transaction { return { stats_data: { statement_fingerprint_ids: ids.map(id => Long.fromInt(id)), @@ -303,35 +301,18 @@ describe("generateRegion", () => { }; } - function statementWithFingerprintAndNodeIDs( - id: number, - ...nodeIDs: number[] - ): Statement { - return { - id: Long.fromInt(id), - stats: { nodes: nodeIDs.map(id => Long.fromInt(id)) }, - }; + function statement(id: number, ...regions: string[]): Statement { + return { id: Long.fromInt(id), stats: { regions } }; } it("gathers up the list of regions for the transaction, sorted", () => { assert.deepEqual( - generateRegion( - transactionWithStatementFingerprintIDs(42), - [statementWithFingerprintAndNodeIDs(42, 1, 2)], - { "1": "gcp-us-west1", "2": "gcp-us-east1" }, - ), - ["gcp-us-east1", "gcp-us-west1"], - ); - }); - - it("skips over nodes with unknown regions", () => { - assert.deepEqual( - generateRegion( - transactionWithStatementFingerprintIDs(42), - [statementWithFingerprintAndNodeIDs(42, 1, 2)], - { "1": "gcp-us-west1" }, - ), - ["gcp-us-west1"], + generateRegion(transaction(42, 43, 44), [ + statement(42, "gcp-us-west1", "gcp-us-east1"), + statement(43, "gcp-us-west1"), + statement(44, "gcp-us-central1"), + ]), + ["gcp-us-central1", "gcp-us-east1", "gcp-us-west1"], ); }); }); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts index 01316df3552f..49459d7e099f 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts @@ -156,6 +156,7 @@ export const searchTransactionsData = ( ); }; +// TODO(todd): Remove unused nodeRegions parameter. export const filterTransactions = ( data: Transaction[], filters: Filters, @@ -210,31 +211,31 @@ export const filterTransactions = ( timeValue === "empty", ) .filter((t: Transaction) => { - // The transaction must contain at least one value of the nodes - // and regions list (if the list is not empty). - if (regions.length == 0 && nodes.length == 0) return true; - // If the cluster is a tenant cluster we don't care - // about nodes. - let foundRegion: boolean = regions.length == 0; - let foundNode: boolean = isTenant || nodes.length == 0; - - getStatementsByFingerprintId( + // The transaction must contain at least one value of the regions list + // (if the list is not empty). + if (regions.length == 0) return true; + + return getStatementsByFingerprintId( t.stats_data.statement_fingerprint_ids, statements, - ).some(stmt => { - stmt.stats.nodes && - stmt.stats.nodes.some(node => { - if (foundRegion || regions.includes(nodeRegions[node.toString()])) { - foundRegion = true; - } - if (foundNode || nodes.includes("n" + node)) { - foundNode = true; - } - if (foundNode && foundRegion) return true; - }); - }); + ).some(stmt => + stmt.stats.regions?.some(region => regions.includes(region)), + ); + }) + .filter((t: Transaction) => { + // The transaction must contain at least one value of the nodes list + // (if the list is not empty). + if (nodes.length == 0) return true; + + // If the cluster is a tenant cluster we don't care about nodes. + if (isTenant) return true; - return foundRegion && foundNode; + return getStatementsByFingerprintId( + t.stats_data.statement_fingerprint_ids, + statements, + ).some(stmt => + stmt.stats.nodes?.some(node => nodes.includes("n" + node)), + ); }); return { @@ -249,32 +250,21 @@ export const filterTransactions = ( * E.g. of one element of the list: `gcp-us-east1` * @param transaction: list of transactions. * @param statements: list of all statements collected. - * @param nodeRegions: object with keys being the node id and the value - * which region it belongs to. */ export const generateRegion = ( transaction: Transaction, statements: Statement[], - nodeRegions: { [p: string]: string }, ): string[] => { const regions: Set = new Set(); - // Get the list of statements that were executed on the transaction. Combine all - // nodes and regions of all the statements to a single list of `region: nodes` - // for the transaction. - // E.g. {"gcp-us-east1" : [1,3,4]} + getStatementsByFingerprintId( transaction.stats_data.statement_fingerprint_ids, statements, ).forEach(stmt => { - stmt.stats.nodes && - stmt.stats.nodes.forEach(n => { - regions.add(nodeRegions[n.toString()]); - }); + stmt.stats.regions?.forEach(region => regions.add(region)); }); - return Array.from(regions) - .filter(r => r) // Remove undefined / unknown regions. - .sort(); + return Array.from(regions).sort(); }; /**