Skip to content

Commit

Permalink
ui: remove auto polling from fingerprints pages
Browse files Browse the repository at this point in the history 
Previously, we were automatically fetching new data
on the fingerprints pages every 5m if the time period
selected was of the form 'Past xxxx of data'. We should
not automatically poll because:
1. this is an expensive request, we don't want to
unnecessarily send new ones if the user does not need
to refresh their data.
2. It can be a weird experience to be viewing your table
and have things suddenly update/change, since we don't
communicate that we're fetching new data every 5m.

As part of this change, we move the invalidation of data
depending on the global time scale object to the saga
observing the timescale update in redux, rather than in the
saga that dispatches the update time scale action. This
ensures that the update ts action has been reduced at the
time of invalidation. In the future, we should remove the
`SET_GLOBAL_TIME_SCALE` action as it is just a wrapper
dispatching `SET_SCALE`.

This commit also removes issuing a stats request from the
reset sql 14 stats saga, since invalidating the statements
15 will cause a refresh in the page already.

Epic: none
Part of: cockroachdb#97875

Release note (ui change): New data is not auto fetched
every 5m on the stmt / txn fingerprints pages.
  • Loading branch information
@xinhaoz
xinhaoz committed Apr 17, 2023
1 parent d8f79a6 commit 497898f
Show file tree
Hide file tree
Showing 21 changed files with 272 additions and 213 deletions.
4 changes: 4 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts
View file
Expand Up @@ -246,6 +246,7 @@ const diagnosticsReportsInProgress: IStatementDiagnosticsReport[] = [

const aggregatedTs = Date.parse("Sep 15 2021 01:00:00 GMT") * 1e-3;
const aggregationInterval = 3600; // 1 hour
const lastUpdated = moment("Sep 15 2021 01:30:00 GMT");

const statementsPagePropsFixture: StatementsPageProps = {
history,
Expand Down Expand Up @@ -283,6 +284,9 @@ const statementsPagePropsFixture: StatementsPageProps = {
regions: "",
nodes: "",
},
lastUpdated,
isDataValid: true,
isReqInFlight: false,
// Aggregate key values in these statements will need to change if implementation
// of 'statementKey' in appStats.ts changes.
statements: [
Expand Down
26 changes: 24 additions & 2 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts
View file
Expand Up @@ -25,6 +25,7 @@ import {
} from "src/util";
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
import { RouteComponentProps } from "react-router-dom";
import { Moment } from "moment";

import { AppState } from "src/store";
import { selectDiagnosticsReportsPerStatement } from "../store/statementDiagnostics";
Expand Down Expand Up @@ -93,13 +94,13 @@ export const selectApps = createSelector(sqlStatsSelector, sqlStatsState => {
// in the data.
export const selectDatabases = createSelector(
sqlStatsSelector,
sqlStatsState => {
(sqlStatsState): string[] => {
if (!sqlStatsState.data) {
return [];
}

return Array.from(
new Set(
new Set<string>(
sqlStatsState.data.statements.map(s =>
s.key.key_data.database ? s.key.key_data.database : unset,
),
Expand Down Expand Up @@ -133,6 +134,27 @@ export const selectLastReset = createSelector(sqlStatsSelector, state => {
return formatDate(TimestampToMoment(state.data.last_reset));
});

export const selectStatementsDataValid = createSelector(
sqlStatsSelector,
(state: SQLStatsState): boolean => {
return state.valid;
},
);

export const selectStatementsDataInFlight = createSelector(
sqlStatsSelector,
(state: SQLStatsState): boolean => {
return state.inFlight;
},
);

export const selectStatementsLastUpdated = createSelector(
sqlStatsSelector,
(state: SQLStatsState): Moment => {
return state.lastUpdated;
},
);

export const selectStatements = createSelector(
sqlStatsSelector,
(_: AppState, props: RouteComponentProps) => props,
Expand Down
88 changes: 49 additions & 39 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
View file
Expand Up @@ -10,7 +10,7 @@

import React from "react";
import { RouteComponentProps } from "react-router-dom";
import { isNil, merge } from "lodash";
import { merge } from "lodash";
import classNames from "classnames/bind";
import { getValidErrorsList, Loading } from "src/loading";
import { Delayed } from "src/delayed";
Expand Down Expand Up @@ -96,7 +96,7 @@ export interface StatementsPageDispatchProps {
refreshStatementDiagnosticsRequests: () => void;
refreshNodes: () => void;
refreshUserSQLRoles: () => void;
resetSQLStats: (req: StatementsRequest) => void;
resetSQLStats: () => void;
dismissAlertMessage: () => void;
onActivateStatementDiagnostics: (
statement: string,
Expand All @@ -122,6 +122,9 @@ export interface StatementsPageDispatchProps {

export interface StatementsPageStateProps {
statements: AggregateStatistics[];
isDataValid: boolean;
isReqInFlight: boolean;
lastUpdated: moment.Moment | null;
timeScale: TimeScale;
statementsError: Error | null;
apps: string[];
Expand All @@ -142,7 +145,6 @@ export interface StatementsPageState {
pagination: ISortedTablePagination;
filters?: Filters;
activeFilters?: number;
startRequest?: Date;
}

export type StatementsPageProps = StatementsPageDispatchProps &
Expand Down Expand Up @@ -188,26 +190,18 @@ export class StatementsPage extends React.Component<
StatementsPageState
> {
activateDiagnosticsRef: React.RefObject<ActivateDiagnosticsModalRef>;

constructor(props: StatementsPageProps) {
super(props);
const defaultState = {
pagination: {
pageSize: 20,
current: 1,
},
startRequest: new Date(),
};
const stateFromHistory = this.getStateFromHistory();
this.state = merge(defaultState, stateFromHistory);
this.activateDiagnosticsRef = React.createRef();

// In case the user selected a option not available on this page,
// force a selection of a valid option. This is necessary for the case
// where the value 10/30 min is selected on the Metrics page.
const ts = getValidOption(this.props.timeScale, timeScale1hMinOptions);
if (ts !== this.props.timeScale) {
this.changeTimeScale(ts);
}
}

getStateFromHistory = (): Partial<StatementsPageState> => {
Expand Down Expand Up @@ -266,9 +260,6 @@ export class StatementsPage extends React.Component<
if (this.props.onTimeScaleChange) {
this.props.onTimeScaleChange(ts);
}
this.setState({
startRequest: new Date(),
});
};

resetPagination = (): void => {
Expand All @@ -287,18 +278,24 @@ export class StatementsPage extends React.Component<
this.props.refreshStatements(req);
};
resetSQLStats = (): void => {
const req = statementsRequestFromProps(this.props);
this.props.resetSQLStats(req);
this.setState({
startRequest: new Date(),
});
this.props.resetSQLStats();
};

componentDidMount(): void {
this.setState({
startRequest: new Date(),
});
this.refreshStatements();
// In case the user selected a option not available on this page,
// force a selection of a valid option. This is necessary for the case
// where the value 10/30 min is selected on the Metrics page.
const ts = getValidOption(this.props.timeScale, timeScale1hMinOptions);
if (ts !== this.props.timeScale) {
this.changeTimeScale(ts);
} else if (
!this.props.isDataValid ||
!this.props.lastUpdated ||
!this.props.statements
) {
this.refreshStatements();
}

this.props.refreshUserSQLRoles();
if (!this.props.isTenant) {
this.props.refreshNodes();
Expand Down Expand Up @@ -340,14 +337,21 @@ export class StatementsPage extends React.Component<
);
}

componentDidUpdate = (): void => {
componentDidUpdate = (prevProps: StatementsPageProps): void => {
this.updateQueryParams();
if (!this.props.isTenant) {
this.props.refreshNodes();
if (!this.props.hasViewActivityRedactedRole) {
this.props.refreshStatementDiagnosticsRequests();
}
}

if (
prevProps.timeScale !== this.props.timeScale ||
(prevProps.isDataValid && !this.props.isDataValid)
) {
this.refreshStatements();
}
};

componentWillUnmount(): void {
Expand Down Expand Up @@ -438,8 +442,12 @@ export class StatementsPage extends React.Component<
};

filteredStatementsData = (): AggregateStatistics[] => {
const { filters } = this.state;
const { search, statements, nodeRegions, isTenant } = this.props;
const { search, statements, nodeRegions, isTenant, filters } = this.props;

if (!statements) {
return [];
}

const timeValue = getTimeValueInSeconds(filters);
const sqlTypes =
filters.sqlType.length > 0
Expand Down Expand Up @@ -517,7 +525,6 @@ export class StatementsPage extends React.Component<
renderStatements = (regions: string[]): React.ReactElement => {
const { pagination, filters, activeFilters } = this.state;
const {
statements,
onSelectDiagnosticsReportDropdownOption,
onStatementClick,
columns: userSelectedColumnsToShow,
Expand All @@ -530,6 +537,8 @@ export class StatementsPage extends React.Component<
} = this.props;
const data = this.filteredStatementsData();
const totalWorkload = calculateTotalWorkload(data);
const statements = this.props.statements ?? [];

const totalCount = data.length;
const isEmptySearchResults = statements?.length > 0 && search?.length > 0;
// If the cluster is a tenant cluster we don't show info
Expand Down Expand Up @@ -647,15 +656,14 @@ export class StatementsPage extends React.Component<
: unique(nodes.map(node => nodeRegions[node.toString()])).sort();
const { filters, activeFilters } = this.state;

const longLoadingMessage = isNil(this.props.statements) &&
isNil(getValidErrorsList(this.props.statementsError)) && (
<Delayed delay={moment.duration(2, "s")}>
<InlineAlert
intent="info"
title="If the selected time period contains a large amount of data, this page might take a few minutes to load."
/>
</Delayed>
);
const longLoadingMessage = (
<Delayed delay={moment.duration(2, "s")}>
<InlineAlert
intent="info"
title="If the selected time interval contains a large amount of data, this page might take a few minutes to load."
/>
</Delayed>
);

return (
<div className={cx("root", "table-area")}>
Expand Down Expand Up @@ -704,7 +712,7 @@ export class StatementsPage extends React.Component<
)}
</PageConfig>
<Loading
loading={isNil(this.props.statements)}
loading={this.props.isReqInFlight}
page={"statements"}
error={this.props.statementsError}
render={() => this.renderStatements(regions)}
Expand All @@ -717,7 +725,9 @@ export class StatementsPage extends React.Component<
})
}
/>
{longLoadingMessage}
{this.props.isReqInFlight &&
getValidErrorsList(this.props.statementsError) == null &&
longLoadingMessage}
<ActivateStatementDiagnosticsModal
ref={this.activateDiagnosticsRef}
activate={onActivateStatementDiagnostics}
Expand Down
14 changes: 9 additions & 5 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx
View file
Expand Up @@ -21,21 +21,23 @@ import { actions as nodesActions } from "../store/nodes";
import {
StatementsPage,
StatementsPageDispatchProps,
StatementsPageProps,
StatementsPageStateProps,
} from "./statementsPage";
import {
selectApps,
selectDatabases,
selectLastReset,
selectStatements,
selectStatementsDataValid,
selectStatementsDataInFlight,
selectStatementsLastError,
selectTotalFingerprints,
selectColumns,
selectTimeScale,
selectSortSetting,
selectFilters,
selectSearch,
selectStatementsLastUpdated,
} from "./statementsPage.selectors";
import {
selectIsTenant,
Expand All @@ -62,7 +64,7 @@ export const ConnectedStatementsPage = withRouter(
StatementsPageDispatchProps,
RouteComponentProps
>(
(state: AppState, props: StatementsPageProps) => ({
(state: AppState, props): StatementsPageStateProps => ({
apps: selectApps(state),
columns: selectColumns(state),
databases: selectDatabases(state),
Expand All @@ -72,10 +74,13 @@ export const ConnectedStatementsPage = withRouter(
hasViewActivityRedactedRole: selectHasViewActivityRedactedRole(state),
hasAdminRole: selectHasAdminRole(state),
lastReset: selectLastReset(state),
nodeRegions: selectIsTenant(state) ? {} : nodeRegionsByIDSelector(state),
nodeRegions: nodeRegionsByIDSelector(state),
search: selectSearch(state),
sortSetting: selectSortSetting(state),
statements: selectStatements(state, props),
isDataValid: selectStatementsDataValid(state),
isReqInFlight: selectStatementsDataInFlight(state),
lastUpdated: selectStatementsLastUpdated(state),
statementsError: selectStatementsLastError(state),
totalFingerprints: selectTotalFingerprints(state),
}),
Expand All @@ -94,8 +99,7 @@ export const ConnectedStatementsPage = withRouter(
refreshNodes: () => dispatch(nodesActions.refresh()),
refreshUserSQLRoles: () =>
dispatch(uiConfigActions.refreshUserSQLRoles()),
resetSQLStats: (req: StatementsRequest) =>
dispatch(sqlStatsActions.reset(req)),
resetSQLStats: () => dispatch(sqlStatsActions.reset()),
dismissAlertMessage: () =>
dispatch(
localStorageActions.update({
Expand Down
23 changes: 18 additions & 5 deletions pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts
View file
Expand Up @@ -8,7 +8,6 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import moment from "moment";
import { createSlice, PayloadAction } from "@reduxjs/toolkit";
import { DOMAIN_NAME } from "../utils";
import { defaultFilters, Filters } from "../../queryFilter";
Expand All @@ -19,12 +18,16 @@ type SortSetting = {
columnTitle: string;
};

export enum LocalStorageKeys {
GLOBAL_TIME_SCALE = "timeScale/SQLActivity",
}

export type LocalStorageState = {
"adminUi/showDiagnosticsModal": boolean;
"showColumns/StatementsPage": string;
"showColumns/TransactionPage": string;
"showColumns/SessionsPage": string;
"timeScale/SQLActivity": TimeScale;
[LocalStorageKeys.GLOBAL_TIME_SCALE]: TimeScale;
"sortSetting/StatementsPage": SortSetting;
"sortSetting/TransactionsPage": SortSetting;
"sortSetting/SessionsPage": SortSetting;
Expand All @@ -37,7 +40,11 @@ export type LocalStorageState = {

type Payload = {
key: keyof LocalStorageState;
value: any;
value: unknown;
};

export type TypedPayload<T> = {
value: T;
};

const defaultSortSetting: SortSetting = {
Expand All @@ -61,8 +68,8 @@ const initialState: LocalStorageState = {
JSON.parse(localStorage.getItem("showColumns/TransactionPage")) || null,
"showColumns/SessionsPage":
JSON.parse(localStorage.getItem("showColumns/SessionsPage")) || null,
"timeScale/SQLActivity":
JSON.parse(localStorage.getItem("timeScale/SQLActivity")) ||
[LocalStorageKeys.GLOBAL_TIME_SCALE]:
JSON.parse(localStorage.getItem(LocalStorageKeys.GLOBAL_TIME_SCALE)) ||
defaultTimeScaleSelected,
"sortSetting/StatementsPage":
JSON.parse(localStorage.getItem("sortSetting/StatementsPage")) ||
Expand Down Expand Up @@ -94,6 +101,12 @@ const localStorageSlice = createSlice({
update: (state: any, action: PayloadAction<Payload>) => {
state[action.payload.key] = action.payload.value;
},
updateTimeScale: (
state,
action: PayloadAction<TypedPayload<TimeScale>>,
) => {
state[LocalStorageKeys.GLOBAL_TIME_SCALE] = action.payload.value;
},
},
});

Expand Down

0 comments on commit 497898f

Please sign in to comment.