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.

Sign up for GitHub

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: refactor txns pages to use generic RequestState #102029

Merged
merged 2 commits into from
May 8, 2023
Merged

ui: refactor txns pages to use generic RequestState #102029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cbe078a
ui: refactor txns page to use generic RequestState
xinhaoz Mar 22, 2023
43b9763
ui: refactor txn details page to use RequestState typed prop
xinhaoz Apr 20, 2023
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
2 changes: 1 addition & 1 deletion pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type StatementDetailsResponseWithKey = {
key: string;
};

export type SqlStatsResponse = cockroach.server.serverpb.StatementsResponse;
export type SqlStatsResponse = cockroach.server.serverpb.IStatementsResponse;
export const SqlStatsSortOptions = cockroach.server.serverpb.StatsSortOptions;
export type SqlStatsSortType = cockroach.server.serverpb.StatsSortOptions;

Expand Down
10 changes: 8 additions & 2 deletions pkg/ui/workspaces/cluster-ui/src/api/testUtils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import Long from "long";

export type Stmt =
cockroach.server.serverpb.StatementsResponse.ICollectedStatementStatistics;
type Txn =
export type Txn =
cockroach.server.serverpb.StatementsResponse.IExtendedCollectedTransactionStatistics;
type ILatencyInfo = cockroach.sql.ILatencyInfo;

