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: #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 Mar 16, 2023
1 parent dbbf20d commit 40dd354
Show file tree
Hide file tree
Showing 22 changed files with 179 additions and 242 deletions.
3 changes: 1 addition & 2 deletions pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetailsConnected.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ export const SessionDetailsPageConnected = withRouter(
refreshNodes: nodesActions.refresh,
refreshNodesLiveness: nodesLivenessActions.refresh,
setTimeScale: (ts: TimeScale) =>
localStorageActions.update({
key: "timeScale/SQLActivity",
localStorageActions.updateTimeScale({
value: ts,
}),
onTerminateSessionClick: () =>
Expand Down
1 change: 1 addition & 0 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
},
lastUpdated,
isDataValid: true,
isReqInFlight: false,
// Aggregate key values in these statements will need to change if implementation
// of 'statementKey' in appStats.ts changes.
statementsError: null,
Expand Down
7 changes: 7 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ export const selectStatementsDataValid = createSelector(
},
);

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

export const selectStatements = createSelector(
sqlStatsSelector,
(_: AppState, props: RouteComponentProps) => props,
Expand Down
127 changes: 40 additions & 87 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import React from "react";
import { RouteComponentProps } from "react-router-dom";
import { flatMap, isNil, merge } from "lodash";
import { flatMap, merge } from "lodash";
import classNames from "classnames/bind";
import { getValidErrorsList, Loading } from "src/loading";
import { Delayed } from "src/delayed";
Expand Down Expand Up @@ -82,8 +82,6 @@ import { filteredStatementsData } from "../sqlActivity/util";
const cx = classNames.bind(styles);
const sortableTableCx = classNames.bind(sortableTableStyles);

const POLLING_INTERVAL_MILLIS = 300000;

// Most of the props are supposed to be provided as connected props
// from redux store.
// StatementsPageDispatchProps, StatementsPageStateProps, and StatementsPageOuterProps interfaces
Expand All @@ -95,7 +93,7 @@ export interface StatementsPageDispatchProps {
refreshStatementDiagnosticsRequests: () => void;
refreshNodes: () => void;
refreshUserSQLRoles: () => void;
resetSQLStats: (req: StatementsRequest) => void;
resetSQLStats: () => void;
dismissAlertMessage: () => void;
onActivateStatementDiagnostics: (
insertStmtDiagnosticsRequest: InsertStmtDiagnosticRequest,
Expand All @@ -120,6 +118,7 @@ export interface StatementsPageDispatchProps {
export interface StatementsPageStateProps {
statements: AggregateStatistics[];
isDataValid: boolean;
isReqInFlight: boolean;
lastUpdated: moment.Moment | null;
timeScale: TimeScale;
statementsError: Error | null;
Expand All @@ -141,7 +140,6 @@ export interface StatementsPageState {
pagination: ISortedTablePagination;
filters?: Filters;
activeFilters?: number;
startRequest?: Date;
}

export type StatementsPageProps = StatementsPageDispatchProps &
Expand Down Expand Up @@ -187,7 +185,6 @@ export class StatementsPage extends React.Component<
StatementsPageState
> {
activateDiagnosticsRef: React.RefObject<ActivateDiagnosticsModalRef>;
refreshDataTimeout: NodeJS.Timeout;

constructor(props: StatementsPageProps) {
super(props);
Expand All @@ -196,19 +193,10 @@ export class StatementsPage extends React.Component<
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 @@ -267,10 +255,6 @@ export class StatementsPage extends React.Component<
if (this.props.onTimeScaleChange) {
this.props.onTimeScaleChange(ts);
}
this.refreshStatements(ts);
this.setState({
startRequest: new Date(),
});
};

resetPagination = (): void => {
Expand All @@ -284,72 +268,34 @@ export class StatementsPage extends React.Component<
});
};

clearRefreshDataTimeout(): void {
if (this.refreshDataTimeout != null) {
clearTimeout(this.refreshDataTimeout);
}
}

resetPolling(ts: TimeScale): void {
this.clearRefreshDataTimeout();
if (ts.key !== "Custom") {
this.refreshDataTimeout = setTimeout(
this.refreshStatements,
POLLING_INTERVAL_MILLIS, // 5 minutes
ts,
);
}
}

refreshStatements = (ts?: TimeScale): void => {
const time = ts ?? this.props.timeScale;
const req = stmtsRequestFromTimeScale(time);
refreshStatements = (): void => {
const req = stmtsRequestFromTimeScale(this.props.timeScale);
this.props.refreshStatements(req);

this.resetPolling(time);
};

refreshDatabases = (): void => {
this.props.refreshDatabases();
};

resetSQLStats = (): void => {
const req = stmtsRequestFromTimeScale(this.props.timeScale);
this.props.resetSQLStats(req);
this.setState({
startRequest: new Date(),
});
this.props.resetSQLStats();
};

componentDidMount(): void {
this.setState({
startRequest: new Date(),
});

// For the first data fetch for this page, we refresh immediately if:
// - Last updated is null (no statements fetched previously)
// - The data is not valid (time scale may have changed on other pages)
// - The time range selected is a moving window and the last udpated time
// is >= 5 minutes.
// Otherwise, we schedule a refresh at 5 mins from the lastUpdated time if
// the time range selected is a moving window (i.e. not custom).
const now = moment();
let nextRefresh = null;
if (this.props.lastUpdated == null || !this.props.isDataValid) {
nextRefresh = now;
} else if (this.props.timeScale.key !== "Custom") {
nextRefresh = this.props.lastUpdated
.clone()
.add(POLLING_INTERVAL_MILLIS, "milliseconds");
}
if (nextRefresh) {
this.refreshDataTimeout = setTimeout(
this.refreshStatements,
Math.max(0, nextRefresh.diff(now, "milliseconds")),
);
}

this.refreshDatabases();
// 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) {
Expand Down Expand Up @@ -392,19 +338,25 @@ 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 {
this.props.dismissAlertMessage();
this.clearRefreshDataTimeout();
}

onChangePage = (current: number): void => {
Expand Down Expand Up @@ -493,7 +445,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 @@ -504,6 +455,7 @@ export class StatementsPage extends React.Component<
sortSetting,
search,
} = this.props;
const statements = this.props.statements ?? [];
const data = filteredStatementsData(
filters,
search,
Expand Down Expand Up @@ -626,15 +578,14 @@ export class StatementsPage extends React.Component<

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 interval 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")}>
Expand Down Expand Up @@ -684,7 +635,7 @@ export class StatementsPage extends React.Component<
</PageConfig>
<div className={cx("table-area")}>
<Loading
loading={isNil(this.props.statements)}
loading={this.props.isReqInFlight}
page={"statements"}
error={this.props.statementsError}
render={() => this.renderStatements(regions)}
Expand All @@ -697,7 +648,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
5 changes: 3 additions & 2 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
selectLastReset,
selectStatements,
selectStatementsDataValid,
selectStatementsDataInFlight,
selectStatementsLastError,
selectTotalFingerprints,
selectColumns,
Expand Down Expand Up @@ -97,6 +98,7 @@ export const ConnectedStatementsPage = withRouter(
sortSetting: selectSortSetting(state),
statements: selectStatements(state, props),
isDataValid: selectStatementsDataValid(state),
isReqInFlight: selectStatementsDataInFlight(state),
lastUpdated: selectStatementsLastUpdated(state),
statementsError: selectStatementsLastError(state),
totalFingerprints: selectTotalFingerprints(state),
Expand All @@ -120,8 +122,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
20 changes: 17 additions & 3 deletions pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ type SortSetting = {
columnTitle: string;
};

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

export type LocalStorageState = {
"adminUi/showDiagnosticsModal": boolean;
"showColumns/ActiveStatementsPage": string;
Expand All @@ -28,7 +32,7 @@ export type LocalStorageState = {
"showColumns/SessionsPage": string;
"showColumns/StatementInsightsPage": string;
"showColumns/JobsPage": string;
"timeScale/SQLActivity": TimeScale;
[LocalStorageKeys.GLOBAL_TIME_SCALE]: TimeScale;
"sortSetting/ActiveStatementsPage": SortSetting;
"sortSetting/ActiveTransactionsPage": SortSetting;
"sortSetting/StatementsPage": SortSetting;
Expand Down Expand Up @@ -58,6 +62,10 @@ type Payload = {
value: any;
};

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

const defaultSortSetting: SortSetting = {
ascending: false,
columnTitle: "executionCount",
Expand Down Expand Up @@ -134,8 +142,8 @@ const initialState: LocalStorageState = {
"showSetting/JobsPage":
JSON.parse(localStorage.getItem("showSetting/JobsPage")) ||
defaultJobShowSetting,
"timeScale/SQLActivity":
JSON.parse(localStorage.getItem("timeScale/SQLActivity")) ||
[LocalStorageKeys.GLOBAL_TIME_SCALE]:
JSON.parse(localStorage.getItem(LocalStorageKeys.GLOBAL_TIME_SCALE)) ||
defaultTimeScaleSelected,
"sortSetting/ActiveStatementsPage":
JSON.parse(localStorage.getItem("sortSetting/ActiveStatementsPage")) ||
Expand Down Expand Up @@ -205,6 +213,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
Loading

0 comments on commit 40dd354

Please sign in to comment.