Skip to content

Commit

Permalink
Merge cockroachdb#97925
Browse files Browse the repository at this point in the history 
97925: ui, server: remove aggregationInterval from stmts response r=xinhaoz a=xinhaoz

The aggregation interval is no longer a field we group stmts on. This commit removes it from the GROUP BY clause in the stats api for both stmts and txns, and removes remaining dead code in the UI for the aggregation interval field.

Epic: none

Release note: None

Co-authored-by: Xin Hao Zhang <[email protected]>
  • Loading branch information
@xinhaoz
craig[bot] and xinhaoz committed Mar 6, 2023
2 parents 709d765 + 25e7879 commit 71d8239
Show file tree
Hide file tree
Showing 13 changed files with 9 additions and 62 deletions.
37 changes: 9 additions & 28 deletions pkg/server/combined_statement_stats.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,19 +145,16 @@ func collectCombinedStatements(
app_name,
max(aggregated_ts) as aggregated_ts,
metadata,
crdb_internal.merge_statement_stats(array_agg(statistics)) AS statistics,
max(sampled_plan) AS sampled_plan,
aggregation_interval
crdb_internal.merge_statement_stats(array_agg(statistics)) AS statistics
FROM crdb_internal.statement_statistics %s
GROUP BY
fingerprint_id,
transaction_fingerprint_id,
app_name,
metadata,
aggregation_interval
metadata
%s`, whereClause, orderAndLimit)

const expectedNumDatums = 8
const expectedNumDatums = 6

it, err := ie.QueryIteratorEx(ctx, "combined-stmts-by-interval", nil,
sessiondata.NodeUserSessionDataOverride, query, args...)
Expand Down Expand Up @@ -213,20 +210,10 @@ func collectCombinedStatements(
return nil, serverError(ctx, err)
}

planJSON := tree.MustBeDJSON(row[6]).JSON
plan, err := sqlstatsutil.JSONToExplainTreePlanNode(planJSON)
if err != nil {
return nil, serverError(ctx, err)
}
metadata.Stats.SensitiveInfo.MostRecentPlanDescription = *plan

aggInterval := tree.MustBeDInterval(row[7]).Duration

stmt := serverpb.StatementsResponse_CollectedStatementStatistics{
Key: serverpb.StatementsResponse_ExtendedStatementStatisticsKey{
KeyData: metadata.Key,
AggregatedTs: aggregatedTs,
AggregationInterval: time.Duration(aggInterval.Nanos()),
KeyData: metadata.Key,
AggregatedTs: aggregatedTs,
},
ID: appstatspb.StmtFingerprintID(statementFingerprintID),
Stats: metadata.Stats,
Expand Down Expand Up @@ -257,17 +244,15 @@ func collectCombinedTransactions(
max(aggregated_ts) as aggregated_ts,
fingerprint_id,
metadata,
crdb_internal.merge_transaction_stats(array_agg(statistics)) AS statistics,
aggregation_interval
crdb_internal.merge_transaction_stats(array_agg(statistics)) AS statistics
FROM crdb_internal.transaction_statistics %s
GROUP BY
app_name,
fingerprint_id,
metadata,
aggregation_interval
metadata
%s`, whereClause, orderAndLimit)

const expectedNumDatums = 6
const expectedNumDatums = 5

it, err := ie.QueryIteratorEx(ctx, "combined-txns-by-interval", nil,
sessiondata.NodeUserSessionDataOverride, query, args...)
Expand Down Expand Up @@ -313,15 +298,12 @@ func collectCombinedTransactions(
return nil, serverError(ctx, err)
}

aggInterval := tree.MustBeDInterval(row[5]).Duration

