Skip to content

Commit

Permalink
Previously, the decommissioned node lists considered node status entries
Browse files Browse the repository at this point in the history 
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.

lint fix

fix test
  • Loading branch information
Zach Lite committed May 2, 2022
1 parent c6c29b8 commit bb7cf90
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 47 deletions.
3 changes: 3 additions & 0 deletions pkg/ui/src/redux/nodes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,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
52 changes: 35 additions & 17 deletions pkg/ui/src/views/cluster/containers/nodesOverview/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ import { FixLong } from "src/util/fixLong";
import { getNodeLocalityTiers } from "src/util/localities";
import { LocalityTier } from "src/redux/localities";

import { cockroach } from "src/js/protos";
import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus;

import TableSection from "./tableSection";
import "./nodes.styl";

Expand Down Expand Up @@ -166,13 +169,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 @@ -417,14 +431,14 @@ class DecommissionedNodeList extends React.Component<DecommissionedNodeListProps
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 @@ -579,11 +593,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 @@ -593,20 +604,27 @@ export const decommissionedNodesTableDataSelector = createSelector(
return 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)
const data = _.chain(decommissionedNodes)
.orderBy(
[(ns: INodeStatus) => getDecommissionedTime(ns.desc.node_id)],
[(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 pkg/ui/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 { cockroach } from "src/js/protos";
import { livenessByNodeIDSelector, LivenessStatus } from "src/redux/nodes";

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 @@ -353,6 +354,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 @@ -361,6 +363,7 @@ describe("Nodes Overview page", () => {
{
node_id: 7,
expiration: { wall_time: Long.fromNumber(Date.now()) },
membership: MembershipStatus.DECOMMISSIONED,
},
],
statuses: {
Expand Down Expand Up @@ -409,7 +412,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 @@ -425,12 +427,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
44 changes: 18 additions & 26 deletions pkg/ui/src/views/reports/containers/nodeHistory/decommissionedNodeHistory.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@
import * as React from "react";
import { Helmet } from "react-helmet";
import { connect } from "react-redux";
import { Link, withRouter } from "react-router-dom";
import { 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 { LongToMoment } from "src/util/convert";
import { SortSetting } from "src/views/shared/components/sortabletable";
import { LocalSetting } from "src/redux/localsettings";
Expand All @@ -28,6 +27,9 @@ import { Text } from "src/components";
import { ColumnsConfig, Table } from "@cockroachlabs/cluster-ui";
import { createSelector } from "reselect";

import { cockroach } from "src/js/protos";
import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus;

const decommissionedNodesSortSetting = new LocalSetting<
AdminUIState,
SortSetting
Expand All @@ -36,7 +38,6 @@ const decommissionedNodesSortSetting = new LocalSetting<
interface DecommissionedNodeStatusRow {
key: string;
nodeId: number;
address: string;
decommissionedDate: Moment;
}

Expand Down Expand Up @@ -80,22 +81,12 @@ export class DecommissionedNodeHistory extends React.Component<DecommissionedNod
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 @@ -130,11 +121,8 @@ export class DecommissionedNodeHistory extends React.Component<DecommissionedNod
}

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 @@ -143,18 +131,22 @@ const decommissionedNodesTableData = createSelector(
const deadTime = liveness.expiration.wall_time;
return LongToMoment(deadTime);
};

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

0 comments on commit bb7cf90

Please sign in to comment.