Skip to content

Commit

Permalink
ui: Use liveness info to populate decommissioned node lists
Browse files Browse the repository at this point in the history 
Previously, the decommissioned node lists considered node status entries
to determine decommissioning and decommissioned status. This changed in cockroachdb#56529,
resulting in empty lists. Now, the node's liveness entry is considered
and these lists are correctly populated.

Release note (bug fix): The list of recently decommissioned nodes
and the historical list of decommissioned nodes correctly display
decommissioned nodes.

fix tests

fix pb import and add default arg value
  • Loading branch information
@RajivTS
Zach Lite authored and RajivTS committed Mar 6, 2022
1 parent 152deb1 commit 61e95b4
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 52 deletions.
3 changes: 3 additions & 0 deletions pkg/ui/workspaces/db-console/src/redux/nodes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,9 @@ function isNoConnection(

// nodeDisplayNameByIDSelector provides a unique, human-readable display name
// for each node.

// This function will never be passed decommissioned nodes because
// #56529 removed a node's status entry once it's decommissioned.
export const nodeDisplayNameByIDSelector = createSelector(
nodeStatusesSelector,
livenessStatusByNodeIDSelector,
Expand Down
54 changes: 34 additions & 20 deletions pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ import {
VersionTooltip,
StatusTooltip,
} from "./tooltips";
import { cockroach } from "src/js/protos";
import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus;

const liveNodesSortSetting = new LocalSetting<AdminUIState, SortSetting>(
"nodes/live_sort_setting",
Expand Down Expand Up @@ -169,13 +171,24 @@ export const getLivenessStatusName = (status: LivenessStatus): string => {

const NodeNameColumn: React.FC<{
record: NodeStatusRow | DecommissionedNodeStatusRow;
}> = ({ record }) => {
return (
<Link className="nodes-table__link" to={`/node/${record.nodeId}`}>
shouldLink?: boolean;
}> = ({ record, shouldLink = true }) => {
const columnValue = (
<>
<Text>{record.nodeName}</Text>
<Text textType={TextTypes.BodyStrong}>{` (n${record.nodeId})`}</Text>
</Link>
</>
);

if (shouldLink) {
return (
<Link className="nodes-table__link" to={`/node/${record.nodeId}`}>
{columnValue}
</Link>
);
}

return columnValue;
};

const NodeLocalityColumn: React.FC<{ record: NodeStatusRow }> = ({
Expand Down Expand Up @@ -422,14 +435,14 @@ class DecommissionedNodeList extends React.Component<
key: "nodes",
title: "decommissioned nodes",
render: (_text: string, record: DecommissionedNodeStatusRow) => (
<NodeNameColumn record={record} />
<NodeNameColumn record={record} shouldLink={false} />
),
},
{
key: "decommissionedSince",
title: "decommissioned on",
render: (_text: string, record: DecommissionedNodeStatusRow) =>
record.decommissionedDate.format("LL[ at ]h:mm a"),
record.decommissionedDate.format("LL[ at ]h:mm a UTC"),
},
{
key: "status",
Expand Down Expand Up @@ -584,11 +597,8 @@ export const liveNodesTableDataSelector = createSelector(
);

export const decommissionedNodesTableDataSelector = createSelector(
partitionedStatuses,
nodesSummarySelector,
(statuses, nodesSummary): DecommissionedNodeStatusRow[] => {
const decommissionedStatuses = statuses.decommissioned || [];

(nodesSummary): DecommissionedNodeStatusRow[] => {
const getDecommissionedTime = (nodeId: number) => {
const liveness = nodesSummary.livenessByNodeID[nodeId];
if (!liveness) {
Expand All @@ -598,20 +608,24 @@ export const decommissionedNodesTableDataSelector = createSelector(
return util.LongToMoment(deadTime);
};

const decommissionedNodes = Object.values(
nodesSummary.livenessByNodeID,
).filter(liveness => {
return liveness?.membership === MembershipStatus.DECOMMISSIONED;
});

// DecommissionedNodeList displays 5 most recent nodes.
const data = _.chain(decommissionedStatuses)
.orderBy(
[(ns: INodeStatus) => getDecommissionedTime(ns.desc.node_id)],
["desc"],
)
const data = _.chain(decommissionedNodes)
.orderBy([liveness => getDecommissionedTime(liveness.node_id)], ["desc"])
.take(5)
.map((ns: INodeStatus, idx: number) => {
.map((liveness, idx: number) => {
const { node_id } = liveness;
return {
key: `${idx}`,
nodeId: ns.desc.node_id,
nodeName: ns.desc.address.address_field,
status: nodesSummary.livenessStatusByNodeID[ns.desc.node_id],
decommissionedDate: getDecommissionedTime(ns.desc.node_id),
nodeId: node_id,
nodeName: `${node_id}`,
status: nodesSummary.livenessStatusByNodeID[node_id],
decommissionedDate: getDecommissionedTime(node_id),
};
})
.value();
Expand Down
8 changes: 4 additions & 4 deletions ...i/workspaces/db-console/src/views/cluster/containers/nodesOverview/nodesOverview.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { livenessByNodeIDSelector, LivenessStatus } from "src/redux/nodes";
import { SortSetting } from "@cockroachlabs/cluster-ui";

import NodeLivenessStatus = cockroach.kv.kvserver.liveness.livenesspb.NodeLivenessStatus;
import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus;

describe("Nodes Overview page", () => {
describe("Live <NodeList/> section initial state", () => {
Expand Down Expand Up @@ -365,6 +366,7 @@ describe("Nodes Overview page", () => {
{
node_id: 2,
expiration: { wall_time: Long.fromNumber(Date.now()) },
membership: MembershipStatus.DECOMMISSIONED,
},
{ node_id: 3 },
{ node_id: 4 },
Expand All @@ -373,6 +375,7 @@ describe("Nodes Overview page", () => {
{
node_id: 7,
expiration: { wall_time: Long.fromNumber(Date.now()) },
membership: MembershipStatus.DECOMMISSIONED,
},
],
statuses: {
Expand Down Expand Up @@ -421,7 +424,6 @@ describe("Nodes Overview page", () => {
it("returns node records with 'decommissioned' status only", () => {
const expectedDecommissionedNodeIds = [2, 7];
const records = decommissionedNodesTableDataSelector.resultFunc(
partitionedNodes,
nodeSummary,
);

Expand All @@ -437,12 +439,10 @@ describe("Nodes Overview page", () => {

it("returns correct node name", () => {
const recordsGroupedByRegion = decommissionedNodesTableDataSelector.resultFunc(
partitionedNodes,
nodeSummary,
);
recordsGroupedByRegion.forEach(record => {
const expectedName = `127.0.0.${record.nodeId}:50945`;
assert.equal(record.nodeName, expectedName);
assert.equal(record.nodeName, record.nodeId.toString());
});
});
});
Expand Down
47 changes: 19 additions & 28 deletions ...kspaces/db-console/src/views/reports/containers/nodeHistory/decommissionedNodeHistory.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@
import * as React from "react";
import { Helmet } from "react-helmet";
import { connect } from "react-redux";
import { Link, RouteComponentProps, withRouter } from "react-router-dom";
import { RouteComponentProps, withRouter } from "react-router-dom";
import { Moment } from "moment";
import _ from "lodash";

import { AdminUIState } from "src/redux/state";
import { nodesSummarySelector, partitionedStatuses } from "src/redux/nodes";
import { nodesSummarySelector } from "src/redux/nodes";
import { refreshLiveness, refreshNodes } from "src/redux/apiReducers";
import { INodeStatus } from "src/util/proto";
import { cockroach } from "src/js/protos";
import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus;
import { LocalSetting } from "src/redux/localsettings";

import "./decommissionedNodeHistory.styl";
Expand All @@ -39,7 +40,6 @@ const decommissionedNodesSortSetting = new LocalSetting<
interface DecommissionedNodeStatusRow {
key: string;
nodeId: number;
address: string;
decommissionedDate: Moment;
}

Expand Down Expand Up @@ -85,22 +85,12 @@ export class DecommissionedNodeHistory extends React.Component<
sorter: sortByNodeId,
render: (_text, record) => <Text>{`n${record.nodeId}`}</Text>,
},
{
key: "address",
title: "Address",
sorter: true,
render: (_text, record) => (
<Link to={`/node/${record.nodeId}`}>
<Text>{record.address}</Text>
</Link>
),
},
{
key: "decommissionedOn",
title: "Decommissioned On",
sorter: sortByDecommissioningDate,
render: (_text, record) => {
return record.decommissionedDate.format("LL[ at ]h:mm a");
return record.decommissionedDate.format("LL[ at ]h:mm a UTC");
},
},
];
Expand Down Expand Up @@ -135,11 +125,8 @@ export class DecommissionedNodeHistory extends React.Component<
}

const decommissionedNodesTableData = createSelector(
partitionedStatuses,
nodesSummarySelector,
(statuses, nodesSummary): DecommissionedNodeStatusRow[] => {
const decommissionedStatuses = statuses.decommissioned || [];

(nodesSummary): DecommissionedNodeStatusRow[] => {
const getDecommissionedTime = (nodeId: number) => {
const liveness = nodesSummary.livenessByNodeID[nodeId];
if (!liveness) {
Expand All @@ -149,20 +136,24 @@ const decommissionedNodesTableData = createSelector(
return util.LongToMoment(deadTime);
};

const data = _.chain(decommissionedStatuses)
.orderBy(
[(ns: INodeStatus) => getDecommissionedTime(ns.desc.node_id)],
["desc"],
)
.map((ns: INodeStatus, idx: number) => {
const decommissionedNodes = Object.values(
nodesSummary.livenessByNodeID,
).filter(liveness => {
return liveness?.membership === MembershipStatus.DECOMMISSIONED;
});

const data = _.chain(decommissionedNodes)
.orderBy([liveness => getDecommissionedTime(liveness.node_id)], ["desc"])
.map((liveness, idx: number) => {
const { node_id } = liveness;
return {
key: `${idx}`,
nodeId: ns.desc.node_id,
address: ns.desc.address.address_field,
decommissionedDate: getDecommissionedTime(ns.desc.node_id),
nodeId: node_id,
decommissionedDate: getDecommissionedTime(node_id),
};
})
.value();

return data;
},
);
Expand Down

0 comments on commit 61e95b4

Please sign in to comment.