Skip to content

Commit

Permalink
ui: sql activity pages depend on node info
Browse files Browse the repository at this point in the history
Fixes #85249.

Previously, the statements, transactions, and transaction details pages
had an implicit dependency on the `state.adminUI.nodes` list in the
redux store, and without that list, the Regions/Nodes columns and
filters would be incorrectly hidden for multiregion clusters, unless the
user happened to have already browsed through a page (such as statement
details) that loaded the list.

(This bug was affecting dedicated cloud clusters only. Self-hosted
clusters were working properly because their global mechanism to sync
alert data happened to [refresh the node list][1].)

This change makes the dependency explicit and refreshes the list of
nodes on entering those three pages.

[1]: https://github.com/cockroachdb/cockroach/blob/6a25c6f4b985ddcd08ed51772f9e8a2645aa4582/pkg/ui/workspaces/db-console/src/redux/alerts.ts#L678-L682

Release note (bug fix): The statements, transactions, and transaction
details pages in the admin console now properly show the Regions/Nodes
columns and filters for multiregion clusters.
  • Loading branch information
matthewtodd committed Sep 23, 2022
1 parent 97fbd44 commit 4fe7d93
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
refreshStatementDiagnosticsRequests: noop,
refreshStatements: noop,
refreshUserSQLRoles: noop,
refreshNodes: noop,
resetSQLStats: noop,
onTimeScaleChange: noop,
onActivateStatementDiagnostics: noop,
Expand Down
15 changes: 11 additions & 4 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ const sortableTableCx = classNames.bind(sortableTableStyles);
export interface StatementsPageDispatchProps {
refreshStatements: (req: StatementsRequest) => void;
refreshStatementDiagnosticsRequests: () => void;
refreshNodes: () => void;
refreshUserSQLRoles: () => void;
resetSQLStats: (req: StatementsRequest) => void;
dismissAlertMessage: () => void;
Expand Down Expand Up @@ -343,8 +344,11 @@ export class StatementsPage extends React.Component<
}

this.props.refreshUserSQLRoles();
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 @@ -382,8 +386,11 @@ export class StatementsPage extends React.Component<

