cluster-ui: update transactions page with execution stats #193
Conversation
|
I didn't see any tests to update |
| // addTransactionStats adds together two stat objects into one using their counts to compute a new | ||
| // average for the numeric statistics. It's modeled after the similar `addStatementStats` function | ||
| function addTransactionStats( | ||
| a: TransactionStats, | ||
| b: TransactionStats, | ||
| ): TransactionStats { | ||
| ): Required<TransactionStats> { |
There was a problem hiding this comment.
I added this in because I thought that incorrect addition of transaction stats was causing some null property accesses, but it was in fact due to something else. However, I kept it in because I thought it's hygienic. When is this used? I didn't update my statements page PR to update the similar addStatementStats (however, something similar exists in the main cockroach repo, which is not the case for transactions). What's the consequence of this? Should I update addStatementStats?
There was a problem hiding this comment.
Yes I like making changes like this. I think making the same change to addStatementStats is worth it.
Because the Statements Page has been integrated into CC, it has its own redux layer in cluster-ui that was built to work nicely with CC's redux store. That redux layer is not used with DB Console so there is some code duplication there. I think what we'll end up doing in the near term is moving the redux layer directly into CC to clarify that the two redux configurations are separate and make peace with some code duplication.
So making that change to addStatementStats will protect CC's version of the Statements Page from null lookups.
stddev for rows and bytes read used the sample count, when the calculations should use the statement count, since these stats are not sampled.
This commit removes the rows affected column from the transactions page and adds rows/bytes read, max memory usage, and latency. A contention column is left for a future commit (pending backend changes).
Previously, the resullt of addStatementStats would have some fields unset. This commit sets those fields and adds Required<StatementStatistics> so that any new fields have to be handled by addStatementStats.
This PR has cockroachdb/cockroach#59761 as a dependency. I'll just wait for that to go in before updating with the new protobuf version number.
This PR, similarly to #178, changes the transactions page to what we've designed for 21.1 (@Annebirzin). This is the result:
Checklist
cc @awoods187
Closes cockroachdb/cockroach#59295