ui: remove polling from fingerprints pages, allow new stats requests while one is in flight

[xinhaoz]

See individual commits.

The changes here serve as a base for the performance improvements we'll be making.
Since they're not really related to those changes specifically (e.g. adding limit/sort and splitting the api calls), I've put them in their own PR here to make reviewing easier.

Loom verifying behaviour and showing new requests being dispatched while ones are pending
Pages shown:

• stmt fingerprints
• txn fingerprints
• txn fingerprint details
• stmt fingerprint details

https://www.loom.com/share/3448117bfecf404c8d698f4ad1240e8c

CC:
https://www.loom.com/share/bb30b51ebe5144528ab0c6fabdbfb2f1

[cockroach-teamcity]

This change is 

[maryliag]

Reviewed 3 of 3 files at r1, 2 of 2 files at r2, 1 of 2 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @xinhaoz)

---

-- commits line 37 at r3:
nit: experiences

---

-- commits line 43 at r3:
nit: this phrase is confusing

---

-- commits line 50 at r3:
nice!

---

pkg/ui/workspaces/db-console/src/redux/cachedDataReducer.ts line 324 at r3 (raw file):

      this.pendingRequest = request;

      return request;

can you add test to confirm the behaviour and/or a loom showing network tab before/after?

[xinhaoz]

I have to do some testing on CC still. I'll add that Loom once it is done.

Edit: Included in the description.

[j82w]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @maryliag and @xinhaoz)

---

-- commits line 6 at r9:
nit: typo finegerprints

---

-- commits line 29 at r9:
Should this be labeled SQL Activity pages instead of fingerprint pages?

---

-- commits line 31 at r9:
Should a refresh/apply button be added to the page to make it clear that it's not refreshing?

---

pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx line 212 at r9 (raw file):

  refreshData = (): void => {
    console.log("aaaaaaaaaaah");

nit: I think this might need to be removed..

---

pkg/ui/workspaces/db-console/src/util/api.ts line 782 at r9 (raw file):

  return timeoutFetch(
    serverpb.StatementsResponse,
    `${STATUS_PREFIX}/combinedstmts?${queryStr}`,

Is this a new api? If so should it be a concern this will likely break during upgrades?

[xinhaoz]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dongniwang, @j82w, and @maryliag)

---

-- commits line 43 at r3:

Previously, maryliag (Marylia Gutierrez) wrote…

nit: this phrase is confusing

Rephrased.

---

-- commits line 6 at r9:

Previously, j82w (Jake) wrote…

nit: typo finegerprints

Fixed.

---

-- commits line 29 at r9:

Previously, j82w (Jake) wrote…

Should this be labeled SQL Activity pages instead of fingerprint pages?

I wanted to distinguish them from insights pages / active exec pages, I can change it SQL Activity if it's more clear though.

---

-- commits line 31 at r9:

Previously, j82w (Jake) wrote…

Should a refresh/apply button be added to the page to make it clear that it's not refreshing?

Sounds like a good idea -- cc @dongniwang

---

pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx line 212 at r9 (raw file):

Previously, j82w (Jake) wrote…

nit: I think this might need to be removed..

Oops, removed.

---

pkg/ui/workspaces/db-console/src/redux/cachedDataReducer.ts line 324 at r3 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

can you add test to confirm the behaviour and/or a loom showing network tab before/after?

Loom added.

---

pkg/ui/workspaces/db-console/src/util/api.ts line 782 at r9 (raw file):

Previously, j82w (Jake) wrote…

Is this a new api? If so should it be a concern this will likely break during upgrades?

It's not a new api. It existed since the creation of persisted stats from 21.2, we just did not use it and instead opted to have the older api just route to this one using a req param in case we needed to revert anything. We're fully committed to persisted stats now though so I don't see why we shouldn't be using it.

[xinhaoz]

I've updated the Looms to also show the txn details page since that request behaviour was modified as well, and also to show the long loading messages appearing on both stmt and txn overview pages.

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from 40dd354 to blathers/backport-release-22.1-98331: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.

---

error creating merge commit from 40dd354 to blathers/backport-release-22.2-98331: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.x failed. See errors above.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}