ui: show data when max size reached #97153
Conversation
|
This change is |
maryliag
force-pushed
the
max-size
branch
3 times, most recently
from
56a7c7a to
496aacb
Compare
There was a problem hiding this comment.
Reviewed 13 of 13 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @dongniwang, @kevin-v-ngo, and @maryliag)
pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts line 156 at r1 (raw file):
if (sqlResultsAreEmpty(result)) { return formatApiResult([], result.error, "retrieving insights information");
Should the query be modified to order by the cpu usage or some other value so the most impactful insights are always loaded?
There was a problem hiding this comment.
Reviewable status:
pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts line 156 at r1 (raw file):
Previously, j82w (Jake) wrote…
Should the query be modified to order by the cpu usage or some other value so the most impactful insights are always loaded?
Created a follow-up task to update insights all around: #97243
Previously, when the sql api returned a max size reached error, we were just showing the error, but not the data that was also being returned. This PR creates a new function to format the return of the api calls, so when is a max size error it doesn't throw an error, but still pass that info so we can display a warning on the pages. This commit updates the Insights Workload > Statement page with the new behaviour. Following PRs will update other usages of the sql api. Part Of: cockroachdb#96184 Release note (ui change): Still show data on the console (with a warning) for Statement Insights when we reach a "max size exceed" error from the sql api.
| @@ -60,6 +60,11 @@ export type SqlExecutionErrorMessage = { | |||
| source: { file: string; line: number; function: "string" }; | |||
| }; | |||
|
|
|||
| export type ApiResponse<ResultType> = { | |||
There was a problem hiding this comment.
nit: just a super small preference thing, feel free to leave; I prefer SqlApiResponse here, it's a bit more indicative of what we're using to get the results.
There was a problem hiding this comment.
I wanna get this in, so I will update on my next PR (taking advantage of all tests that passed already :D )
|
TFTR! |
|
Build failed (retrying...): |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from a6fd886 to blathers/backport-release-22.2-97153: 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. |
Previously, when the sql api returned a max size reached error, we were just showing the error, but not the data that was also being returned.
This PR creates a new function to format the return of the api calls, so when is a max size error it doesn't throw an error, but still pass that info so we can display a warning on the pages.
This commit updates the Insights Workload > Statement page
with the new behaviour.
Following PRs will update other usages of the sql api.
https://www.loom.com/share/9d9a24c486ce466ab355e1040af095c9
Part Of: #96184
Release note (ui change): Still show data on the console
(with a warning) for Statement Insights when we reach a
"max size exceed" error from the sql api.