From 40dd35464be6a39771218dd73dea0e7ee3e0bfd4 Mon Sep 17 00:00:00 2001 From: Xin Hao Zhang Date: Thu, 9 Mar 2023 14:20:21 -0500 Subject: [PATCH] ui: remove auto polling from fingerprints pages 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. --- .../src/sessions/sessionDetailsConnected.tsx | 3 +- .../statementsPage/statementsPage.fixture.ts | 1 + .../statementsPage.selectors.ts | 7 + .../src/statementsPage/statementsPage.tsx | 127 ++++++------------ .../statementsPageConnected.tsx | 5 +- .../localStorage/localStorage.reducer.ts | 20 ++- .../store/localStorage/localStorage.saga.ts | 18 ++- .../src/store/sqlStats/sqlStats.reducer.ts | 16 ++- .../src/store/sqlStats/sqlStats.sagas.spec.ts | 10 +- .../src/store/sqlStats/sqlStats.sagas.ts | 19 +-- .../cluster-ui/src/store/utils/selectors.ts | 3 +- .../src/transactionsPage/transactionsPage.tsx | 100 +++++--------- .../transactionsPageConnected.tsx | 5 +- .../src/redux/sqlStats/sqlStatsActions.ts | 17 +-- .../src/redux/sqlStats/sqlStatsSagas.spec.ts | 17 +-- .../src/redux/sqlStats/sqlStatsSagas.ts | 19 +-- .../src/redux/statements/statementsSagas.ts | 6 - .../db-console/src/redux/timeScale.ts | 19 ++- .../executionFingerprintsSelectors.tsx | 3 + .../src/views/statements/statements.spec.tsx | 2 +- .../src/views/statements/statementsPage.tsx | 2 + .../views/transactions/transactionsPage.tsx | 2 + 22 files changed, 179 insertions(+), 242 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetailsConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetailsConnected.tsx index f89122ea9f79..87974f24b8bb 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetailsConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetailsConnected.tsx @@ -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: () => diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts index e41138051303..c13d351ae5b2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts @@ -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, diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts index f8252ba5b6a1..f0be5e98c63c 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts @@ -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, diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 17c5c8dc081f..4e3e07e461f7 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -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"; @@ -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 @@ -95,7 +93,7 @@ export interface StatementsPageDispatchProps { refreshStatementDiagnosticsRequests: () => void; refreshNodes: () => void; refreshUserSQLRoles: () => void; - resetSQLStats: (req: StatementsRequest) => void; + resetSQLStats: () => void; dismissAlertMessage: () => void; onActivateStatementDiagnostics: ( insertStmtDiagnosticsRequest: InsertStmtDiagnosticRequest, @@ -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; @@ -141,7 +140,6 @@ export interface StatementsPageState { pagination: ISortedTablePagination; filters?: Filters; activeFilters?: number; - startRequest?: Date; } export type StatementsPageProps = StatementsPageDispatchProps & @@ -187,7 +185,6 @@ export class StatementsPage extends React.Component< StatementsPageState > { activateDiagnosticsRef: React.RefObject; - refreshDataTimeout: NodeJS.Timeout; constructor(props: StatementsPageProps) { super(props); @@ -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 => { @@ -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 => { @@ -284,29 +268,9 @@ 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 => { @@ -314,42 +278,24 @@ export class StatementsPage extends React.Component< }; 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) { @@ -392,7 +338,7 @@ export class StatementsPage extends React.Component< ); } - componentDidUpdate = (): void => { + componentDidUpdate = (prevProps: StatementsPageProps): void => { this.updateQueryParams(); if (!this.props.isTenant) { this.props.refreshNodes(); @@ -400,11 +346,17 @@ export class StatementsPage extends React.Component< 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 => { @@ -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, @@ -504,6 +455,7 @@ export class StatementsPage extends React.Component< sortSetting, search, } = this.props; + const statements = this.props.statements ?? []; const data = filteredStatementsData( filters, search, @@ -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)) && ( - - - - ); + const longLoadingMessage = ( + + + + ); return (
@@ -684,7 +635,7 @@ export class StatementsPage extends React.Component<
this.renderStatements(regions)} @@ -697,7 +648,9 @@ export class StatementsPage extends React.Component< }) } /> - {longLoadingMessage} + {this.props.isReqInFlight && + getValidErrorsList(this.props.statementsError) == null && + longLoadingMessage} dispatch(nodesActions.refresh()), refreshUserSQLRoles: () => dispatch(uiConfigActions.refreshUserSQLRoles()), - resetSQLStats: (req: StatementsRequest) => - dispatch(sqlStatsActions.reset(req)), + resetSQLStats: () => dispatch(sqlStatsActions.reset()), dismissAlertMessage: () => dispatch( localStorageActions.update({ diff --git a/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts b/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts index e91f13dbc49f..75aa472c490d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts @@ -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; @@ -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; @@ -58,6 +62,10 @@ type Payload = { value: any; }; +type TypedPayload = { + value: T; +}; + const defaultSortSetting: SortSetting = { ascending: false, columnTitle: "executionCount", @@ -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")) || @@ -205,6 +213,12 @@ const localStorageSlice = createSlice({ update: (state: any, action: PayloadAction) => { state[action.payload.key] = action.payload.value; }, + updateTimeScale: ( + state, + action: PayloadAction>, + ) => { + state[LocalStorageKeys.GLOBAL_TIME_SCALE] = action.payload.value; + }, }, }); diff --git a/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.saga.ts b/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.saga.ts index c2d0ca08df60..d4b8dc526d54 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.saga.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.saga.ts @@ -9,8 +9,11 @@ // licenses/APL.txt. import { AnyAction } from "redux"; -import { all, call, takeEvery } from "redux-saga/effects"; +import { all, call, takeEvery, takeLatest, put } from "redux-saga/effects"; import { actions } from "./localStorage.reducer"; +import { actions as sqlStatsActions } from "src/store/sqlStats"; +import { actions as stmtInsightActions } from "src/store/insights/statementInsights"; +import { actions as txnInsightActions } from "src/store/insights/transactionInsights"; export function* updateLocalStorageItemSaga(action: AnyAction) { const { key, value } = action.payload; @@ -21,6 +24,17 @@ export function* updateLocalStorageItemSaga(action: AnyAction) { ); } +export function* updateTimeScale(action: AnyAction) { + yield all([ + put(sqlStatsActions.invalidated()), + put(stmtInsightActions.invalidated()), + put(txnInsightActions.invalidated()), + ]); +} + export function* localStorageSaga() { - yield all([takeEvery(actions.update, updateLocalStorageItemSaga)]); + yield all([ + takeEvery(actions.update, updateLocalStorageItemSaga), + takeLatest(actions.updateTimeScale, updateLocalStorageItemSaga), + ]); } diff --git a/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.reducer.ts b/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.reducer.ts index ad6a3f6b36b2..64b07f0f0317 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.reducer.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.reducer.ts @@ -22,6 +22,7 @@ export type SQLStatsState = { lastError: Error; valid: boolean; lastUpdated: moment.Moment | null; + inFlight: boolean; }; const initialState: SQLStatsState = { @@ -29,6 +30,7 @@ const initialState: SQLStatsState = { lastError: null, valid: false, lastUpdated: null, + inFlight: false, }; export type UpdateTimeScalePayload = { @@ -44,19 +46,25 @@ const sqlStatsSlice = createSlice({ state.valid = true; state.lastError = null; state.lastUpdated = moment.utc(); + state.inFlight = false; }, failed: (state, action: PayloadAction) => { state.valid = false; state.lastError = action.payload; state.lastUpdated = moment.utc(); + state.inFlight = false; }, invalidated: state => { state.valid = false; }, - refresh: (_, action: PayloadAction) => {}, - request: (_, action: PayloadAction) => {}, - updateTimeScale: (_, action: PayloadAction) => {}, - reset: (_, action: PayloadAction) => {}, + refresh: (state, action: PayloadAction) => { + state.inFlight = true; + }, + request: (state, action: PayloadAction) => { + state.inFlight = true; + }, + updateTimeScale: (_, _action: PayloadAction) => {}, + reset: (_, _action: PayloadAction) => {}, }, }); diff --git a/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.spec.ts b/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.spec.ts index 232574bb34e0..9d0b0322198a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.spec.ts @@ -82,6 +82,7 @@ describe("SQLStats sagas", () => { lastError: null, valid: true, lastUpdated, + inFlight: false, }) .run(); }); @@ -97,6 +98,7 @@ describe("SQLStats sagas", () => { lastError: error, valid: false, lastUpdated, + inFlight: false, }) .run(); }); @@ -107,23 +109,24 @@ describe("SQLStats sagas", () => { new cockroach.server.serverpb.ResetSQLStatsResponse(); it("successfully resets SQL stats", () => { - return expectSaga(resetSQLStatsSaga, payload) + return expectSaga(resetSQLStatsSaga) .provide([[matchers.call.fn(resetSQLStats), resetSQLStatsResponse]]) .put(sqlDetailsStatsActions.invalidateAll()) - .put(actions.refresh()) + .put(actions.invalidated()) .withReducer(reducer) .hasFinalState({ data: null, lastError: null, valid: false, lastUpdated: null, + inFlight: false, }) .run(); }); it("returns error on failed reset", () => { const err = new Error("failed to reset"); - return expectSaga(resetSQLStatsSaga, payload) + return expectSaga(resetSQLStatsSaga) .provide([[matchers.call.fn(resetSQLStats), throwError(err)]]) .put(actions.failed(err)) .withReducer(reducer) @@ -132,6 +135,7 @@ describe("SQLStats sagas", () => { lastError: err, valid: false, lastUpdated, + inFlight: false, }) .run(); }); diff --git a/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.ts b/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.ts index 9ef9a790b6cd..d6d6ae21911c 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.ts @@ -21,8 +21,6 @@ import { UpdateTimeScalePayload, } from "./sqlStats.reducer"; import { actions as sqlDetailsStatsActions } from "../statementDetails/statementDetails.reducer"; -import { actions as stmtInsightActions } from "../insights/statementInsights"; -import { actions as txnInsightActions } from "../insights/transactionInsights"; export function* refreshSQLStatsSaga(action: PayloadAction) { yield put(sqlStatsActions.request(action.payload)); @@ -44,24 +42,19 @@ export function* updateSQLStatsTimeScaleSaga( ) { const { ts } = action.payload; yield put( - localStorageActions.update({ - key: "timeScale/SQLActivity", + localStorageActions.updateTimeScale({ value: ts, }), ); - yield all([ - put(sqlStatsActions.invalidated()), - put(stmtInsightActions.invalidated()), - put(txnInsightActions.invalidated()), - ]); } -export function* resetSQLStatsSaga(action: PayloadAction) { +export function* resetSQLStatsSaga() { try { yield call(resetSQLStats); - yield put(sqlDetailsStatsActions.invalidateAll()); - yield put(sqlStatsActions.invalidated()); - yield put(sqlStatsActions.refresh(action.payload)); + yield all([ + put(sqlDetailsStatsActions.invalidateAll()), + put(sqlStatsActions.invalidated()), + ]); } catch (e) { yield put(sqlStatsActions.failed(e)); } diff --git a/pkg/ui/workspaces/cluster-ui/src/store/utils/selectors.ts b/pkg/ui/workspaces/cluster-ui/src/store/utils/selectors.ts index 4e70cd56dc58..9b986ae61c7d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/utils/selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/utils/selectors.ts @@ -9,6 +9,7 @@ // licenses/APL.txt. import { createSelector } from "reselect"; +import { LocalStorageKeys } from "../localStorage"; import { AppState } from "../reducers"; export const adminUISelector = createSelector( @@ -23,5 +24,5 @@ export const localStorageSelector = createSelector( export const selectTimeScale = createSelector( localStorageSelector, - localStorage => localStorage["timeScale/SQLActivity"], + localStorage => localStorage[LocalStorageKeys.GLOBAL_TIME_SCALE], ); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx index f1495ca0ffdc..435caf799600 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx @@ -78,7 +78,6 @@ type IStatementsResponse = protos.cockroach.server.serverpb.IStatementsResponse; const cx = classNames.bind(styles); -const POLLING_INTERVAL_MILLIS = 300000; interface TState { filters?: Filters; pagination: ISortedTablePagination; @@ -88,6 +87,7 @@ export interface TransactionsPageStateProps { columns: string[]; data: IStatementsResponse; isDataValid: boolean; + isReqInFlight: boolean; lastUpdated: moment.Moment | null; timeScale: TimeScale; error?: Error | null; @@ -104,7 +104,7 @@ export interface TransactionsPageDispatchProps { refreshData: (req: StatementsRequest) => void; refreshNodes: () => void; refreshUserSQLRoles: () => void; - resetSQLStats: (req: StatementsRequest) => void; + resetSQLStats: () => void; onTimeScaleChange?: (ts: TimeScale) => void; onColumnsChange?: (selectedColumns: string[]) => void; onFilterChange?: (value: Filters) => void; @@ -134,8 +134,6 @@ export class TransactionsPage extends React.Component< TransactionsPageProps, TState > { - refreshDataTimeout: NodeJS.Timeout; - constructor(props: TransactionsPageProps) { super(props); this.state = { @@ -146,14 +144,6 @@ export class TransactionsPage extends React.Component< }; const stateFromHistory = this.getStateFromHistory(); this.state = merge(this.state, stateFromHistory); - - // 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 => { @@ -194,61 +184,28 @@ export class TransactionsPage extends React.Component< }; }; - clearRefreshDataTimeout(): void { - if (this.refreshDataTimeout != null) { - clearTimeout(this.refreshDataTimeout); - } - } - - // Schedule the next data request depending on the time - // range key. - resetPolling(ts: TimeScale): void { - this.clearRefreshDataTimeout(); - if (ts.key !== "Custom") { - this.refreshDataTimeout = setTimeout( - this.refreshData, - POLLING_INTERVAL_MILLIS, // 5 minutes - ts, - ); - } - } - - refreshData = (ts?: TimeScale): void => { - const time = ts ?? this.props.timeScale; - const req = stmtsRequestFromTimeScale(time); + refreshData = (): void => { + const req = stmtsRequestFromTimeScale(this.props.timeScale); this.props.refreshData(req); - - this.resetPolling(time); }; resetSQLStats = (): void => { - const req = stmtsRequestFromTimeScale(this.props.timeScale); - this.props.resetSQLStats(req); - this.resetPolling(this.props.timeScale); + this.props.resetSQLStats(); }; componentDidMount(): void { - // 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.refreshData, - Math.max(0, nextRefresh.diff(now, "milliseconds")), - ); + // 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.data || + !this.props.lastUpdated + ) { + this.refreshData(); } if (!this.props.isTenant) { @@ -258,10 +215,6 @@ export class TransactionsPage extends React.Component< this.props.refreshUserSQLRoles(); } - componentWillUnmount(): void { - this.clearRefreshDataTimeout(); - } - updateQueryParams(): void { const { history, search, sortSetting } = this.props; const tab = "Transactions"; @@ -294,11 +247,18 @@ export class TransactionsPage extends React.Component< ); } - componentDidUpdate(): void { + componentDidUpdate(prevProps: TransactionsPageProps): void { this.updateQueryParams(); if (!this.props.isTenant) { this.props.refreshNodes(); } + + if ( + prevProps.timeScale !== this.props.timeScale || + (prevProps.isDataValid && !this.props.isDataValid) + ) { + this.refreshData(); + } } onChangeSortSetting = (ss: SortSetting): void => { @@ -407,7 +367,6 @@ export class TransactionsPage extends React.Component< if (this.props.onTimeScaleChange) { this.props.onTimeScaleChange(ts); } - this.refreshData(ts); }; render(): React.ReactElement { @@ -454,7 +413,8 @@ export class TransactionsPage extends React.Component< data?.transactions || [], internal_app_name_prefix, ); - const longLoadingMessage = !this.props?.data && ( + + const longLoadingMessage = (
{ @@ -520,7 +480,7 @@ export class TransactionsPage extends React.Component< }), ); const { current, pageSize } = pagination; - const hasData = data.transactions?.length > 0; + const hasData = data?.transactions?.length > 0; const isUsedFilter = search?.length > 0; // Creates a list of all possible columns, @@ -607,7 +567,7 @@ export class TransactionsPage extends React.Component< }) } /> - {longLoadingMessage} + {this.props.isReqInFlight && longLoadingMessage}
); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx index 504934470822..aa8eced85861 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx @@ -32,6 +32,7 @@ import { nodeRegionsByIDSelector } from "../store/nodes"; import { selectStatementsLastUpdated, selectStatementsDataValid, + selectStatementsDataInFlight, } from "src/statementsPage/statementsPage.selectors"; import { selectTimeScale } from "../store/utils/selectors"; import { StatementsRequest } from "src/api/statementsApi"; @@ -75,6 +76,7 @@ export const TransactionsPageConnected = withRouter( columns: selectTxnColumns(state), data: selectTransactionsData(state), isDataValid: selectStatementsDataValid(state), + isReqInFlight: selectStatementsDataInFlight(state), lastUpdated: selectStatementsLastUpdated(state), timeScale: selectTimeScale(state), error: selectTransactionsLastError(state), @@ -94,8 +96,7 @@ export const TransactionsPageConnected = withRouter( refreshNodes: () => dispatch(nodesActions.refresh()), refreshUserSQLRoles: () => dispatch(uiConfigActions.refreshUserSQLRoles()), - resetSQLStats: (req: StatementsRequest) => - dispatch(sqlStatsActions.reset(req)), + resetSQLStats: () => dispatch(sqlStatsActions.reset()), onTimeScaleChange: (ts: TimeScale) => { dispatch( sqlStatsActions.updateTimeScale({ diff --git a/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsActions.ts b/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsActions.ts index 4a257e7c2e75..453d0117fec4 100644 --- a/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsActions.ts +++ b/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsActions.ts @@ -9,29 +9,14 @@ // licenses/APL.txt. import { Action } from "redux"; -import { PayloadAction } from "@reduxjs/toolkit"; -import { cockroach } from "src/js/protos"; export const RESET_SQL_STATS = "cockroachui/sqlStats/RESET_SQL_STATS"; -export const RESET_SQL_STATS_COMPLETE = - "cockroachui/sqlStats/RESET_SQL_STATS_COMPLETE"; export const RESET_SQL_STATS_FAILED = "cockroachui/sqlStats/RESET_SQL_STATS_FAILED"; -import StatementsRequest = cockroach.server.serverpb.StatementsRequest; - -export function resetSQLStatsAction( - req: StatementsRequest, -): PayloadAction { +export function resetSQLStatsAction(): Action { return { type: RESET_SQL_STATS, - payload: req, - }; -} - -export function resetSQLStatsCompleteAction(): Action { - return { - type: RESET_SQL_STATS_COMPLETE, }; } diff --git a/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.spec.ts b/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.spec.ts index 1bdabd03fd78..7816d90a540c 100644 --- a/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.spec.ts +++ b/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.spec.ts @@ -11,11 +11,7 @@ import { expectSaga } from "redux-saga-test-plan"; import { call } from "redux-saga-test-plan/matchers"; -import { - resetSQLStatsFailedAction, - resetSQLStatsCompleteAction, - resetSQLStatsAction, -} from "./sqlStatsActions"; +import { resetSQLStatsFailedAction } from "./sqlStatsActions"; import { resetSQLStatsSaga } from "./sqlStatsSagas"; import { resetSQLStats } from "src/util/api"; import { @@ -26,25 +22,18 @@ import { import { throwError } from "redux-saga-test-plan/providers"; import { cockroach } from "src/js/protos"; -import Long from "long"; describe("SQL Stats sagas", () => { describe("resetSQLStatsSaga", () => { - const payload = new cockroach.server.serverpb.StatementsRequest({ - start: Long.fromNumber(1596816675), - end: Long.fromNumber(1596820675), - combined: false, - }); const resetSQLStatsResponse = new cockroach.server.serverpb.ResetSQLStatsResponse(); it("successfully resets SQL stats", () => { // TODO(azhng): validate refreshStatement() actions once we can figure out // how to get ThunkAction to work with sagas. - return expectSaga(resetSQLStatsSaga, resetSQLStatsAction(payload)) + return expectSaga(resetSQLStatsSaga) .withReducer(apiReducersReducer) .provide([[call.fn(resetSQLStats), resetSQLStatsResponse]]) - .put(resetSQLStatsCompleteAction()) .put(invalidateStatements()) .put(invalidateAllStatementDetails()) .run(); @@ -52,7 +41,7 @@ describe("SQL Stats sagas", () => { it("returns error on failed reset", () => { const err = new Error("failed to reset"); - return expectSaga(resetSQLStatsSaga, resetSQLStatsAction(payload)) + return expectSaga(resetSQLStatsSaga) .provide([[call.fn(resetSQLStats), throwError(err)]]) .put(resetSQLStatsFailedAction()) .run(); diff --git a/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.ts b/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.ts index 0b44677d2223..d313b4f217ed 100644 --- a/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.ts +++ b/pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.ts @@ -11,22 +11,15 @@ import { cockroach } from "src/js/protos"; import { resetSQLStats } from "src/util/api"; import { all, call, put, takeEvery } from "redux-saga/effects"; -import { - RESET_SQL_STATS, - resetSQLStatsCompleteAction, - resetSQLStatsFailedAction, -} from "./sqlStatsActions"; +import { RESET_SQL_STATS, resetSQLStatsFailedAction } from "./sqlStatsActions"; import { invalidateAllStatementDetails, invalidateStatements, - refreshStatements, } from "src/redux/apiReducers"; import ResetSQLStatsRequest = cockroach.server.serverpb.ResetSQLStatsRequest; -import StatementsRequest = cockroach.server.serverpb.StatementsRequest; -import { PayloadAction } from "@reduxjs/toolkit"; -export function* resetSQLStatsSaga(action: PayloadAction) { +export function* resetSQLStatsSaga() { const resetSQLStatsRequest = new ResetSQLStatsRequest({ // reset_persisted_stats is set to true in order to clear both // in-memory stats as well as persisted stats. @@ -34,10 +27,10 @@ export function* resetSQLStatsSaga(action: PayloadAction) { }); try { yield call(resetSQLStats, resetSQLStatsRequest); - yield put(resetSQLStatsCompleteAction()); - yield put(invalidateStatements()); - yield put(invalidateAllStatementDetails()); - yield put(refreshStatements(action.payload) as any); + yield all([ + put(invalidateStatements()), + put(invalidateAllStatementDetails()), + ]); } catch (e) { yield put(resetSQLStatsFailedAction()); } diff --git a/pkg/ui/workspaces/db-console/src/redux/statements/statementsSagas.ts b/pkg/ui/workspaces/db-console/src/redux/statements/statementsSagas.ts index 85be877484a8..7000106fa789 100644 --- a/pkg/ui/workspaces/db-console/src/redux/statements/statementsSagas.ts +++ b/pkg/ui/workspaces/db-console/src/redux/statements/statementsSagas.ts @@ -23,9 +23,6 @@ import { import { invalidateStatementDiagnosticsRequests, refreshStatementDiagnosticsRequests, - invalidateStatements, - invalidateExecutionInsights, - invalidateTxnInsights, } from "src/redux/apiReducers"; import { createStatementDiagnosticsAlertLocalSetting, @@ -124,9 +121,6 @@ export function* setCombinedStatementsTimeScaleSaga( const ts = action.payload; yield put(setTimeScale(ts)); - yield put(invalidateStatements()); - yield put(invalidateExecutionInsights()); - yield put(invalidateTxnInsights()); } export function* statementsSaga() { diff --git a/pkg/ui/workspaces/db-console/src/redux/timeScale.ts b/pkg/ui/workspaces/db-console/src/redux/timeScale.ts index 8e7b3ddbd926..3bff1da8b9c4 100644 --- a/pkg/ui/workspaces/db-console/src/redux/timeScale.ts +++ b/pkg/ui/workspaces/db-console/src/redux/timeScale.ts @@ -14,7 +14,7 @@ */ import { Action } from "redux"; -import { put, takeEvery } from "redux-saga/effects"; +import { put, takeEvery, all } from "redux-saga/effects"; import { PayloadAction } from "src/interfaces/action"; import _ from "lodash"; import { defaultTimeScaleOptions, TimeScale } from "@cockroachlabs/cluster-ui"; @@ -25,6 +25,11 @@ import { getValueFromSessionStorage, setLocalSetting, } from "src/redux/localsettings"; +import { + invalidateExecutionInsights, + invalidateTxnInsights, + invalidateStatements, +} from "./apiReducers"; export const SET_SCALE = "cockroachui/timewindow/SET_SCALE"; export const SET_METRICS_MOVING_WINDOW = @@ -110,14 +115,17 @@ export function timeScaleReducer( } case SET_METRICS_MOVING_WINDOW: { const { payload: tw } = action as PayloadAction; - state = _.cloneDeep(state); + // We don't want to deep clone the state here, because we're + // not changing the scale object here. For components observing + // timescale changes, we don't want to update them unnecessarily. + state = { ...state, metricsTime: _.cloneDeep(state.metricsTime) }; state.metricsTime.currentWindow = tw; state.metricsTime.shouldUpdateMetricsWindowFromScale = false; return state; } case SET_METRICS_FIXED_WINDOW: { const { payload: data } = action as PayloadAction; - state = _.cloneDeep(state); + state = { ...state, metricsTime: _.cloneDeep(state.metricsTime) }; state.metricsTime.currentWindow = data; state.metricsTime.isFixedWindow = true; state.metricsTime.shouldUpdateMetricsWindowFromScale = false; @@ -222,5 +230,10 @@ export const adjustTimeScale = ( export function* timeScaleSaga() { yield takeEvery(SET_SCALE, function* ({ payload }: PayloadAction) { yield put(setLocalSetting(TIME_SCALE_SESSION_STORAGE_KEY, payload)); + yield all([ + put(invalidateStatements()), + put(invalidateExecutionInsights()), + put(invalidateTxnInsights()), + ]); }); } diff --git a/pkg/ui/workspaces/db-console/src/selectors/executionFingerprintsSelectors.tsx b/pkg/ui/workspaces/db-console/src/selectors/executionFingerprintsSelectors.tsx index 11e436555545..356f91025274 100644 --- a/pkg/ui/workspaces/db-console/src/selectors/executionFingerprintsSelectors.tsx +++ b/pkg/ui/workspaces/db-console/src/selectors/executionFingerprintsSelectors.tsx @@ -15,3 +15,6 @@ export const selectStatementsLastUpdated = (state: AdminUIState) => export const selectStatementsDataValid = (state: AdminUIState) => state.cachedData.statements?.valid; + +export const selectStatementsDataInFlight = (state: AdminUIState) => + state.cachedData.statements?.inFlight; diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx index e236038bf94c..83052686f871 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx @@ -614,7 +614,7 @@ function makeStateWithStatementsAndLastReset( }, }, localSettings: { - "timeScale/SQLActivity": timeScale, + [localStorage.GlOBAL_TIME_SCALE]: timeScale, }, timeScale: { scale: timeScale, diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx index c064d2be3ffe..d7cd2bafa47b 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx @@ -70,6 +70,7 @@ import { selectTimeScale } from "src/redux/timeScale"; import { selectStatementsLastUpdated, selectStatementsDataValid, + selectStatementsDataInFlight, } from "src/selectors/executionFingerprintsSelectors"; import { api as clusterUiApi } from "@cockroachlabs/cluster-ui"; @@ -380,6 +381,7 @@ export default withRouter( sortSetting: sortSettingLocalSetting.selector(state), statements: selectStatements(state, props), isDataValid: selectStatementsDataValid(state), + isReqInFlight: selectStatementsDataInFlight(state), lastUpdated: selectStatementsLastUpdated(state), statementsError: state.cachedData.statements.lastError, totalFingerprints: selectTotalFingerprints(state), diff --git a/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx b/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx index d28bbb933efe..f24e495452a6 100644 --- a/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx @@ -47,6 +47,7 @@ import { selectTimeScale } from "src/redux/timeScale"; import { selectStatementsLastUpdated, selectStatementsDataValid, + selectStatementsDataInFlight, } from "src/selectors/executionFingerprintsSelectors"; // selectStatements returns the array of AggregateStatistics to show on the @@ -151,6 +152,7 @@ const TransactionsPageConnected = withRouter( columns: transactionColumnsLocalSetting.selectorToArray(state), data: selectData(state), isDataValid: selectStatementsDataValid(state), + isReqInFlight: selectStatementsDataInFlight(state), lastUpdated: selectStatementsLastUpdated(state), timeScale: selectTimeScale(state), error: selectLastError(state),