Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: transaction insight details page hangs on refresh in CC console #94380

Closed
ericharmeling opened this issue Dec 28, 2022 · 2 comments · Fixed by #92285
Closed

ui: transaction insight details page hangs on refresh in CC console #94380

ericharmeling opened this issue Dec 28, 2022 · 2 comments · Fixed by #92285
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@ericharmeling
Copy link
Contributor

ericharmeling commented Dec 28, 2022

The transaction insight details page in the CC console hangs indefinitely on refresh.

When selectTxnInsightDetailsCombiner (a selector called when the transaction insight details page is loaded) returns null, a refresh is triggered.

In the case of transaction insight detail events of type "High Contention" (i.e., every time you navigate to the details page of a High Contention insight event), a refresh will always be triggered, as selectTxnInsightDetailsCombiner will always return null when the transactionInsightDetails.cachedData is in its initial empty state.

Here's a loom demonstrating that scenario: https://www.loom.com/share/8dce9d06ae2548b2adf5fc38831f790f

As shown in the loom video, in the case of all other transaction detail events where type != "High Contention", selectTxnInsightDetailsCombiner returns the insights data cached in the executionInsights and transactionInsights slices when you navigate to the details page from the overview page. However, on a manual refresh, the page hangs indefinitely.

I'm not exactly sure why this is happening, but the browser console errors point to a "MapSet" error. I interpreted this as an issue with the initial state of transactionInsightDetails.cachedData (populated using new Map()), and was able to resolve the error by changing the cachedData type from a map to an object with an initial state of {}. This is how we've been typing/initializing other "map"-like slices (e.g., statement details).

Here's another loom demonstrating the error on management staging: https://www.loom.com/share/fec151a3a39f4a469c1efd0bc8fec54b

Jira issue: CRDB-22864

@ericharmeling ericharmeling added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-observability labels Dec 28, 2022
@ericharmeling ericharmeling self-assigned this Dec 28, 2022
@ericharmeling
Copy link
Contributor Author

I'm not exactly sure why this is happening, but the browser console errors point to a "MapSet" error. I interpreted this as an issue with the initial state of transactionInsightDetails.cachedData (populated using new Map()), and was able to resolve the error by changing the cachedData type from a map to an object with an initial state of {}. This is how we've been typing/initializing other "map"-like slices (e.g., statement details).

I'm going to add this temporary fix to #92285, as I see it as an additional blocker for #83780.

@ericharmeling
Copy link
Contributor Author

This issue predates #94445, although both independently break the transaction insight details page.

@craig craig bot closed this as completed in 8b1be12 Jan 11, 2023
ericharmeling added a commit to ericharmeling/cockroach that referenced this issue Jan 11, 2023
This commit reverts the cachedData field of
TransactionInsightDetailsCachedState from a map to an object.

Fixes cockroachdb#94380 in release-22.2.

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant