Skip to content

Commit

Permalink
Merge #88461
Browse files Browse the repository at this point in the history
88461: ui: sql activity pages depend on node info r=matthewtodd a=matthewtodd

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

| Before | After |
|--|--|
|<img width="1536" alt="Screen Shot 2022-09-22 at 10 42 33 AM" src="https://user-images.githubusercontent.com/5261/191789436-e2276010-5eeb-4fc6-8723-0e0581b7ac44.png">|<img width="1536" alt="Screen Shot 2022-09-22 at 10 41 12 AM" src="https://user-images.githubusercontent.com/5261/191789502-07461bea-ed09-4c85-a878-68edb0ffe7e7.png">|
|<img width="1536" alt="Screen Shot 2022-09-22 at 10 42 42 AM" src="https://user-images.githubusercontent.com/5261/191789576-ed08278c-3cb7-45ff-a719-dd5385f58e66.png">|<img width="1536" alt="Screen Shot 2022-09-22 at 10 41 22 AM" src="https://user-images.githubusercontent.com/5261/191789618-71433f0b-fbd1-4b48-bb86-41e12c869745.png">|
|<img width="1536" alt="Screen Shot 2022-09-22 at 10 43 00 AM" src="https://user-images.githubusercontent.com/5261/191789657-dd3cadc7-2757-417f-94b8-5a31ce760bde.png">|<img width="1536" alt="Screen Shot 2022-09-22 at 10 41 38 AM" src="https://user-images.githubusercontent.com/5261/191789688-0c4da2b1-0018-450c-931d-c24b70290425.png">|

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.

Co-authored-by: Matthew Todd <[email protected]>
  • Loading branch information
craig[bot] and matthewtodd committed Sep 23, 2022
2 parents 4f3bb99 + 4fe7d93 commit 975701e
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 975701e

Please sign in to comment.