txnStats := serverpb.StatementsResponse_ExtendedCollectedTransactionStatistics{
StatsData: appstatspb.CollectedTransactionStatistics{
StatementFingerprintIDs: metadata.StatementFingerprintIDs,
App: app,
Stats: metadata.Stats,
AggregatedTs: aggregatedTs,
AggregationInterval: time.Duration(aggInterval.Nanos()),
TransactionFingerprintID: appstatspb.TransactionFingerprintID(fingerprintID),
},
}
Expand Down Expand Up @@ -376,7 +358,7 @@ func getStatementDetails(
if err != nil {
return nil, serverError(ctx, err)
}
statementStatisticsPerPlanHash, err := getStatementDetailsPerPlanHash(ctx, ie, whereClause, args, limit, settings)
statementStatisticsPerPlanHash, err := getStatementDetailsPerPlanHash(ctx, ie, whereClause, args, limit)
if err != nil {
return nil, serverError(ctx, err)
}
Expand Down Expand Up @@ -719,7 +701,6 @@ func getStatementDetailsPerPlanHash(
whereClause string,
args []interface{},
limit int64,
settings *cluster.Settings,
) ([]serverpb.StatementDetailsResponse_CollectedStatementGroupedByPlanHash, error) {

query := fmt.Sprintf(
Expand Down
1 change: 0 additions & 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 @@ -138,7 +138,6 @@ export function convertStatementRawFormatToAggregatedStatistics(
s: StatementRawFormat,
): AggregateStatistics {
return {
aggregationInterval: s.agg_interval,
applicationName: s.app_name,
database: s.metadata.db,
fullScan: s.metadata.fullScan,
Expand Down
2 changes: 0 additions & 2 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ const diagnosticsReportsInProgress: StatementDiagnosticsReport[] = [

const aggregatedTs = Date.parse("Sep 15 2021 01:00:00 GMT") * 1e-3;
const lastUpdated = moment("Sep 15 2021 01:30:00 GMT");
const aggregationInterval = 3600; // 1 hour

export const mockStmt = (
partialStmt: Partial<Omit<AggregateStatistics, "aggregatedFingerprintHexID">>,
Expand All @@ -286,7 +285,6 @@ export const mockStmt = (
label: "SELECT 1",
summary: "SELECT 1",
aggregatedTs,
aggregationInterval,
implicitTxn: true,
database: "defaultdb",
applicationName: "app",
Expand Down
3 changes: 0 additions & 3 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export interface StatementsSummaryData {
statement: string;
statementSummary: string;
aggregatedTs: number;
aggregationInterval: number;
implicitTxn: boolean;
fullScan: boolean;
database: string;
Expand Down Expand Up @@ -182,7 +181,6 @@ export const selectStatements = createSelector(
statement: stmt.statement,
statementSummary: stmt.statement_summary,
aggregatedTs: stmt.aggregated_ts,
aggregationInterval: stmt.aggregation_interval,
implicitTxn: stmt.implicit_txn,
fullScan: stmt.full_scan,
database: stmt.database,
Expand All @@ -203,7 +201,6 @@ export const selectStatements = createSelector(
label: stmt.statement,
summary: stmt.statementSummary,
aggregatedTs: stmt.aggregatedTs,
aggregationInterval: stmt.aggregationInterval,
implicitTxn: stmt.implicitTxn,
fullScan: stmt.fullScan,
database: stmt.database,
Expand Down
1 change: 0 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ describe("StatementsPage", () => {
aggregatedFingerprintID: "",
aggregatedFingerprintHexID: "",
aggregatedTs: 0,
aggregationInterval: 0,
database: "",
applicationName: "",
fullScan: false,
Expand Down
1 change: 0 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ describe("populateRegionNodeForStatements", () => {
label: "",
summary: "",
aggregatedTs: 0,
aggregationInterval: 0,
implicitTxn: false,
fullScan: false,
database: "",
Expand Down
1 change: 0 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ export interface AggregateStatistics {
// replaced with shortStatement otherwise.
summary: string;
aggregatedTs: number;
aggregationInterval: number;
implicitTxn: boolean;
fullScan: boolean;
database: string;
Expand Down
4 changes: 0 additions & 4 deletions pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export const StatementTableCell = {
statement={stmt.label}
statementSummary={stmt.summary}
aggregatedTs={stmt.aggregatedTs}
aggregationInterval={stmt.aggregationInterval}
appNames={selectedApp}
implicitTxn={stmt.implicitTxn}
search={search}
Expand Down Expand Up @@ -187,7 +186,6 @@ export const StatementLinkTarget = (
interface StatementLinkProps {
statementFingerprintID: string;
aggregatedTs?: number;
aggregationInterval?: number;
appNames?: string[];
implicitTxn: boolean;
statement: string;
Expand All @@ -200,7 +198,6 @@ interface StatementLinkProps {

export const StatementLink = ({
statementFingerprintID,
aggregationInterval,
appNames,
implicitTxn,
statement,
Expand All @@ -217,7 +214,6 @@ export const StatementLink = ({

const linkProps = {
statementFingerprintID,
aggregationInterval,
appNames,
implicitTxn,
};
Expand Down
15 changes: 0 additions & 15 deletions pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1024,18 +1024,3 @@ export const statisticsTableTitles: StatisticTableTitleType = {
);
},
};

export function formatAggregationIntervalColumn(
aggregatedTs: number,
interval: number,
): string {
const formatStr = "MMM D, H:mm";
const formatStrWithoutDay = "H:mm";
const start = moment.unix(aggregatedTs).utc();
const end = moment.unix(aggregatedTs + interval).utc();
const isSameDay = start.isSame(end, "day");

return `${start.format(formatStr)} - ${end.format(
isSameDay ? formatStrWithoutDay : formatStr,
)}`;
}
1 change: 0 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ export const aggregateStatements = (
label: s.statement,
summary: s.statement_summary,
aggregatedTs: s.aggregated_ts,
aggregationInterval: s.aggregation_interval,
implicitTxn: s.implicit_txn,
database: s.database,
applicationName: s.app,
Expand Down
1 change: 0 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/util/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

export const aggregationIntervalAttr = "aggregation_interval";
export const aggregatedTsAttr = "aggregated_ts";
export const appAttr = "app";
export const appNamesAttr = "appNames";
Expand Down
1 change: 0 additions & 1 deletion pkg/ui/workspaces/db-console/src/util/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { util } from "@cockroachlabs/cluster-ui";
export const indexNameAttr = "index_name";

export const {
aggregationIntervalAttr,
aggregatedTsAttr,
appAttr,
appNamesAttr,
Expand Down
3 changes: 0 additions & 3 deletions pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ interface StatementsSummaryData {
statement: string;
statementSummary: string;
aggregatedTs: number;
aggregationInterval: number;
implicitTxn: boolean;
fullScan: boolean;
database: string;
Expand Down Expand Up @@ -155,7 +154,6 @@ export const selectStatements = createSelector(
statement: stmt.statement,
statementSummary: stmt.statement_summary,
aggregatedTs: stmt.aggregated_ts,
aggregationInterval: stmt.aggregation_interval,
implicitTxn: stmt.implicit_txn,
fullScan: stmt.full_scan,
database: stmt.database,
Expand All @@ -176,7 +174,6 @@ export const selectStatements = createSelector(
label: stmt.statement,
summary: stmt.statementSummary,
aggregatedTs: stmt.aggregatedTs,
aggregationInterval: stmt.aggregationInterval,
implicitTxn: stmt.implicitTxn,
fullScan: stmt.fullScan,
database: stmt.database,
Expand Down

0 comments on commit 71d8239

Please sign in to comment.