From 9b123283c816ed0f99a0b58c95d35ed6b021dec9 Mon Sep 17 00:00:00 2001 From: Xin Hao Zhang Date: Wed, 9 Oct 2024 18:24:34 -0400 Subject: [PATCH 1/2] cluster-ui: update nodes label for single regions For single region clusters instead of grouping nodes by region we'll just display the list of nodes, with a max of 4 being displayed in a cell at a time. Overflow will be shown in a tooltip on hover. Epic: CRDB-37558 Release note: None --- .../components/nodesList.module.scss | 10 +++++ .../regionNodesLabel/components/nodesList.tsx | 35 ++++++++++++++++ .../regionLabel.module.scss} | 0 .../components/regionLabel.tsx | 41 ++++++++++++++++++ .../regionNodesLabel/regionNodesLabel.tsx | 42 +++++++++---------- .../src/databaseDetailsV2/tablesView.tsx | 10 +---- .../cluster-ui/src/databasesV2/index.tsx | 12 ++---- 7 files changed, 109 insertions(+), 41 deletions(-) create mode 100644 pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.module.scss create mode 100644 pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.tsx rename pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/{regionNodesLabel.module.scss => components/regionLabel.module.scss} (100%) create mode 100644 pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/regionLabel.tsx diff --git a/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.module.scss b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.module.scss new file mode 100644 index 000000000000..192596b80df6 --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.module.scss @@ -0,0 +1,10 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +@import "src/core/index.module.scss"; + +.label-body { + background-color: $colors--neutral-3; +} diff --git a/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.tsx b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.tsx new file mode 100644 index 000000000000..2d9a4f10f057 --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.tsx @@ -0,0 +1,35 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +import { Tag } from "antd"; +import React from "react"; + +import { Tooltip } from "src/components/tooltip"; +import { NodeID } from "src/types/clusterTypes"; + +import styles from "./nodesList.module.scss"; + +type Props = { + nodes: NodeID[]; +}; + +export const NodesList: React.FC = ({ nodes }) => { + const displayedNodes = nodes.slice(0, 4); + const hiddenNodes = nodes?.length > 4 ? nodes.slice(4) : []; + return ( +
+ {displayedNodes.map(nid => ( + + N{nid} + + ))} + {hiddenNodes?.length > 0 && ( + `n${nid}`).join(", ")}> + +{hiddenNodes.length} + + )} +
+ ); +}; diff --git a/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/regionNodesLabel.module.scss b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/regionLabel.module.scss similarity index 100% rename from pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/regionNodesLabel.module.scss rename to pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/regionLabel.module.scss diff --git a/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/regionLabel.tsx b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/regionLabel.tsx new file mode 100644 index 000000000000..94bed1d26d5e --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/regionLabel.tsx @@ -0,0 +1,41 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +import { Tooltip, Badge, Typography } from "antd"; +import React from "react"; + +import { NodeID, Region } from "src/types/clusterTypes"; + +import styles from "./regionLabel.module.scss"; + +const { Text } = Typography; + +type Props = { + nodes: NodeID[]; + region: Region; + showCode?: boolean; +}; + +// TODO(xinhaoz): We may also be unable to show a flag for regions in SH. +export const RegionLabel: React.FC = ({ + nodes = [], + region, + // TODO (xinhaoz): Investigate if we can determine labels for regions in SH. + showCode = false, +}) => { + return ( +
+ "n" + nid).join(", ")}> +
+ {region.label || "Unknown Region"} + {showCode && ({region.code})} +
+ +
+
+ +
+ ); +}; diff --git a/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/regionNodesLabel.tsx b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/regionNodesLabel.tsx index cd6d2f156e13..14647833b108 100644 --- a/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/regionNodesLabel.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/regionNodesLabel.tsx @@ -3,39 +3,35 @@ // Use of this software is governed by the CockroachDB Software License // included in the /LICENSE file. -import { Tooltip, Badge, Typography } from "antd"; import React from "react"; -import { NodeID, Region } from "src/types/clusterTypes"; +import { NodeID } from "src/types/clusterTypes"; -import styles from "./regionNodesLabel.module.scss"; - -const { Text } = Typography; +import { NodesList } from "./components/nodesList"; +import { RegionLabel } from "./components/regionLabel"; type RegionNodesLabelProps = { - nodes: NodeID[]; - region: Region; - showCode?: boolean; + nodesByRegion: Record; }; -// TODO(xinhaoz): We may also be unable to show a flag for regions in SH. export const RegionNodesLabel: React.FC = ({ - nodes = [], - region, - // TODO (xinhaoz): Investigate if we can determine labels for regions in SH. - showCode = false, + nodesByRegion = {}, }) => { + if (Object.keys(nodesByRegion).length === 1) { + return ; + } return ( -
- "n" + nid).join(", ")}> -
- {region.label || "Unknown Region"} - {showCode && ({region.code})} -
- -
-
- +
+ {Object.entries(nodesByRegion).map(([region, nodes]) => ( + + ))}
); }; diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/tablesView.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/tablesView.tsx index d25fe18c8307..854ec0f42daf 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/tablesView.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/tablesView.tsx @@ -104,15 +104,7 @@ const COLUMNS: (TableColumnProps & { sortKey?: TableSortOption })[] = ), width: "20%", render: (t: TableRow) => ( -
- {Object.entries(t.nodesByRegion ?? {}).map(([region, nodes]) => ( - - ))} -
+ ), }, { diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesV2/index.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesV2/index.tsx index 11a9d014ecff..bfc3486f232c 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesV2/index.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesV2/index.tsx @@ -32,6 +32,8 @@ import useTable, { TableParams } from "src/sharedFromCloud/useTable"; import { StoreID } from "src/types/clusterTypes"; import { Bytes } from "src/util"; +import { getNodesByRegionString } from "../databases"; + import { DatabaseColName } from "./constants"; import { DatabaseRow } from "./databaseTypes"; import { rawDatabaseMetadataToDatabaseRows } from "./utils"; @@ -89,15 +91,7 @@ const COLUMNS: (TableColumnProps & { ), render: (db: DatabaseRow) => ( -
- {Object.entries(db.nodesByRegion?.data).map(([region, nodes]) => ( - - ))} -
+ ), }, From 42d1d7af816d2ad62e4d02e9219d613675ad7c5c Mon Sep 17 00:00:00 2001 From: Xin Hao Zhang Date: Thu, 10 Oct 2024 16:26:53 -0400 Subject: [PATCH 2/2] ui: fix store ids to node regions mapping This commit fixes the store ids to node regions mapping for the new db pages. Previously, we mapped each store id to its distinct entry in the nodesByRegionsMap which did not consider that the set of store ids can be bigger than the set of node ids. This lead to duplicate node entries in the map. Epic: CRDB-37558 Release note: None --- .../regionNodesLabel/components/nodesList.tsx | 4 +- .../src/databaseDetailsV2/utils.tsx | 17 +++---- .../cluster-ui/src/databasesV2/index.tsx | 2 - .../cluster-ui/src/databasesV2/utils.ts | 17 +++---- .../cluster-ui/src/util/nodeUtils.spec.ts | 46 +++++++++++++++++++ .../cluster-ui/src/util/nodeUtils.ts | 36 +++++++++++++++ 6 files changed, 96 insertions(+), 26 deletions(-) create mode 100644 pkg/ui/workspaces/cluster-ui/src/util/nodeUtils.spec.ts create mode 100644 pkg/ui/workspaces/cluster-ui/src/util/nodeUtils.ts diff --git a/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.tsx b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.tsx index 2d9a4f10f057..87098a88ef25 100644 --- a/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.tsx @@ -15,9 +15,9 @@ type Props = { nodes: NodeID[]; }; -export const NodesList: React.FC = ({ nodes }) => { +export const NodesList: React.FC = ({ nodes = [] }) => { const displayedNodes = nodes.slice(0, 4); - const hiddenNodes = nodes?.length > 4 ? nodes.slice(4) : []; + const hiddenNodes = nodes.length > 4 ? nodes.slice(4) : []; return (
{displayedNodes.map(nid => ( diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/utils.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/utils.tsx index 5f65320d32a0..6aa92998738a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/utils.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/utils.tsx @@ -5,6 +5,7 @@ import { TableMetadata } from "src/api/databases/getTableMetadataApi"; import { NodeID, StoreID } from "src/types/clusterTypes"; +import { mapStoreIDsToNodeRegions } from "src/util/nodeUtils"; import { TableRow } from "./types"; @@ -17,17 +18,11 @@ export const tableMetadataToRows = ( }, ): TableRow[] => { return tables.map(table => { - const nodesByRegion: Record = {}; - if (!nodesInfo.isLoading) { - table.storeIds?.forEach(storeID => { - const nodeID = nodesInfo.storeIDToNodeID[storeID as StoreID]; - const region = nodesInfo.nodeIDToRegion[nodeID]; - if (!nodesByRegion[region]) { - nodesByRegion[region] = []; - } - nodesByRegion[region].push(nodeID); - }); - } + const nodesByRegion = mapStoreIDsToNodeRegions( + table.storeIds, + nodesInfo?.nodeIDToRegion, + nodesInfo?.storeIDToNodeID, + ); return { ...table, nodesByRegion: nodesByRegion, diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesV2/index.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesV2/index.tsx index bfc3486f232c..bbf5bfdb6e12 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesV2/index.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesV2/index.tsx @@ -32,8 +32,6 @@ import useTable, { TableParams } from "src/sharedFromCloud/useTable"; import { StoreID } from "src/types/clusterTypes"; import { Bytes } from "src/util"; -import { getNodesByRegionString } from "../databases"; - import { DatabaseColName } from "./constants"; import { DatabaseRow } from "./databaseTypes"; import { rawDatabaseMetadataToDatabaseRows } from "./utils"; diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesV2/utils.ts b/pkg/ui/workspaces/cluster-ui/src/databasesV2/utils.ts index 84d91561087d..ec3caa6f6474 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesV2/utils.ts +++ b/pkg/ui/workspaces/cluster-ui/src/databasesV2/utils.ts @@ -5,6 +5,7 @@ import { DatabaseMetadata } from "src/api/databases/getDatabaseMetadataApi"; import { NodeID, StoreID } from "src/types/clusterTypes"; +import { mapStoreIDsToNodeRegions } from "src/util/nodeUtils"; import { DatabaseRow } from "./databaseTypes"; @@ -17,17 +18,11 @@ export const rawDatabaseMetadataToDatabaseRows = ( }, ): DatabaseRow[] => { return raw.map((db: DatabaseMetadata): DatabaseRow => { - const nodesByRegion: Record = {}; - if (!nodesInfo.isLoading) { - db.storeIds?.forEach(storeID => { - const nodeID = nodesInfo.storeIDToNodeID[storeID as StoreID]; - const region = nodesInfo.nodeIDToRegion[nodeID]; - if (!nodesByRegion[region]) { - nodesByRegion[region] = []; - } - nodesByRegion[region].push(nodeID); - }); - } + const nodesByRegion = mapStoreIDsToNodeRegions( + db.storeIds, + nodesInfo?.nodeIDToRegion, + nodesInfo?.storeIDToNodeID, + ); return { name: db.dbName, id: db.dbId, diff --git a/pkg/ui/workspaces/cluster-ui/src/util/nodeUtils.spec.ts b/pkg/ui/workspaces/cluster-ui/src/util/nodeUtils.spec.ts new file mode 100644 index 000000000000..e46166285edf --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/util/nodeUtils.spec.ts @@ -0,0 +1,46 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +import { StoreID } from "src/types/clusterTypes"; + +import { mapStoreIDsToNodeRegions } from "./nodeUtils"; + +describe("nodeUtils", () => { + describe("mapStoreIDsToNodeRegions", () => { + it("should return a mapping of regions to the nodes that are present in that region based on the provided storeIDs", () => { + const stores = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] as StoreID[]; + const clusterNodeIDToRegion = { + 1: "region1", + 2: "region2", + 3: "region1", + 4: "region2", + 5: "region1", + }; + const clusterStoreIDToNodeID = { + 1: 1, + 2: 2, + 3: 3, + 4: 4, + 5: 5, + 6: 1, + 7: 2, + 8: 3, + 9: 4, + 10: 5, + }; + + const result = mapStoreIDsToNodeRegions( + stores, + clusterNodeIDToRegion, + clusterStoreIDToNodeID, + ); + + expect(result).toEqual({ + region1: [1, 3, 5], + region2: [2, 4], + }); + }); + }); +}); diff --git a/pkg/ui/workspaces/cluster-ui/src/util/nodeUtils.ts b/pkg/ui/workspaces/cluster-ui/src/util/nodeUtils.ts new file mode 100644 index 000000000000..c3b39ca09668 --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/util/nodeUtils.ts @@ -0,0 +1,36 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +import { NodeID, StoreID } from "src/types/clusterTypes"; + +// mapStoreIDsToNodeRegions creates a mapping of regions +// to the nodes that are present in that region based on +// the provided storeIDs. +export const mapStoreIDsToNodeRegions = ( + stores: StoreID[], + clusterNodeIDToRegion: Record = {}, + clusterStoreIDToNodeID: Record = {}, +): Record => { + const nodes = stores.reduce((acc, storeID) => { + acc.add(clusterStoreIDToNodeID[storeID]); + return acc; + }, new Set()); + + const nodesByRegion: Record = {}; + nodes.forEach(nodeID => { + const region = clusterNodeIDToRegion[nodeID]; + if (!nodesByRegion[region]) { + nodesByRegion[region] = []; + } + nodesByRegion[region].push(nodeID); + }); + + // Sort nodes. + Object.keys(nodesByRegion).forEach(region => { + nodesByRegion[region].sort(); + }); + + return nodesByRegion; +};