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),