From 3b7333685a0dc68309da06df3e3a0e389b2f85e0 Mon Sep 17 00:00:00 2001 From: Xin Hao Zhang Date: Thu, 16 Mar 2023 10:49:01 -0400 Subject: [PATCH] server, ui: add % runtime as a top-k sort for sql activity pages Previously, the calculatation for the `% of runtime` column for the sql activity pages, was performed on the client app by summing the (execCount* avgAvcLat) from all statements returned. This was okay when we were returning a large number of stmts (20,000) but now that we have reduced that limit significantly (default 100, with a max provided option of 500), performing this calculation on the client side won't give us the full picture of total runtime of the workload in the requested time interval. This commit modifies the statements API to return the total estimated runtime of the workload so that we can continue to show the '% of runtime` column. We also now provide this as a server sort option, so that users can request the top-k stmts or txns by % of runtime. Epic: none Release note (ui change): Users can request top-k stmts by % runtime in sql activity fingerprints pages. --- docs/generated/http/full.md | 6 + pkg/server/combined_statement_stats.go | 105 ++++++++++++++++++ pkg/server/serverpb/status.proto | 5 + .../cluster-ui/src/api/statementsApi.ts | 10 +- .../cluster-ui/src/barCharts/barCharts.tsx | 9 +- .../statementsPage/statementsPage.fixture.ts | 1 + .../src/statementsPage/statementsPage.tsx | 12 +- .../statementsPageConnected.tsx | 2 + .../transactionStats/txnStats.sagas.spec.ts | 1 + .../transactionDetails.fixture.ts | 2 + .../src/util/sqlActivityConstants.ts | 6 +- .../src/views/statements/statementsPage.tsx | 2 + 12 files changed, 142 insertions(+), 19 deletions(-) diff --git a/docs/generated/http/full.md b/docs/generated/http/full.md index 34f1c78a9fd8..d149793676aa 100644 --- a/docs/generated/http/full.md +++ b/docs/generated/http/full.md @@ -3982,6 +3982,8 @@ tenant pods. | last_reset | [google.protobuf.Timestamp](#cockroach.server.serverpb.StatementsResponse-google.protobuf.Timestamp) | | Timestamp of the last stats reset. | [reserved](#support-status) | | internal_app_name_prefix | [string](#cockroach.server.serverpb.StatementsResponse-string) | | If set and non-empty, indicates the prefix to application_name used for statements/queries issued internally by CockroachDB. | [reserved](#support-status) | | transactions | [StatementsResponse.ExtendedCollectedTransactionStatistics](#cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.ExtendedCollectedTransactionStatistics) | repeated | Transactions is transaction-level statistics for the collection of statements in this response. | [reserved](#support-status) | +| stmts_total_runtime_secs | [float](#cockroach.server.serverpb.StatementsResponse-float) | | | [reserved](#support-status) | +| txns_total_runtime_secs | [float](#cockroach.server.serverpb.StatementsResponse-float) | | | [reserved](#support-status) | @@ -4055,6 +4057,7 @@ Support status: [reserved](#support-status) | start | [int64](#cockroach.server.serverpb.CombinedStatementsStatsRequest-int64) | | Unix time range for aggregated statements. | [reserved](#support-status) | | end | [int64](#cockroach.server.serverpb.CombinedStatementsStatsRequest-int64) | | | [reserved](#support-status) | | fetch_mode | [CombinedStatementsStatsRequest.FetchMode](#cockroach.server.serverpb.CombinedStatementsStatsRequest-cockroach.server.serverpb.CombinedStatementsStatsRequest.FetchMode) | | Note that if fetch_mode is set to transactions only, we will also include the statement statistics for the stmts in the transactions response. This is more of a hack-y method to get the complete stats for txns, because in the client we need to fill in some txn stats info from its stmt stats, such as the query string.

We prefer this hackier method right now to reduce surface area for backporting these changes, but in the future we will introduce more endpoints to properly organize these differing requests. TODO (xinhaoz) - Split this API into stmts and txns properly instead of using this param. | [reserved](#support-status) | +| limit | [int64](#cockroach.server.serverpb.CombinedStatementsStatsRequest-int64) | | | [reserved](#support-status) | @@ -4069,6 +4072,7 @@ Support status: [reserved](#support-status) | Field | Type | Label | Description | Support status | | ----- | ---- | ----- | ----------- | -------------- | | stats_type | [CombinedStatementsStatsRequest.StatsType](#cockroach.server.serverpb.CombinedStatementsStatsRequest-cockroach.server.serverpb.CombinedStatementsStatsRequest.StatsType) | | | [reserved](#support-status) | +| sort | [StatsSortOptions](#cockroach.server.serverpb.CombinedStatementsStatsRequest-cockroach.server.serverpb.StatsSortOptions) | | | [reserved](#support-status) | @@ -4089,6 +4093,8 @@ Support status: [reserved](#support-status) | last_reset | [google.protobuf.Timestamp](#cockroach.server.serverpb.StatementsResponse-google.protobuf.Timestamp) | | Timestamp of the last stats reset. | [reserved](#support-status) | | internal_app_name_prefix | [string](#cockroach.server.serverpb.StatementsResponse-string) | | If set and non-empty, indicates the prefix to application_name used for statements/queries issued internally by CockroachDB. | [reserved](#support-status) | | transactions | [StatementsResponse.ExtendedCollectedTransactionStatistics](#cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.ExtendedCollectedTransactionStatistics) | repeated | Transactions is transaction-level statistics for the collection of statements in this response. | [reserved](#support-status) | +| stmts_total_runtime_secs | [float](#cockroach.server.serverpb.StatementsResponse-float) | | | [reserved](#support-status) | +| txns_total_runtime_secs | [float](#cockroach.server.serverpb.StatementsResponse-float) | | | [reserved](#support-status) | diff --git a/pkg/server/combined_statement_stats.go b/pkg/server/combined_statement_stats.go index d8201790df11..3f23bf1e7521 100644 --- a/pkg/server/combined_statement_stats.go +++ b/pkg/server/combined_statement_stats.go @@ -98,21 +98,124 @@ func getCombinedStatementStats( return nil, serverError(ctx, err) } + stmtsRunTime, txnsRunTime, err := getTotalRuntimeSecs(ctx, req, ie, testingKnobs) + + if err != nil { + return nil, serverError(ctx, err) + } + response := &serverpb.StatementsResponse{ Statements: statements, Transactions: transactions, LastReset: statsProvider.GetLastReset(), InternalAppNamePrefix: catconstants.InternalAppNamePrefix, + StmtsTotalRuntimeSecs: stmtsRunTime, + TxnsTotalRuntimeSecs: txnsRunTime, } return response, nil } +func getTotalRuntimeSecs( + ctx context.Context, + req *serverpb.CombinedStatementsStatsRequest, + ie *sql.InternalExecutor, + testingKnobs *sqlstats.TestingKnobs, +) (stmtsRuntime float32, txnsRuntime float32, err error) { + var buffer strings.Builder + buffer.WriteString(testingKnobs.GetAOSTClause()) + var args []interface{} + startTime := getTimeFromSeconds(req.Start) + endTime := getTimeFromSeconds(req.End) + + buffer.WriteString(" WHERE true") + + if startTime != nil { + args = append(args, *startTime) + buffer.WriteString(fmt.Sprintf(" AND aggregated_ts >= $%d", len(args))) + } + + if endTime != nil { + args = append(args, *endTime) + buffer.WriteString(fmt.Sprintf(" AND aggregated_ts <= $%d", len(args))) + } + + whereClause := buffer.String() + + queryWithPlaceholders := ` +SELECT +COALESCE( + sum( + (statistics -> 'statistics' -> 'svcLat' ->> 'mean')::FLOAT * + (statistics-> 'statistics' ->> 'cnt')::FLOAT + ) +, 0) +FROM crdb_internal.%s_statistics_persisted +%s +` + + getRuntime := func(table string) (float32, error) { + it, err := ie.QueryIteratorEx( + ctx, + fmt.Sprintf(`%s-total-runtime`, table), + nil, + sessiondata.NodeUserSessionDataOverride, + fmt.Sprintf(queryWithPlaceholders, table, whereClause), + args...) + + if err != nil { + return 0, err + } + + defer func() { + closeErr := it.Close() + if closeErr != nil { + err = errors.CombineErrors(err, closeErr) + } + }() + + ok, err := it.Next(ctx) + if err != nil { + return 0, err + } + + if !ok { + return 0, errors.New("expected one row but got none") + } + + var row tree.Datums + if row = it.Cur(); row == nil { + return 0, errors.New("unexpected null row") + } + + return float32(tree.MustBeDFloat(row[0])), nil + + } + + if req.FetchMode == nil || req.FetchMode.StatsType != serverpb.CombinedStatementsStatsRequest_TxnStatsOnly { + stmtsRuntime, err = getRuntime("statement") + if err != nil { + return 0, 0, err + } + } + + if req.FetchMode == nil || req.FetchMode.StatsType != serverpb.CombinedStatementsStatsRequest_StmtStatsOnly { + txnsRuntime, err = getRuntime("transaction") + if err != nil { + return 0, 0, err + } + } + + return stmtsRuntime, txnsRuntime, err +} + // Common stmt and txn columns to sort on. const ( sortSvcLatDesc = `(statistics -> 'statistics' -> 'svcLat' ->> 'mean')::FLOAT DESC` sortExecCountDesc = `(statistics -> 'statistics' ->> 'cnt')::INT DESC` sortContentionTimeDesc = `(statistics -> 'execution_statistics' -> 'contentionTime' ->> 'mean')::FLOAT DESC` + sortPCTRuntimeDesc = `((statistics -> 'statistics' -> 'svcLat' ->> 'mean')::FLOAT * + (statistics -> 'statistics' ->> 'cnt')::FLOAT) DESC` ) func getStmtColumnFromSortOption(sort serverpb.StatsSortOptions) string { @@ -136,6 +239,8 @@ func getTxnColumnFromSortOption(sort serverpb.StatsSortOptions) string { return sortExecCountDesc case serverpb.StatsSortOptions_CONTENTION_TIME: return sortContentionTimeDesc + case serverpb.StatsSortOptions_PCT_RUNTIME: + return sortPCTRuntimeDesc default: return sortSvcLatDesc } diff --git a/pkg/server/serverpb/status.proto b/pkg/server/serverpb/status.proto index 52ad8c20fb7a..ee878a444087 100644 --- a/pkg/server/serverpb/status.proto +++ b/pkg/server/serverpb/status.proto @@ -1560,6 +1560,10 @@ message StatementsResponse { // Transactions is transaction-level statistics for the collection of // statements in this response. repeated ExtendedCollectedTransactionStatistics transactions = 5 [(gogoproto.nullable) = false]; + + float stmts_total_runtime_secs = 6; + + float txns_total_runtime_secs = 7; } enum StatsSortOptions { @@ -1568,6 +1572,7 @@ enum StatsSortOptions { EXECUTION_COUNT = 2; reserved 3; // This is for P99 in 23.1 CONTENTION_TIME = 4; + PCT_RUNTIME = 5; } message CombinedStatementsStatsRequest { enum StatsType { diff --git a/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts index d103e32c1b77..ea8768241016 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts @@ -78,8 +78,8 @@ export const getCombinedStatements = ( start: req.start.toInt(), end: req.end.toInt(), "fetch_mode.stats_type": FetchStatsMode.StmtStatsOnly, - "fetch_mode.sort": req.fetch_mode.sort, - limit: req.limit.toInt(), + "fetch_mode.sort": req.fetch_mode?.sort, + limit: req.limit?.toInt() ?? DEFAULT_STATS_REQ_OPTIONS.limit, }); return fetchData( cockroach.server.serverpb.StatementsResponse, @@ -94,11 +94,11 @@ export const getFlushedTxnStatsApi = ( req: StatementsRequest, ): Promise => { const queryStr = propsToQueryString({ - start: req.start.toInt(), - end: req.end.toInt(), + start: req.start?.toInt(), + end: req.end?.toInt(), "fetch_mode.stats_type": FetchStatsMode.TxnStatsOnly, "fetch_mode.sort": req.fetch_mode?.sort, - limit: req.limit.toInt() ?? DEFAULT_STATS_REQ_OPTIONS.limit, + limit: req.limit?.toInt() ?? DEFAULT_STATS_REQ_OPTIONS.limit, }); return fetchData( cockroach.server.serverpb.StatementsResponse, diff --git a/pkg/ui/workspaces/cluster-ui/src/barCharts/barCharts.tsx b/pkg/ui/workspaces/cluster-ui/src/barCharts/barCharts.tsx index 4d73cdac519e..36800cb0638a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/barCharts/barCharts.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/barCharts/barCharts.tsx @@ -133,10 +133,11 @@ export function workloadPctBarChart( return barChartFactory( "grey", [ - bar( - "pct-workload", - (d: StatementStatistics) => - (d.stats.service_lat.mean * longToInt(d.stats.count)) / totalWorkload, + bar("pct-workload", (d: StatementStatistics) => + totalWorkload !== 0 + ? (d.stats.service_lat.mean * longToInt(d.stats.count)) / + totalWorkload + : 0, ), ], v => Percentage(v, 1, 1), diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts index 21a53658b6a1..394f7b8e4541 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts @@ -251,6 +251,7 @@ const aggregatedTs = Date.parse("Sep 15 2021 01:00:00 GMT") * 1e-3; const lastUpdated = moment("Sep 15 2021 01:30:00 GMT"); const statementsPagePropsFixture: StatementsPageProps = { + stmtsTotalRuntimeSecs: 100, history, location: { pathname: "/statements", diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 8edc5313593b..c29f73bea357 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -33,13 +33,7 @@ import { updateFiltersQueryParamsOnTab, } from "../queryFilter"; -import { - calculateTotalWorkload, - containAny, - syncHistory, - unique, - unset, -} from "src/util"; +import { containAny, syncHistory, unique, unset } from "src/util"; import { AggregateStatistics, makeStatementsColumns, @@ -154,6 +148,7 @@ export interface StatementsPageStateProps { isTenant?: UIConfigState["isTenant"]; hasViewActivityRedactedRole?: UIConfigState["hasViewActivityRedactedRole"]; hasAdminRole?: UIConfigState["hasAdminRole"]; + stmtsTotalRuntimeSecs: number; } export interface StatementsPageState { @@ -581,7 +576,6 @@ export class StatementsPage extends React.Component< } = this.props; const statements = this.props.statements ?? []; const data = this.filteredStatementsData(); - const totalWorkload = calculateTotalWorkload(statements); const totalCount = data.length; const isEmptySearchResults = statements?.length > 0 && search?.length > 0; // If the cluster is a tenant cluster we don't show info @@ -594,7 +588,7 @@ export class StatementsPage extends React.Component< const columns = makeStatementsColumns( statements, filters.app?.split(","), - totalWorkload, + this.props.stmtsTotalRuntimeSecs, nodeRegions, "statement", isTenant, diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx index 1b961742c48f..5215888a1a15 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx @@ -120,6 +120,8 @@ export const ConnectedStatementsPage = withRouter( totalFingerprints: selectTotalFingerprints(state), limit: selectStmtsPageLimit(state), reqSortSetting: selectStmtsPageReqSort(state), + stmtsTotalRuntimeSecs: + state.adminUI?.statements?.data?.stmts_total_runtime_secs ?? 0, }, activePageProps: mapStateToActiveStatementsPageProps(state), }), diff --git a/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts b/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts index 97c5305bc245..7f88306fa492 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts @@ -38,6 +38,7 @@ describe("txnStats sagas", () => { const payload = new cockroach.server.serverpb.CombinedStatementsStatsRequest({ start: Long.fromNumber(1596816675), end: Long.fromNumber(1596820675), + limit: Long.fromNumber(100), }); const txnStatsResponse = new cockroach.server.serverpb.StatementsResponse({ diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.fixture.ts index a4f6647045bf..2d089bc04080 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.fixture.ts @@ -124,6 +124,8 @@ export const transaction = { export const transactionDetailsData: StatementsResponse = { toJSON: () => ({}), + stmts_total_runtime_secs: 1, + txns_total_runtime_secs: 1, statements: [ { key: { diff --git a/pkg/ui/workspaces/cluster-ui/src/util/sqlActivityConstants.ts b/pkg/ui/workspaces/cluster-ui/src/util/sqlActivityConstants.ts index a5da57d40486..d7b9b7ac7e4c 100644 --- a/pkg/ui/workspaces/cluster-ui/src/util/sqlActivityConstants.ts +++ b/pkg/ui/workspaces/cluster-ui/src/util/sqlActivityConstants.ts @@ -26,6 +26,8 @@ export function getSortLabel(sort: SqlStatsSortType): string { return "Execution Count"; case SqlStatsSortOptions.CONTENTION_TIME: return "Contention Time"; + case SqlStatsSortOptions.PCT_RUNTIME: + return "% Of All Run Time"; default: return ""; } @@ -38,6 +40,8 @@ export const stmtRequestSortOptions = Object.values(SqlStatsSortOptions).map( }), ); -export const txnRequestSortOptions = stmtRequestSortOptions.map(opt => opt); +export const txnRequestSortOptions = stmtRequestSortOptions.filter( + option => option.value !== SqlStatsSortOptions.PCT_RUNTIME, +); export const STATS_LONG_LOADING_DURATION = duration(2, "s"); diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx index 3b7ceca1c887..da0b329da58c 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx @@ -400,6 +400,8 @@ export default withRouter( hasAdminRole: selectHasAdminRole(state), limit: limitSetting.selector(state), reqSortSetting: reqSortSetting.selector(state), + stmtsTotalRuntimeSecs: + state.cachedData?.statements?.data?.stmts_total_runtime_secs ?? 0, }, activePageProps: mapStateToActiveStatementViewProps(state), }),