Skip to content

Commit

Permalink
cluster-ui: tenants use sqlstats-supplied regions
Browse files Browse the repository at this point in the history
Fixes #98056.

As of #95449, the SQL Activity pages in the DB Console can draw regions
information directly from the sqlstats tables, rather than having to
translate node IDs to regions on page load.

Here, we make that switch, but for non-system tenants only, because:

1. The ephemeral nature of serverless nodes made this view-time mapping
   especially problematic in that context. (See further notes in #95449.)

2. The system-tenant views also include KV node IDs in a special
   Regions/Nodes column, which we are unable to recreate given the
   backend storage structure. (Future design work might suggest removing
   these node IDs altogether, for a unified UI.)

Release note: None
  • Loading branch information
matthewtodd committed Mar 14, 2023
1 parent 0205bfe commit 7ea327a
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import React from "react";
import classNames from "classnames/bind";
import { flatMap } from "lodash";
import {
ISortedTablePagination,
SortedTable,
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ export class SessionDetails extends React.Component<SessionDetailsProps> {
terminateQueryRef: React.RefObject<TerminateQueryModalRef>;

componentDidMount(): void {
this.props.refreshNodes();
if (!this.props.isTenant) {
this.props.refreshNodes();
this.props.refreshNodesLiveness();
}
this.props.refreshSessions();
Expand Down
9 changes: 1 addition & 8 deletions pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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 =
Expand Down
23 changes: 15 additions & 8 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
}
}
}

Expand Down Expand Up @@ -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();
}
}
};

Expand Down Expand Up @@ -603,6 +607,7 @@ export class StatementsPage extends React.Component<
onDiagnosticsModalOpen,
apps,
databases,
statements,
search,
isTenant,
nodeRegions,
Expand All @@ -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;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -417,7 +416,6 @@ export function populateRegionNodeForStatements(
")",
);
});
stmt.regions = Object.keys(regions).sort();
stmt.regionNodes = regionNodes;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -251,7 +251,10 @@ export class TransactionsPage extends React.Component<
);
}

this.props.refreshNodes();
if (!this.props.isTenant) {
this.props.refreshNodes();
}

this.props.refreshUserSQLRoles();
}

Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
}),
);
Expand Down
Loading

0 comments on commit 7ea327a

Please sign in to comment.