componentDidUpdate = (): void => {
this.updateQueryParams();
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { actions as statementDiagnosticsActions } from "src/store/statementDiagn
import { actions as analyticsActions } from "src/store/analytics";
import { actions as localStorageActions } from "src/store/localStorage";
import { actions as sqlStatsActions } from "src/store/sqlStats";
import { actions as nodesActions } from "../store/nodes";
import {
StatementsPageDispatchProps,
StatementsPageStateProps,
Expand Down Expand Up @@ -119,6 +120,7 @@ export const ConnectedStatementsPage = withRouter(
},
refreshStatementDiagnosticsRequests: () =>
dispatch(statementDiagnosticsActions.refresh()),
refreshNodes: () => dispatch(nodesActions.refresh()),
refreshUserSQLRoles: () =>
dispatch(uiConfigActions.refreshUserSQLRoles()),
resetSQLStats: (req: StatementsRequest) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export interface TransactionDetailsStateProps {

export interface TransactionDetailsDispatchProps {
refreshData: (req?: StatementsRequest) => void;
refreshNodes: () => void;
refreshUserSQLRoles: () => void;
onTimeScaleChange: (ts: TimeScale) => void;
}
Expand Down Expand Up @@ -195,10 +196,16 @@ export class TransactionDetails extends React.Component<
componentDidMount(): void {
this.refreshData("");
this.props.refreshUserSQLRoles();
if (!this.props.isTenant) {
this.props.refreshNodes();
}
}

componentDidUpdate(prevProps: TransactionDetailsProps): void {
this.getTransactionStateInfo(prevProps.transactionFingerprintId);
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 @@ -14,6 +14,7 @@ import { RouteComponentProps, withRouter } from "react-router-dom";
import { Dispatch } from "redux";

import { AppState, uiConfigActions } from "src/store";
import { actions as nodesActions } from "../store/nodes";
import { actions as sqlStatsActions } from "src/store/sqlStats";
import {
TransactionDetails,
Expand Down Expand Up @@ -89,6 +90,7 @@ const mapDispatchToProps = (
): TransactionDetailsDispatchProps => ({
refreshData: (req?: StatementsRequest) =>
dispatch(sqlStatsActions.refresh(req)),
refreshNodes: () => dispatch(nodesActions.refresh()),
refreshUserSQLRoles: () => dispatch(uiConfigActions.refreshUserSQLRoles()),
onTimeScaleChange: (ts: TimeScale) => {
dispatch(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ storiesOf("Transactions Page", module)
onFilterChange={noop}
onSortingChange={noop}
refreshData={noop}
refreshNodes={noop}
resetSQLStats={noop}
search={""}
sortSetting={sortSetting}
Expand All @@ -63,6 +64,7 @@ storiesOf("Transactions Page", module)
onFilterChange={noop}
onSortingChange={noop}
refreshData={noop}
refreshNodes={noop}
resetSQLStats={noop}
search={""}
sortSetting={sortSetting}
Expand All @@ -89,6 +91,7 @@ storiesOf("Transactions Page", module)
onFilterChange={noop}
onSortingChange={noop}
refreshData={noop}
refreshNodes={noop}
resetSQLStats={noop}
search={""}
sortSetting={sortSetting}
Expand All @@ -108,6 +111,7 @@ storiesOf("Transactions Page", module)
onFilterChange={noop}
onSortingChange={noop}
refreshData={noop}
refreshNodes={noop}
resetSQLStats={noop}
search={""}
sortSetting={sortSetting}
Expand All @@ -134,6 +138,7 @@ storiesOf("Transactions Page", module)
onFilterChange={noop}
onSortingChange={noop}
refreshData={noop}
refreshNodes={noop}
resetSQLStats={noop}
search={""}
sortSetting={sortSetting}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export interface TransactionsPageStateProps {

export interface TransactionsPageDispatchProps {
refreshData: (req: StatementsRequest) => void;
refreshNodes: () => void;
resetSQLStats: (req: StatementsRequest) => void;
onTimeScaleChange?: (ts: TimeScale) => void;
onColumnsChange?: (selectedColumns: string[]) => void;
Expand Down Expand Up @@ -234,6 +235,10 @@ export class TransactionsPage extends React.Component<
Math.max(0, nextRefresh.diff(now, "milliseconds")),
);
}

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

componentWillUnmount(): void {
Expand Down Expand Up @@ -274,6 +279,9 @@ export class TransactionsPage extends React.Component<

componentDidUpdate(): void {
this.updateQueryParams();
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 @@ -13,6 +13,7 @@ import { RouteComponentProps, withRouter } from "react-router-dom";
import { Dispatch } from "redux";

import { AppState } from "src/store";
import { actions as nodesActions } from "src/store/nodes";
import { actions as sqlStatsActions } from "src/store/sqlStats";
import {
TransactionsPageStateProps,
Expand Down Expand Up @@ -87,6 +88,7 @@ export const TransactionsPageConnected = withRouter(
fingerprintsPageProps: {
refreshData: (req: StatementsRequest) =>
dispatch(sqlStatsActions.refresh(req)),
refreshNodes: () => dispatch(nodesActions.refresh()),
resetSQLStats: (req: StatementsRequest) =>
dispatch(sqlStatsActions.reset(req)),
onTimeScaleChange: (ts: TimeScale) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { createSelector } from "reselect";
import { RouteComponentProps, withRouter } from "react-router-dom";
import * as protos from "src/js/protos";
import {
refreshNodes,
refreshStatementDiagnosticsRequests,
refreshStatements,
refreshUserSQLRoles,
Expand Down Expand Up @@ -287,6 +288,7 @@ const fingerprintsPageActions = {
refreshStatements,
onTimeScaleChange: setGlobalTimeScaleAction,
refreshStatementDiagnosticsRequests,
refreshNodes,
refreshUserSQLRoles,
resetSQLStats: resetSQLStatsAction,
dismissAlertMessage: () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
import { connect } from "react-redux";
import { createSelector } from "reselect";
import { RouteComponentProps, withRouter } from "react-router-dom";
import { refreshStatements, refreshUserSQLRoles } from "src/redux/apiReducers";
import {
refreshNodes,
refreshStatements,
refreshUserSQLRoles,
} from "src/redux/apiReducers";
import { AdminUIState } from "src/redux/state";
import { txnFingerprintIdAttr } from "src/util/constants";
import { getMatchParamByName } from "src/util/query";
Expand Down Expand Up @@ -80,6 +84,7 @@ export default withRouter(
},
{
refreshData: refreshStatements,
refreshNodes,
refreshUserSQLRoles,
onTimeScaleChange: setGlobalTimeScaleAction,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import { connect } from "react-redux";
import { createSelector } from "reselect";
import { RouteComponentProps, withRouter } from "react-router-dom";
import { refreshStatements } from "src/redux/apiReducers";
import { refreshNodes, refreshStatements } from "src/redux/apiReducers";
import { resetSQLStatsAction } from "src/redux/sqlStats";
import { CachedDataReducerState } from "src/redux/cachedDataReducer";
import { AdminUIState } from "src/redux/state";
Expand Down Expand Up @@ -95,6 +95,7 @@ export const transactionColumnsLocalSetting = new LocalSetting(

const fingerprintsPageActions = {
refreshData: refreshStatements,
refreshNodes,
resetSQLStats: resetSQLStatsAction,
onTimeScaleChange: setGlobalTimeScaleAction,
// We use `null` when the value was never set and it will show all columns.
Expand Down

0 comments on commit 4fe7d93

Please sign in to comment.