Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: remove polling from fingerprints pages, allow new stats requests while one is in flight #98331

Merged
merged 5 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ import {
} from "src/util";
import Long from "long";
import { AggregateStatistics } from "../statementsTable";
const STATEMENTS_PATH = "/_status/statements";
const STATEMENTS_PATH = "/_status/combinedstmts";
const STATEMENT_DETAILS_PATH = "/_status/stmtdetails";

export type StatementsRequest = cockroach.server.serverpb.StatementsRequest;
export type StatementsRequest =
cockroach.server.serverpb.CombinedStatementsStatsRequest;
export type StatementDetailsRequest =
cockroach.server.serverpb.StatementDetailsRequest;
export type StatementDetailsResponse =
Expand All @@ -42,7 +43,6 @@ export const getCombinedStatements = (
const queryStr = propsToQueryString({
start: req.start.toInt(),
end: req.end.toInt(),
combined: req.combined,
});
return fetchData(
cockroach.server.serverpb.StatementsResponse,
Expand Down
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
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
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
135 changes: 43 additions & 92 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
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 @@ -78,12 +78,11 @@ import {
StatementDiagnosticsReport,
} from "../api";
import { filteredStatementsData } from "../sqlActivity/util";
import { STATS_LONG_LOADING_DURATION } from "../util/constants";

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 +94,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 +119,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,19 +141,15 @@ export interface StatementsPageState {
pagination: ISortedTablePagination;
filters?: Filters;
activeFilters?: number;
startRequest?: Date;
}

export type StatementsPageProps = StatementsPageDispatchProps &
StatementsPageStateProps &
RouteComponentProps<unknown>;

function stmtsRequestFromTimeScale(
ts: TimeScale,
): cockroach.server.serverpb.StatementsRequest {
function stmtsRequestFromTimeScale(ts: TimeScale): StatementsRequest {
const [start, end] = toRoundedDateRange(ts);
return new cockroach.server.serverpb.StatementsRequest({
combined: true,
return new cockroach.server.serverpb.CombinedStatementsStatsRequest({
start: Long.fromNumber(start.unix()),
end: Long.fromNumber(end.unix()),
});
Expand Down Expand Up @@ -187,7 +183,6 @@ export class StatementsPage extends React.Component<
StatementsPageState
> {
activateDiagnosticsRef: React.RefObject<ActivateDiagnosticsModalRef>;
refreshDataTimeout: NodeJS.Timeout;

constructor(props: StatementsPageProps) {
super(props);
Expand All @@ -196,19 +191,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 +253,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 +266,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 +336,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 +443,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 +453,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 +576,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={STATS_LONG_LOADING_DURATION}>
<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 +633,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 +646,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
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
Loading