Merge #132312

132312: cluster-ui: update nodes label for single regions r=xinhaoz a=xinhaoz 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 ---------------------------- 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 Co-authored-by: Xin Hao Zhang <[email protected]>
cockroachdb · Oct 11, 2024 · 34316ac · 34316ac
2 parents 76266cf + 42d1d7a
commit 34316ac
Show file tree

Hide file tree

Showing 11 changed files with 201 additions and 63 deletions.
diff --git a/...ui/workspaces/cluster-ui/src/components/regionNodesLabel/components/nodesList.module.scss b/...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
@@ -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<Props> = ({ nodes = [] }) => {
+  const displayedNodes = nodes.slice(0, 4);
+  const hiddenNodes = nodes.length > 4 ? nodes.slice(4) : [];
+  return (
+    <div>
+      {displayedNodes.map(nid => (
+        <Tag className={styles["label-body"]} key={nid}>
+          N{nid}
+        </Tag>
+      ))}
+      {hiddenNodes?.length > 0 && (
+        <Tooltip title={hiddenNodes.map(nid => `n${nid}`).join(", ")}>
+          +{hiddenNodes.length}
+        </Tooltip>
+      )}
+    </div>
+  );
+};
diff --git a/...onNodesLabel/regionNodesLabel.module.scss → ...sLabel/components/regionLabel.module.scss b/...onNodesLabel/regionNodesLabel.module.scss → ...sLabel/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
@@ -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<Props> = ({
+  nodes = [],
+  region,
+  // TODO (xinhaoz): Investigate if we can determine labels for regions in SH.
+  showCode = false,
+}) => {
+  return (
+    <div className={styles.container}>
+      <Tooltip placement="top" title={nodes.map(nid => "n" + nid).join(", ")}>
+        <div className={styles["label-body"]}>
+          <Text strong>{region.label || "Unknown Region"}</Text>
+          {showCode && <Text>({region.code})</Text>}
+          <div>
+            <Badge count={nodes.length} className={styles.badge} />
+          </div>
+        </div>
+      </Tooltip>
+    </div>
+  );
+};
diff --git 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<string, NodeID[]>;
 };
 
-// TODO(xinhaoz): We may also be unable to show a flag for regions in SH.
 export const RegionNodesLabel: React.FC<RegionNodesLabelProps> = ({
-  nodes = [],
-  region,
-  // TODO (xinhaoz): Investigate if we can determine labels for regions in SH.
-  showCode = false,
+  nodesByRegion = {},
 }) => {
+  if (Object.keys(nodesByRegion).length === 1) {
+    return <NodesList nodes={Object.values(nodesByRegion)[0]} />;
+  }
   return (
-    <div className={styles.container}>
-      <Tooltip placement="top" title={nodes.map(nid => "n" + nid).join(", ")}>
-        <div className={styles["label-body"]}>
-          <Text strong>{region.label || "Unknown Region"}</Text>
-          {showCode && <Text>({region.code})</Text>}
-          <div>
-            <Badge count={nodes.length} className={styles.badge} />
-          </div>
-        </div>
-      </Tooltip>
+    <div>
+      {Object.entries(nodesByRegion).map(([region, nodes]) => (
+        <RegionLabel
+          key={region}
+          region={{
+            code: region,
+            label: region,
+          }}
+          nodes={nodes}
+        />
+      ))}
     </div>
   );
 };
diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/tablesView.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsV2/tablesView.tsx
@@ -100,15 +100,7 @@ const COLUMNS: (TableColumnProps<TableRow> & { sortKey?: TableSortOption })[] =
       ),
       width: "20%",
       render: (t: TableRow) => (
-        <div>
-          {Object.entries(t.nodesByRegion ?? {}).map(([region, nodes]) => (
-            <RegionNodesLabel
-              key={region}
-              nodes={nodes}
-              region={{ label: region, code: region }}
-            />
-          ))}
-        </div>
+        <RegionNodesLabel nodesByRegion={t.nodesByRegion} />
       ),
     },
     {

diff --git 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<string, NodeID[]> = {};
-    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
@@ -95,15 +95,7 @@ const COLUMNS: (TableColumnProps<DatabaseRow> & {
     ),
     render: (db: DatabaseRow) => (
       <Skeleton loading={db.nodesByRegion.isLoading}>
-        <div>
-          {Object.entries(db.nodesByRegion?.data).map(([region, nodes]) => (
-            <RegionNodesLabel
-              key={region}
-              nodes={nodes}
-              region={{ label: region, code: region }}
-            />
-          ))}
-        </div>
+        <RegionNodesLabel nodesByRegion={db.nodesByRegion?.data} />
       </Skeleton>
     ),
   },

diff --git 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<string, NodeID[]> = {};
-    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
@@ -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
@@ -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<NodeID, string> = {},
+  clusterStoreIDToNodeID: Record<StoreID, NodeID> = {},
+): Record<string, NodeID[]> => {
+  const nodes = stores.reduce((acc, storeID) => {
+    acc.add(clusterStoreIDToNodeID[storeID]);
+    return acc;
+  }, new Set<NodeID>());
+
+  const nodesByRegion: Record<string, NodeID[]> = {};
+  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;
+};