sql: record sql CPU nanoseconds in statement statistics table #95639
Conversation
|
This change is |
DrewKimball
force-pushed
the
cpu-stats
branch
from
c881e35 to
42169a6
Compare
There was a problem hiding this comment.
Looks great!
You're missing some updates on statements.spec.tsx and appStats.spec.ts, and some nits to deal with the lint errors.
Otherwise
Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @yuzefovich)
pkg/ui/workspaces/cluster-ui/src/util/appStats/appStats.ts line 129 at r1 (raw file):
countA, countB, )
nit: you will get a lint error if you don't add , here
pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts line 51 at r1 (raw file):
mean: 80, squared_diffs: 0.01, }
nit: add , (lint error)
pkg/ui/workspaces/cluster-ui/src/util/totalWorkload.fixture.ts line 47 at r1 (raw file):
mean: 4160407, squared_diffs: 47880000000000, }
nit: add , (lint error)
There was a problem hiding this comment.
Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status:
-- commits line 4 at r1:
nit: these tables are not "system" but are virtual internal, right?
There was a problem hiding this comment.
Reviewable status:
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: these tables are not "system" but are virtual internal, right?
Changed "system" to "internal".
pkg/ui/workspaces/cluster-ui/src/util/appStats/appStats.ts line 129 at r1 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
nit: you will get a lint error if you don't add
,here
Done.
pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts line 51 at r1 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
nit: add
,(lint error)
Done.
pkg/ui/workspaces/cluster-ui/src/util/totalWorkload.fixture.ts line 47 at r1 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
nit: add
,(lint error)
Done.
DrewKimball
force-pushed
the
cpu-stats
branch
from
42169a6 to
d0b9983
Compare
There was a problem hiding this comment.
Reviewed 5 of 5 files at r2.
Reviewable status:
Previously, DrewKimball (Drew Kimball) wrote…
Changed "system" to "internal".
actually is both, because exists in crdb_internal and then gets flushed to system table, so users can see this data in both places.
The changes added here will affect both tables, so you can keep the description for both. (same thing for the release note)
This patch adds a `cpu_nanos` field to the `crdb_internal` and `system` statement and transaction statistics tables that tracks the CPU time in nanoseconds spent during SQL execution. Note that similar to disk and memory usage, CPU time is tracked on a sampling basis. Future work will display this field in the UI. Informs cockroachdb#87213 Release note (sql change): Added a `cpuNanos` field to the statistics column of the `crdb_internal.statement_statistics` and `system.statement_statistics` tables that reports the amount of CPU time in nanoseconds during SQL execution for queries that track CPU time.
DrewKimball
force-pushed
the
cpu-stats
branch
from
d0b9983 to
12697c6
Compare
There was a problem hiding this comment.
Reviewable status:
Previously, maryliag (Marylia Gutierrez) wrote…
actually is both, because exists in crdb_internal and then gets flushed to system table, so users can see this data in both places.
The changes added here will affect both tables, so you can keep the description for both. (same thing for the release note)
Updated the descriptions to refer to both.
|
TFTRs! bors r+ |
|
Build failed: |
|
Looks like a flake. bors r+ |
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build succeeded: |
This patch adds a
cpu_nanosfield to thecrdb_internalandsystemstatement and transaction statistics tables that tracks the CPU time in nanoseconds spent during SQL execution. Note that similar to disk and memory usage, CPU time is tracked on a sampling basis. Future work will display this field in the UI.Informs #87213
Release note (sql change): Added a
cpuNanosfield to the statistics column of thecrdb_internal.statement_statisticsandsystem.statement_statisticstables that reports the amount of CPU time in nanoseconds during SQL execution for queries that track CPU time.