Skip to content

Commit

Permalink
ui: ux improvements on stmt details page
Browse files Browse the repository at this point in the history
This commit adds a few improvements and bug fixes:

- Handles the case where we hit a
timeout on statement details, so it doesn't crash
anymore and you can still see the time picker to
be able to select a new time interval.

- Updates the error message, to
clarify it was a timeout error and increase the
timeout from 30s to 30m on the details endpoint.
Fixes cockroachdb#78979

- Updates the last error for statement
details with the proper value, which previously
was using the error for all statements endpoint,
instead of the specific for that fingerprint id.

- Adds a message when page takes longer to load.

- Uses a proper count formatting for
execution count.

Release justification: bug fixes and smaller improvements
Release note (ui change): Proper formatting of execution count
under Statement Details page.
Increase timeout for Statement Details page and shows
proper timeout error when it happens, no longer
crashing the page.
  • Loading branch information
maryliag committed Aug 31, 2022
1 parent cad5db6 commit 8114759
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,14 @@ const cx = classNames.bind(styles);

interface SQLActivityErrorProps {
statsType: string;
timeout?: boolean;
}

const SQLActivityError: React.FC<SQLActivityErrorProps> = props => {
const error = props.timeout ? "a timeout" : "an unexpected error";
return (
<div className={cx("row")}>
<span>
This page had an unexpected error while loading
{" " + props.statsType}.
</span>
<span>{`This page had ${error} while loading ${props.statsType}.`}</span>
&nbsp;
<a
className={cx("action")}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
TimestampToMoment,
RenderCount,
DATE_FORMAT_24_UTC,
Count,
} from "../../util";

export type PlanHashStats = cockroach.server.serverpb.StatementDetailsResponse.ICollectedStatementGroupedByPlanHash;
Expand Down Expand Up @@ -172,7 +173,7 @@ export function makeExplainPlanColumns(
{
name: "execCount",
title: planDetailsTableTitles.execCount(),
cell: (item: PlanHashStats) => longToInt(item.stats.count),
cell: (item: PlanHashStats) => Count(longToInt(item.stats.count)),
sort: (item: PlanHashStats) => longToInt(item.stats.count),
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const selectStatementDetails = createSelector(
): {
statementDetails: StatementDetailsResponseMessage;
isLoading: boolean;
lastError: Error;
} => {
// Since the aggregation interval is 1h, we want to round the selected timeScale to include
// the full hour. If a timeScale is between 14:32 - 15:17 we want to search for values
Expand All @@ -56,9 +57,10 @@ export const selectStatementDetails = createSelector(
return {
statementDetails: statementDetailsStatsData[key].data,
isLoading: statementDetailsStatsData[key].inFlight,
lastError: statementDetailsStatsData[key].lastError,
};
}
return { statementDetails: null, isLoading: true };
return { statementDetails: null, isLoading: true, lastError: null };
},
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { cockroach, google } from "@cockroachlabs/crdb-protobuf-client";
import { Text, InlineAlert } from "@cockroachlabs/ui-components";
import { ArrowLeft } from "@cockroachlabs/icons";
import { Location } from "history";
import _ from "lodash";
import _, { isNil } from "lodash";
import Long from "long";
import { Helmet } from "react-helmet";
import { Link, RouteComponentProps } from "react-router-dom";
Expand All @@ -34,7 +34,7 @@ import {
TimestampToMoment,
DATE_FORMAT_24_UTC,
} from "src/util";
import { Loading } from "src/loading";
import { getValidErrorsList, Loading } from "src/loading";
import { Button } from "src/button";
import { SqlBox, SqlBoxSize } from "src/sql";
import { SortSetting } from "src/sortedtable";
Expand Down Expand Up @@ -68,6 +68,8 @@ import {
generateRowsProcessedTimeseries,
generateContentionTimeseries,
} from "./timeseriesUtils";
import { Delayed } from "../delayed";
import moment from "moment";
type IDuration = google.protobuf.IDuration;
type StatementDetailsResponse = cockroach.server.serverpb.StatementDetailsResponse;
type IStatementDiagnosticsReport = cockroach.server.serverpb.IStatementDiagnosticsReport;
Expand Down Expand Up @@ -358,6 +360,22 @@ export class StatementDetails extends React.Component<
onDiagnosticsModalOpen,
} = this.props;
const app = queryByName(this.props.location, appAttr);
const longLoadingMessage = this.props.isLoading &&
isNil(this.props.statementDetails) &&
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 hasTimeout = this.props.statementsError?.name
?.toLowerCase()
.includes("timeout");
const error = hasTimeout ? null : this.props.statementsError;

return (
<div className={cx("root")}>
<Helmet title={`Details | ${app ? `${app} App |` : ""} Statements`} />
Expand All @@ -380,14 +398,15 @@ export class StatementDetails extends React.Component<
<Loading
loading={this.props.isLoading}
page={"statement fingerprint"}
error={this.props.statementsError}
error={error}
render={this.renderTabs}
renderError={() =>
SQLActivityError({
statsType: "statements",
})
}
/>
{longLoadingMessage}
<ActivateStatementDiagnosticsModal
ref={this.activateDiagnosticsRef}
activate={createStatementDiagnosticsReport}
Expand All @@ -401,8 +420,11 @@ export class StatementDetails extends React.Component<

renderTabs = (): React.ReactElement => {
const { currentTab } = this.state;
const { stats } = this.props.statementDetails.statement;
const hasData = Number(stats.count) > 0;
const hasTimeout = this.props.statementsError?.name
?.toLowerCase()
.includes("timeout");
const hasData =
Number(this.props.statementDetails?.statement?.stats?.count) > 0;
const period = timeScaleToString(this.props.timeScale);

return (
Expand All @@ -413,10 +435,10 @@ export class StatementDetails extends React.Component<
activeKey={currentTab}
>
<TabPane tab="Overview" key="overview">
{this.renderOverviewTabContent(hasData, period)}
{this.renderOverviewTabContent(hasTimeout, hasData, period)}
</TabPane>
<TabPane tab="Explain Plans" key="explain-plan">
{this.renderExplainPlanTabContent(hasData, period)}
{this.renderExplainPlanTabContent(hasTimeout, hasData, period)}
</TabPane>
{!this.props.isTenant && !this.props.hasViewActivityRedactedRole && (
<TabPane
Expand All @@ -440,7 +462,9 @@ export class StatementDetails extends React.Component<
</section>
);

renderNoDataWithTimeScaleAndSqlBoxTabContent = (): React.ReactElement => (
renderNoDataWithTimeScaleAndSqlBoxTabContent = (
hasTimeout: boolean,
): React.ReactElement => (
<>
<PageConfig>
<PageConfigItem>
Expand All @@ -462,20 +486,32 @@ export class StatementDetails extends React.Component<
</Col>
</Row>
)}
<InlineAlert
intent="info"
title="Data not available for this time frame. Select a different time frame."
/>
{hasTimeout && (
<InlineAlert
intent="danger"
title={SQLActivityError({
statsType: "statements",
timeout: true,
})}
/>
)}
{!hasTimeout && (
<InlineAlert
intent="info"
title="Data not available for this time frame. Select a different time frame."
/>
)}
</section>
</>
);

renderOverviewTabContent = (
hasTimeout: boolean,
hasData: boolean,
period: string,
): React.ReactElement => {
if (!hasData) {
return this.renderNoDataWithTimeScaleAndSqlBoxTabContent();
return this.renderNoDataWithTimeScaleAndSqlBoxTabContent(hasTimeout);
}
const { nodeRegions, isTenant } = this.props;
const { stats } = this.props.statementDetails.statement;
Expand Down Expand Up @@ -713,11 +749,12 @@ export class StatementDetails extends React.Component<
};

renderExplainPlanTabContent = (
hasTimeout: boolean,
hasData: boolean,
period: string,
): React.ReactElement => {
if (!hasData) {
return this.renderNoDataWithTimeScaleAndSqlBoxTabContent();
return this.renderNoDataWithTimeScaleAndSqlBoxTabContent(hasTimeout);
}
const { statement_statistics_per_plan_hash } = this.props.statementDetails;
const { formatted_query } = this.props.statementDetails.statement.metadata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,17 @@ const CancelStatementDiagnosticsReportRequest =
// For tenant cases, we don't show information about node, regions and
// diagnostics.
const mapStateToProps = (state: AppState, props: RouteComponentProps) => {
const { statementDetails, isLoading } = selectStatementDetails(state, props);
const { statementDetails, isLoading, lastError } = selectStatementDetails(
state,
props,
);
return {
statementFingerprintID: getMatchParamByName(props.match, statementAttr),
statementDetails,
isLoading: isLoading,
latestQuery: state.adminUI.sqlDetailsStats.latestQuery,
latestFormattedQuery: state.adminUI.sqlDetailsStats.latestFormattedQuery,
statementsError: state.adminUI.sqlStats.lastError,
statementsError: lastError,
timeScale: selectTimeScale(state),
nodeNames: selectIsTenant(state) ? {} : nodeDisplayNameByIDSelector(state),
nodeRegions: selectIsTenant(state) ? {} : nodeRegionsByIDSelector(state),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,9 @@ export class StatementsPage extends React.Component<
renderError={() =>
SQLActivityError({
statsType: "statements",
timeout: this.props.statementsError?.name
?.toLowerCase()
.includes("timeout"),
})
}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,9 @@ export class TransactionsPage extends React.Component<
renderError={() =>
SQLActivityError({
statsType: "transactions",
timeout: this.props?.error?.name
?.toLowerCase()
.includes("timeout"),
})
}
/>
Expand Down
1 change: 1 addition & 0 deletions pkg/ui/workspaces/db-console/src/redux/apiReducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ export const statementDetailsReducerObj = new KeyedCachedDataReducer(
statementDetailsActionNamespace,
statementDetailsRequestToID,
moment.duration(5, "m"),
moment.duration(30, "m"),
);

export const invalidateStatementDetails =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export const selectStatementDetails = createSelector(
): {
statementDetails: StatementDetailsResponseMessage;
isLoading: boolean;
lastError: Error;
} => {
// Since the aggregation interval is 1h, we want to round the selected timeScale to include
// the full hour. If a timeScale is between 14:32 - 15:17 we want to search for values
Expand All @@ -92,25 +93,29 @@ export const selectStatementDetails = createSelector(
return {
statementDetails: statementDetailsStats[key].data,
isLoading: statementDetailsStats[key].inFlight,
lastError: statementDetailsStats[key].lastError,
};
}
return { statementDetails: null, isLoading: true };
return { statementDetails: null, isLoading: true, lastError: null };
},
);

const mapStateToProps = (
state: AdminUIState,
props: RouteComponentProps,
): StatementDetailsStateProps => {
const { statementDetails, isLoading } = selectStatementDetails(state, props);
const { statementDetails, isLoading, lastError } = selectStatementDetails(
state,
props,
);
return {
statementFingerprintID: getMatchParamByName(props.match, statementAttr),
statementDetails,
isLoading: isLoading,
latestQuery: state.sqlActivity.statementDetailsLatestQuery,
latestFormattedQuery:
state.sqlActivity.statementDetailsLatestFormattedQuery,
statementsError: state.cachedData.statements.lastError,
statementsError: lastError,
timeScale: selectTimeScale(state),
nodeNames: nodeDisplayNameByIDSelector(state),
nodeRegions: nodeRegionsByIDSelector(state),
Expand Down

0 comments on commit 8114759

Please sign in to comment.