Skip to content

Commit b3c64ee

Browse files
craig[bot]maryliagjeffswenson
committed
106623: ui: fix timescale for rolling window r=maryliag a=maryliag The change introduced on cockroachdb#105157 had an undesired change on the Metrics page. We want to show the end period of the select, but when the fixed window end for that this caused the Metrics page to stop loading new data. We want the metrics page to keep updating when any of the rolling windows is selected, and we also want to know the time a time period was requested, so it can be used to provide more information on SQL Activity pages. This commit remove the setting of the fixedWindowEnd, since that was not the best approach and instead creates a value on local storage for the requestTime, that can be used to create the label for the timescale, without interferring on Metrics page (or any other that have an automatic update). Epic: none Release note (bug fix): Fix the Metrics page that was not updating automatically on rolling window options. Release note (bug fix): Statement Diagnostics not always showing is now fixed and they show for the correct time period selected. 106757: upgrademanager: skip TestMigrationFailure r=JeffSwenson a=JeffSwenson Skip TestMigrationFailure until the source of flakes can be fixed. Release note: None Part of: cockroachdb#106648 Co-authored-by: maryliag <[email protected]> Co-authored-by: Jeff <[email protected]>
3 parents c7aed80 + 51f5a93 + 853fae0 commit b3c64ee

21 files changed

+133
-28
lines changed

pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.spec.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ import { TestStoreProvider } from "src/test-utils";
1919
import { StatementDiagnosticsReport } from "../../api";
2020
import moment from "moment-timezone";
2121
import { SortedTable } from "src/sortedtable";
22+
import { TimeScale } from "src/timeScaleDropdown";
2223