Expand All @@ -27,6 +27,7 @@ const latencyInfo: Required<ILatencyInfo> = {

const baseStmt: Partial<Stmt> = {
id: Long.fromInt(11871906682067483964),
txn_fingerprint_ids: [Long.fromInt(1)],
key: {
key_data: {
query: "SELECT node_id FROM system.statement_statistics",
Expand Down Expand Up @@ -193,13 +194,18 @@ const assignObjectPropsIfExists = <T extends { [key: string]: unknown }>(
overrides: Partial<T>,
): T => {
const copiedObj: T = { ...baseObj };

for (const prop in baseObj) {
if (overrides[prop] === undefined) {
continue;
}

const val = copiedObj[prop];
if (typeof val === "object") {
if (
typeof val === "object" &&
!Array.isArray(val) &&
overrides[prop] != null
) {
copiedObj[prop] = assignObjectPropsIfExists(
val as Record<string, unknown>,
overrides[prop] as Record<string, unknown>,
Expand Down
26 changes: 7 additions & 19 deletions pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.reducer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,15 @@
import { createSlice, PayloadAction } from "@reduxjs/toolkit";
import { DOMAIN_NAME } from "../utils";
import { StatementsRequest } from "src/api/statementsApi";
import { TimeScale } from "../../timeScaleDropdown";
import moment from "moment-timezone";
import { StatementsResponse } from "../sqlStats";
import { createInitialState, RequestState } from "src/api/types";

export type TxnStatsState = {
// Note that we request transactions from the
// statements api, hence the StatementsResponse type here.
data: StatementsResponse;
inFlight: boolean;
lastError: Error;
valid: boolean;
lastUpdated: moment.Moment | null;
};
// Note that we request transactions from the
// statements api, hence the StatementsResponse type here.
export type TxnStatsState = RequestState<StatementsResponse>;

const initialState: TxnStatsState = {
data: null,
inFlight: false,
lastError: null,
valid: false,
lastUpdated: null,
};
const initialState = createInitialState<StatementsResponse>();

const txnStatsSlice = createSlice({
name: `${DOMAIN_NAME}/txnStats`,
Expand All @@ -41,13 +29,13 @@ const txnStatsSlice = createSlice({
state.inFlight = false;
state.data = action.payload;
state.valid = true;
state.lastError = null;
state.error = null;
state.lastUpdated = moment.utc();
},
failed: (state, action: PayloadAction<Error>) => {
state.inFlight = false;
state.valid = false;
state.lastError = action.payload;
state.error = action.payload;
state.lastUpdated = moment.utc();
},
invalidated: state => {
Expand Down
4 changes: 2 additions & 2 deletions pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe("txnStats sagas", () => {
.hasFinalState<TxnStatsState>({
inFlight: false,
data: txnStatsResponse,
lastError: null,
error: null,
valid: true,
lastUpdated,
})
Expand All @@ -91,7 +91,7 @@ describe("txnStats sagas", () => {
.hasFinalState<TxnStatsState>({
inFlight: false,
data: null,
lastError: error,
error: error,
valid: false,
lastUpdated,
})
Expand Down
17 changes: 0 additions & 17 deletions pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.selector.ts
View file
Open in desktop

This file was deleted.

109 changes: 52 additions & 57 deletions pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ import { baseHeadingClasses } from "../transactionsPage/transactionsPageClasses"
import { Button } from "../button";
import { tableClasses } from "../transactionsTable/transactionsTableClasses";
import { SqlBox } from "../sql";
import {
aggregateStatements,
statementFingerprintIdsToText,
} from "../transactionsPage/utils";
import { aggregateStatements } from "../transactionsPage/utils";
import { Loading } from "../loading";
import { SummaryCard, SummaryCardItem } from "../summaryCard";
import {
Expand Down Expand Up @@ -63,6 +60,8 @@ import Long from "long";
import {
createCombinedStmtsRequest,
InsightRecommendation,
RequestState,
SqlStatsResponse,
StatementsRequest,
TxnInsightsRequest,
} from "../api";
Expand All @@ -79,7 +78,6 @@ import {
timeScaleRangeToObj,
toRoundedDateRange,
} from "../timeScaleDropdown";
import moment from "moment-timezone";

import timeScaleStyles from "../timeScaleDropdown/timeScale.module.scss";
import insightTableStyles from "../insightsTable/insightsTable.module.scss";
Expand All @@ -90,6 +88,11 @@ import {
import { CockroachCloudContext } from "../contexts";
import { SqlStatsSortType } from "src/api/statementsApi";
import { FormattedTimescale } from "../timeScaleDropdown/formattedTimeScale";
import {
getStatementsForTransaction,
getTxnFromSqlStatsMemoized,
getTxnQueryString,
} from "./transactionDetailsUtils";
const { containerClass } = tableClasses;
const cx = classNames.bind(statementsStyles);
const timeScaleStylesCx = classNames.bind(timeScaleStyles);
Expand All @@ -105,18 +108,13 @@ export interface TransactionDetailsStateProps {
timeScale: TimeScale;
limit: number;
reqSortSetting: SqlStatsSortType;
error?: Error | null;
isTenant: UIConfigState["isTenant"];
hasViewActivityRedactedRole?: UIConfigState["hasViewActivityRedactedRole"];
nodeRegions: { [nodeId: string]: string };
statements?: Statement[];
transaction: Transaction;
transactionFingerprintId: string;
isLoading: boolean;
lastUpdated: moment.Moment | null;
transactionInsights: TxnInsightEvent[];
hasAdminRole?: UIConfigState["hasAdminRole"];
isDataValid: boolean;
txnStatsResp: RequestState<SqlStatsResponse>;
}

export interface TransactionDetailsDispatchProps {
Expand All @@ -135,6 +133,8 @@ interface TState {
sortSetting: SortSetting;
pagination: ISortedTablePagination;
latestTransactionText: string;
txnDetails: Transaction | null;
statements: Statement[] | null;
}

function statementsRequestFromProps(
Expand All @@ -155,6 +155,18 @@ export class TransactionDetails extends React.Component<
> {
constructor(props: TransactionDetailsProps) {
super(props);

const txnDetails = getTxnFromSqlStatsMemoized(
this.props.txnStatsResp?.data,
this.props.match,
this.props.location,
);

const stmts = getStatementsForTransaction(
txnDetails,
this.props.txnStatsResp?.data?.statements,
);

this.state = {
sortSetting: {
// Sort by statement latency as default column.
Expand All @@ -165,7 +177,9 @@ export class TransactionDetails extends React.Component<
pageSize: 10,
current: 1,
},
latestTransactionText: this.getTxnQueryString(),
latestTransactionText: getTxnQueryString(txnDetails, stmts),
txnDetails,
statements: stmts,
};

// In case the user selected a option not available on this page,
Expand All @@ -177,33 +191,33 @@ export class TransactionDetails extends React.Component<
}
}

getTxnQueryString = (): string => {
const { transaction } = this.props;

const statementFingerprintIds =
transaction?.stats_data?.statement_fingerprint_ids;
setTxnDetails = (): void => {
const txnDetails = getTxnFromSqlStatsMemoized(
this.props.txnStatsResp?.data,
this.props.match,
this.props.location,
);

return (
(statementFingerprintIds &&
statementFingerprintIdsToText(
statementFingerprintIds,
this.getStatementsForTransaction(),
)) ??
""
const statements = getStatementsForTransaction(
txnDetails,
this.props.txnStatsResp?.data?.statements,
);
};

setTxnQueryString = (): void => {
const transactionText = this.getTxnQueryString();
// Only overwrite the transaction text if it is non-null.
const transactionText =
getTxnQueryString(txnDetails, statements) ??
this.state.latestTransactionText;

// If a new, non-empty-string transaction text is available (derived from the time-frame-specific endpoint
// response), cache the text.
if (
transactionText &&
transactionText !== this.state.latestTransactionText
transactionText !== this.state.latestTransactionText ||
txnDetails !== this.state.txnDetails
) {
this.setState({
latestTransactionText: transactionText,
txnDetails,
statements,
});
}
};
Expand All @@ -222,7 +236,7 @@ export class TransactionDetails extends React.Component<
};

componentDidMount(): void {
if (!this.props.transaction || !this.props.isDataValid) {
if (!this.props.txnStatsResp?.data || !this.props.txnStatsResp?.valid) {
this.refreshData();
}
this.props.refreshUserSQLRoles();
Expand All @@ -239,9 +253,9 @@ export class TransactionDetails extends React.Component<
if (
prevProps.transactionFingerprintId !==
this.props.transactionFingerprintId ||
prevProps.statements !== this.props.statements
prevProps.txnStatsResp !== this.props.txnStatsResp
) {
this.setTxnQueryString();
this.setTxnDetails();
}

if (this.props.timeScale !== prevProps.timeScale) {
Expand All @@ -265,28 +279,11 @@ export class TransactionDetails extends React.Component<
this.props.history.push("/sql-activity?tab=Transactions&view=fingerprints");
};

getStatementsForTransaction = (): Statement[] => {
const { transaction, statements } = this.props;

const statementFingerprintIds =
transaction?.stats_data?.statement_fingerprint_ids;
if (!statementFingerprintIds) {
return [];
}

return (
statements?.filter(
s =>
s.key.key_data.transaction_fingerprint_id.toString() ===
this.props.transactionFingerprintId,
) ?? []
);
};

render(): React.ReactElement {
const { error, nodeRegions, transaction } = this.props;
const { latestTransactionText } = this.state;
const statementsForTransaction = this.getStatementsForTransaction();
const { nodeRegions } = this.props;
const transaction = this.state.txnDetails;
const error = this.props.txnStatsResp?.error;
const { latestTransactionText, statements } = this.state;
const transactionStats = transaction?.stats_data?.stats;
const period = <FormattedTimescale ts={this.props.timeScale} />;

Expand Down Expand Up @@ -324,7 +321,7 @@ export class TransactionDetails extends React.Component<
<Loading
error={error}
page={"transaction details"}
loading={this.props.isLoading}
loading={this.props.txnStatsResp?.inFlight}
render={() => {
if (!transaction) {
return (
Expand Down Expand Up @@ -358,9 +355,7 @@ export class TransactionDetails extends React.Component<
} = this.props;
const { sortSetting, pagination } = this.state;

const aggregatedStatements = aggregateStatements(
statementsForTransaction,
);
const aggregatedStatements = aggregateStatements(statements);
populateRegionNodeForStatements(aggregatedStatements, nodeRegions);
const duration = (v: number) => Duration(v * 1e9);

Expand Down
Loading