ui: insights transaction details support multiple blocking transactions

[j82w]

trafficstars

This adds support for multiple blocking transactions for a single waiting transaction. The cards were merged into the table, and the data was piped through to show multiple rows. The total contention time was also fixed to aggregate the contention time instead of just picking the latest.

before: https://loom.com/share/0384ed937a344e2fb0105fefbc313acb

after: https://www.loom.com/share/78e906f50a694cd59ac893ddb9c2239a

closes #88264

Release justification: Category 2: Bug fixes and low-risk updates to new functionality

Release note: (ui change): Add support for multiple blocking transaction on insights transaction details page. Merged the cards into the table, and fixed the total contention time.

[cockroach-teamcity]

This change is 

[kevin-v-ngo]

Hi @j82w, I thought we had index as well? Or is that coming in another PR?

[j82w]

That was not in the original page. @ericharmeling is there a reason it does not display the index?

[kevin-v-ngo]

Got it. Let me know if this isn't being tracked. I can file an issue.

[j82w]

pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/util/insightsColumns.tsx line 136 at r2 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

@kevin-v-ngo

Is there a reason why we wouldn't have a contended index?

I think that depends on the query/table. Not all tables have secondary indexes.

And even key/row span? I thought contention would always be at the key level?

We remove the contended key from the txn insight details page.

Discussion thread: https://cockroachlabs.slack.com/archives/G01Q9D01NTU/p1662676417756419?thread_ts=1662662201.540079&cid=G01Q9D01NTU

PR: https://github.com/cockroachdb/cockroach/pull/87645

I added the index to the table column.

[kevin-v-ngo]

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ericharmeling, @j82w, and @maryliag)

pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/util/insightsColumns.tsx line 136 at r2 (raw file):

Previously, kevin-v-ngo wrote…

Is there a reason why we wouldn't have a contended index? And even key/row span? I thought contention would always be at the key level?

We don't always have a contended index because there might not be an index. For the key we removed that recently to make this page more user friendly #87645

There's always an index though. Or are you not counting primary index?

[ericharmeling]

There's always an index though. Or are you not counting primary index?

It doesn't look like crdb_internal.ranges (where we are pulling the schema, database, table, and index names from) records primary index names. We could just list "primary" when there is no other index recorded.

[j82w]

pkg/ui/workspaces/cluster-ui/src/api/insightsApi.ts line 385 at r3 (raw file):

      databaseName: value.database_name,
      tableName: value.table_name,
      indexName: value.index_name,

@maryliag instead of leaving the column empty when there is no index should it say "No Index"?

[j82w]

I renamed the column to secondary indexes. Primary indexes do not show up in the column.

[j82w]

pkg/ui/workspaces/cluster-ui/src/api/insightsApi.ts line 385 at r3 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

I think it makes sense to show "No Index"for this case, leaving the value empty on the table would be weird. We already have precedent for this, where we have "no samples" when there isn't stats for something. We do use all lower for the "no samples" so I would say to keep consistent here and use "no index"

@kevin-v-ngo suggested keeping the column name as "Index Name", but using "primary index" instead of "no index" to avoid confusion.

[j82w]

pkg/ui/workspaces/cluster-ui/src/api/insightsApi.ts line 388 at r4 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

          : "no secondary index",

I went with Kevin's suggestion. The column name is "Index Name" with a default value of "primary index" to avoid confusion.

[j82w]

bors r+

[craig[bot]]

Build succeeded:

• Bazel Essential CI (Cockroach)

[kevin-v-ngo]

Just to follow up and close the loop for those following the thread. @j82w and I were chatting,

It didn't make sense to say "no index" since contention had to have happened somewhere. In this case where there is no secondary index, it's the primary index.

Although technically still correct to have a Secondary index column, we weren't being clear that contention was happening on the primary index.