2324
const activateDiagnosticsRef = { current: { showModalFor: jest.fn() } };
24-
const ts = {
25+
const ts: TimeScale = {
2526
windowSize: moment.duration(20, "day"),
2627
sampleSize: moment.duration(5, "minutes"),
2728
fixedWindowEnd: moment.utc("2023.01.5"),

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,7 @@ export const getStatementDetailsPropsFixture = (
851851
"3": "gcp-us-west1",
852852
"4": "gcp-europe-west1",
853853
},
854+
requestTime: moment.utc("2021.12.12"),
854855
refreshStatementDetails: noop,
855856
refreshStatementDiagnosticsRequests: noop,
856857
refreshNodes: noop,
@@ -860,6 +861,7 @@ export const getStatementDetailsPropsFixture = (
860861
diagnosticsReports: [],
861862
dismissStatementDiagnosticsAlertMessage: noop,
862863
onTimeScaleChange: noop,
864+
onRequestTimeChange: noop,
863865
createStatementDiagnosticsReport: noop,
864866
uiConfig: {
865867
showStatementDiagnosticsLink: true,

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ export interface StatementDetailsDispatchProps {
144144
ascending: boolean,
145145
) => void;
146146
onBackToStatementsClick?: () => void;
147+
onRequestTimeChange: (t: moment.Moment) => void;
147148
}
148149

149150
export interface StatementDetailsStateProps {
@@ -160,6 +161,7 @@ export interface StatementDetailsStateProps {
160161
hasViewActivityRedactedRole?: UIConfigState["hasViewActivityRedactedRole"];
161162
hasAdminRole?: UIConfigState["hasAdminRole"];
162163
statementFingerprintInsights?: StmtInsightEvent[];
164+
requestTime: moment.Moment;
163165
}
164166

165167
export type StatementDetailsOwnProps = StatementDetailsDispatchProps &
@@ -247,12 +249,10 @@ export class StatementDetails extends React.Component<
247249
this.props.diagnosticsReports.length > 0;
248250

249251
changeTimeScale = (ts: TimeScale): void => {
250-
if (ts.key !== "Custom") {
251-
ts.fixedWindowEnd = moment();
252-
}
253252
if (this.props.onTimeScaleChange) {
254253
this.props.onTimeScaleChange(ts);
255254
}
255+
this.props.onRequestTimeChange(moment());
256256
};
257257

258258
refreshStatementDetails = (): void => {
@@ -701,14 +701,17 @@ export class StatementDetails extends React.Component<
701701
<TimeScaleDropdown
702702
options={timeScale1hMinOptions}
703703
currentScale={this.props.timeScale}
704-
setTimeScale={this.props.onTimeScaleChange}
704+
setTimeScale={this.changeTimeScale}
705705
/>
706706
</PageConfigItem>
707707
</PageConfig>
708708
<p className={timeScaleStylesCx("time-label", "label-margin")}>
709709
Showing aggregated stats from{" "}
710710
<span className={timeScaleStylesCx("bold")}>
711-
<FormattedTimescale ts={this.props.timeScale} />
711+
<FormattedTimescale
712+
ts={this.props.timeScale}
713+
requestTime={moment(this.props.requestTime)}
714+
/>
712715
</span>
713716
</p>
714717
<section className={cx("section")}>
@@ -899,7 +902,10 @@ export class StatementDetails extends React.Component<
899902
<p className={timeScaleStylesCx("time-label", "label-margin")}>
900903
Showing explain plans from{" "}
901904
<span className={timeScaleStylesCx("bold")}>
902-
<FormattedTimescale ts={this.props.timeScale} />
905+
<FormattedTimescale
906+
ts={this.props.timeScale}
907+
requestTime={moment(this.props.requestTime)}
908+
/>
903909
</span>
904910
</p>
905911
<section className={cx("section")}>

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetailsConnected.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
} from "src/api";
5151
import { TimeScale } from "../timeScaleDropdown";
5252
import { getMatchParamByName, statementAttr } from "src/util";
53+
import { selectRequestTime } from "src/statementsPage/statementsPage.selectors";
5354

5455
// For tenant cases, we don't show information about node, regions and
5556
// diagnostics.
@@ -76,6 +77,7 @@ const mapStateToProps = (state: AppState, props: RouteComponentProps) => {
7677
isTenant: selectIsTenant(state),
7778
hasViewActivityRedactedRole: selectHasViewActivityRedactedRole(state),
7879
hasAdminRole: selectHasAdminRole(state),
80+
requestTime: selectRequestTime(state),
7981
statementFingerprintInsights: selectStatementFingerprintInsights(
8082
state,
8183
props,
@@ -169,6 +171,14 @@ const mapDispatchToProps = (
169171
tableName,
170172
}),
171173
),
174+
onRequestTimeChange: (t: moment.Moment) => {
175+
dispatch(
176+
localStorageActions.update({
177+
key: "requestTime/StatementsPage",
178+
value: t,
179+
}),
180+
);
181+
},
172182
onBackToStatementsClick: () =>
173183
dispatch(
174184
analyticsActions.track({

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
355355
ascending: false,
356356
columnTitle: "executionCount",
357357
},
358+
requestTime: moment.utc("2021.12.12"),
358359
search: "",
359360
filters: {
360361
app: "",
@@ -380,6 +381,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
380381
isTenant: false,
381382
hasViewActivityRedactedRole: false,
382383
hasAdminRole: true,
384+
statementDiagnostics: [],
383385
dismissAlertMessage: noop,
384386
refreshDatabases: noop,
385387
refreshStatementDiagnosticsRequests: noop,
@@ -398,7 +400,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
398400
onChangeLimit: noop,
399401
onChangeReqSort: noop,
400402
onApplySearchCriteria: noop,
401-
statementDiagnostics: [],
403+
onRequestTimeChange: noop,
402404
};
403405

404406
export const statementsPagePropsWithRequestError: StatementsPageProps = {

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ export const selectSortSetting = createSelector(
3838
localStorage => localStorage["sortSetting/StatementsPage"],
3939
);
4040

41+
export const selectRequestTime = createSelector(
42+
localStorageSelector,
43+
localStorage => localStorage["requestTime/StatementsPage"],
44+
);
45+
4146
export const selectFilters = createSelector(
4247
localStorageSelector,
4348
localStorage => localStorage["filters/StatementsPage"],

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ export interface StatementsPageDispatchProps {
135135
onChangeLimit: (limit: number) => void;
136136
onChangeReqSort: (sort: SqlStatsSortType) => void;
137137
onApplySearchCriteria: (ts: TimeScale, limit: number, sort: string) => void;
138+
onRequestTimeChange: (t: moment.Moment) => void;
138139
}
139140
export interface StatementsPageStateProps {
140141
statementsResponse: RequestState<SqlStatsResponse>;
@@ -152,6 +153,7 @@ export interface StatementsPageStateProps {
152153
hasAdminRole?: UIConfigState["hasAdminRole"];
153154
stmtsTotalRuntimeSecs: number;
154155
statementDiagnostics: StatementDiagnosticsResponse | null;
156+
requestTime: moment.Moment;
155157
}
156158

157159
export interface StatementsPageState {
@@ -264,9 +266,6 @@ export class StatementsPage extends React.Component<
264266
};
265267

266268
changeTimeScale = (ts: TimeScale): void => {
267-
if (ts.key !== "Custom") {
268-
ts.fixedWindowEnd = moment();
269-
}
270269
this.setState(prevState => ({
271270
...prevState,
272271
timeScale: ts,
@@ -282,8 +281,6 @@ export class StatementsPage extends React.Component<
282281
this.props.onChangeReqSort(this.state.reqSortSetting);
283282
}
284283

285-
// Force an update on TimeScale to update the fixedWindowEnd
286-
this.changeTimeScale(this.state.timeScale);
287284
if (this.props.timeScale !== this.state.timeScale) {
288285
this.props.onTimeScaleChange(this.state.timeScale);
289286
}
@@ -294,6 +291,7 @@ export class StatementsPage extends React.Component<
294291
getSortLabel(this.state.reqSortSetting, "Statement"),
295292
);
296293
}
294+
this.props.onRequestTimeChange(moment());
297295
this.refreshStatements();
298296
const ss: SortSetting = {
299297
ascending: false,
@@ -588,7 +586,12 @@ export class StatementsPage extends React.Component<
588586
isSelectedColumn(userSelectedColumnsToShow, c),
589587
);
590588

591-
const period = <FormattedTimescale ts={this.props.timeScale} />;
589+
const period = (
590+
<FormattedTimescale
591+
ts={this.props.timeScale}
592+
requestTime={moment(this.props.requestTime)}
593+
/>
594+
);
592595
const sortSettingLabel = getSortLabel(
593596
this.props.reqSortSetting,
594597
"Statement",

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
selectSortSetting,
3434
selectFilters,
3535
selectSearch,
36+
selectRequestTime,
3637
} from "./statementsPage.selectors";
3738
import {
3839
selectTimeScale,
@@ -96,6 +97,7 @@ export const ConnectedStatementsPage = withRouter(
9697
search: selectSearch(state),
9798
sortSetting: selectSortSetting(state),
9899
limit: selectStmtsPageLimit(state),
100+
requestTime: selectRequestTime(state),
99101
reqSortSetting: selectStmtsPageReqSort(state),
100102
stmtsTotalRuntimeSecs:
101103
state.adminUI?.statements?.data?.stmts_total_runtime_secs ?? 0,
@@ -221,6 +223,14 @@ export const ConnectedStatementsPage = withRouter(
221223
}),
222224
);
223225
},
226+
onRequestTimeChange: (t: moment.Moment) => {
227+
dispatch(
228+
localStorageActions.update({
229+
key: "requestTime/StatementsPage",
230+
value: t,
231+
}),
232+
);
233+
},
224234
onStatementClick: () =>
225235
dispatch(
226236
analyticsActions.track({

pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ export type LocalStorageState = {
8484
"showSetting/JobsPage": string;
8585
[LocalStorageKeys.DB_DETAILS_VIEW_MODE]: ViewMode;
8686
[LocalStorageKeys.ACTIVE_EXECUTIONS_IS_AUTOREFRESH_ENABLED]: boolean;
87+
"requestTime/StatementsPage": moment.Moment;
88+
"requestTime/TransactionsPage": moment.Moment;
8789
};
8890

8991
type Payload = {
@@ -283,6 +285,8 @@ const initialState: LocalStorageState = {
283285
LocalStorageKeys.ACTIVE_EXECUTIONS_IS_AUTOREFRESH_ENABLED,
284286
),
285287
) || defaultIsAutoRefreshEnabledSetting,
288+
"requestTime/StatementsPage": null,
289+
"requestTime/TransactionsPage": null,
286290
};
287291

288292
const localStorageSlice = createSlice({

pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/formattedTimeScale.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import { toRoundedDateRange } from "./utils";
1616
import { TimeScale } from "./timeScaleTypes";
1717
import { Timezone } from "src/timestamp";
1818

19-
export const FormattedTimescale = (props: { ts: TimeScale }) => {
19+
export const FormattedTimescale = (props: {
20+
ts: TimeScale;
21+
requestTime?: moment.Moment;
22+
}) => {
2023
const timezone = useContext(TimezoneContext);
2124

2225
const [start, end] = toRoundedDateRange(props.ts);
@@ -30,8 +33,8 @@ export const FormattedTimescale = (props: { ts: TimeScale }) => {
3033
omitDayFormat || startEndOnSameDay ? "" : endTz.format(dateFormat);
3134
const timeStart = startTz.format(timeFormat);
3235
const timeEnd =
33-
props.ts.key !== "Custom" && props.ts.fixedWindowEnd
34-
? props.ts.fixedWindowEnd.tz(timezone).format(timeFormat)
36+
props.ts.key !== "Custom" && props.requestTime?.isValid()
37+
? props.requestTime.tz(timezone).format(timeFormat)
3538
: endTz.format(timeFormat);
3639

3740
return (

0 commit comments

Comments
 (0)