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

cluster-ui: speed up insights page request #107292

Merged
merged 2 commits into from Aug 4, 2023

Conversation

xinhaoz
Copy link
Member

@xinhaoz xinhaoz commented Jul 20, 2023

Previously, we were querying from crdb_internal.statement_statistics and
crdb_internal.transaction_statistics on the schema insights and txn insights
pages. Querying from this table is slow because it triggers a cluster-wide
fanout to collect unflushsed sql stats. We can use the persisted table instead,
which will lag behind a little in live data, but will be much faster to query from.

Epic: none
Fixes: #107291

Release note (bug fix): Schema insights page should hit request timeouts less
frequently, if at all.

DB Console Loom, showing pages issuing requests work as expected:
https://www.loom.com/share/ef1baad105d0444596a32c8430234b88

@xinhaoz xinhaoz requested review from koorosh and a team July 20, 2023 18:52
@xinhaoz xinhaoz requested a review from a team as a code owner July 20, 2023 18:52
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@xinhaoz
Copy link
Member Author

xinhaoz commented Jul 20, 2023

Will update with Loom.

@xinhaoz xinhaoz changed the title cluster-ui: speed up schema insights page request cluster-ui: speed up insights page request Jul 20, 2023
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @koorosh and @xinhaoz)


pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts line 199 at r2 (raw file):

      ) AS rank 
    FROM 
      crdb_internal.statement_statistics_persisted 

is there any call to the transaction table that should also be updated on this PR?

@xinhaoz
Copy link
Member Author

xinhaoz commented Jul 24, 2023

pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts line 199 at r2 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

is there any call to the transaction table that should also be updated on this PR?

Ah yes, good point! I saw one use case and updated to change that, too.

@xinhaoz xinhaoz requested review from a team and maryliag July 24, 2023 14:31
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, 6 of 6 files at r5, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @koorosh and @xinhaoz)


pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts line 264 at r5 (raw file):

      id =>
        stmtFingerprintToQuery.get(id) ??
        `Query unavailable for stmt fingerprint ${id}`,

is this showing on the UI? if so, use the full name, so statement and not stmt

Previously, we were querying from `crdb_internal.statement_statistics` and
`crdb_internal.transaction_statistics` on the schema insights and txn insights
pages. Querying from this table is slow because it triggers a cluster-wide
fanout to collect unflushsed sql stats. We can use the persisted table instead,
which will lag behind a little in live data, but will be much faster to query from.

Some tests for the txn contention details api are also added.

Epic: none
Fixes: cockroachdb#107291

Release note (bug fix): Schema insights page should hit request timeouts less
frequently, if at all.
@xinhaoz
Copy link
Member Author

xinhaoz commented Aug 4, 2023

pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts line 264 at r5 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

is this showing on the UI? if so, use the full name, so statement and not stmt

Done.

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r6, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @koorosh)

@xinhaoz
Copy link
Member Author

xinhaoz commented Aug 4, 2023

TFTR!
bors r+

@craig
Copy link
Contributor

craig bot commented Aug 4, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 4, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 4, 2023

Build succeeded:

@craig craig bot merged commit 374835f into cockroachdb:master Aug 4, 2023
7 checks passed
@xinhaoz
Copy link
Member Author

xinhaoz commented Aug 18, 2023

blathers backport 23.1

@xinhaoz xinhaoz deleted the schema-insights-query branch April 1, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ui: schema insights request timing out
3 participants