Skip to content

Commit

Permalink
Merge #102029
Browse files Browse the repository at this point in the history 
102029: ui: refactor txns pages to use generic RequestState r=maryliag a=xinhaoz

### Commit 1
ui: refactor txns page to use generic RequestState

This commit refactors the txn fingerprints page component
and its redux store wrappers to use the new RequestState.
This allows us to nicely pass the entire request state to
the component instead of destructuring the response into
individual props. Related selectors are deleted.

Epic: none

Release note: None

### Commit 2
ui: refactor txn details page to use RequestState typed prop

This commit Replaces the request state props (isDataValid, statements,
transaction, error, isLoading) in the txn details page with a single
prop containing the request state.

The logic that was previously in the selectTransaction (and being
duplicated in db-console and CC) has been moved to a new file,
`transactionDetailsUtils` with added unit testing.

Epic: none

Release note: None

Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
  • Loading branch information
@xinhaoz
craig[bot] and xinhaoz committed May 8, 2023
2 parents a195cc1 + 43b9763 commit 50b70df
Show file tree
Hide file tree
Showing 17 changed files with 537 additions and 387 deletions.
2 changes: 1 addition & 1 deletion pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts
View file
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
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
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
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

This file was deleted.

109 changes: 52 additions & 57 deletions pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx
View file
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

0 comments on commit 50b70df

Please sign in to comment.