Skip to content

Commit

Permalink
ui: fix txn aggregations in txns fingerprints page
Browse files Browse the repository at this point in the history
This commit addresses 2 issues on the txns overview page:
1. We were previously grouping txns by txn fingerprint
id, agg time, agg interval, and app name. This is from
a time when we wanted all these fields, but recently
we only want to aggregate on txn fingerprint id.
This commit changes the grouping to only the txn id.

2. Stats aggregation causing undesired data mutations:
We were seeing that in the txns fingerprint page,
stats columns would seemingly randomly continue to
increase while on the page (e.g. exec count, bytes
read). During stats aggregation after grouping by
the fields mentioned above, we were using the first
txn in the grouping  as the base object for stats
aggregation, meaning we inherited and mutated the
stats object of that txn. Since we aggregate on
every re-render, This meant that we were using the
result of any previous aggregations as the base for our
current aggregation in the re-render. This explains
the never-ending incrementing stats. This commit
addresses this bug by ensuring we don't re-use the
stats object between re-renders by creating a new copy
of the stats for every aggregation.

Fixes: #96186
Fixes: #68375

Release note (bug fix): stats columns in txns fingerprint overview
page does not continuously increment
  • Loading branch information
xinhaoz committed Mar 9, 2023
1 parent ceb2940 commit a59d6b6
Showing 1 changed file with 3 additions and 8 deletions.
11 changes: 3 additions & 8 deletions pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,11 +400,12 @@ function combineTransactionStats(
// and returns a copy of the first element with its `stats_data.stats` object replaced with a
// merged stats object that aggregates statistics from every copy of the fingerprint in the list
// provided
// This function SHOULD NOT mutate any objects in the provided txns array.
const mergeTransactionStats = function (txns: Transaction[]): Transaction {
if (txns.length === 0) {
return null;
}
const txn = { ...txns[0] };
const txn = _.cloneDeep(txns[0]);
txn.stats_data.stats = combineTransactionStats(
txns.map(t => t.stats_data.stats),
);
Expand All @@ -426,13 +427,7 @@ export const aggregateAcrossNodeIDs = function (
): Transaction[] {
return _.chain(t)
.map(t => withFingerprint(t, stmts))
.groupBy(
t =>
t.fingerprint +
t.stats_data.app +
TimestampToNumber(t.stats_data.aggregated_ts) +
DurationToNumber(t.stats_data.aggregation_interval),
)
.groupBy(t => t.fingerprint)
.mapValues(mergeTransactionStats)
.values()
.value();
Expand Down

0 comments on commit a59d6b6

Please sign in to comment.