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

sql: track CPU time on gateway node #89122

Closed
wants to merge 1 commit into from

Conversation

jordanlewis
Copy link
Member

Updates #87213

This commit adds a new field to the statement_statistics, which is the number of CPU seconds spent (on the gateway node only) processing a particular SQL query.

It uses the new "grunning" CPU seconds tracker to calculate this quantity.

Release note (sql change): add the cpu_time_avg and cpu_time_var fields to crdb_internal.node_statement_statistics, and the cpu_time field to the statistics JSON in system.statement_statistics.

This commit adds a new field to the statement_statistics, which is the
number of CPU seconds spent (on the gateway node only) processing a
particular SQL query.

It uses the new "grunning" CPU seconds tracker to calculate this
quantity.

Release note (sql change): add the cpu_time_avg and cpu_time_var fields
to crdb_internal.node_statement_statistics, and the cpu_time field to
the statistics JSON in system.statement_statistics.
@cockroach-teamcity
Copy link
Member

This change is Reviewable

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.

If this is just playing around, ignore my comments, otherwise I added a few nits and this would also need tests added.

Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis)


pkg/roachpb/app_stats.proto line 117 at r1 (raw file):

  // cpu_time is the amount of CPU time spent (in seconds) by this query.
  optional NumericStat cpu_time = 28 [(gogoproto.nullable) = false];

nit: you're calling this CPU time in a few places and service CPU time in others, make sure is consistent

add (gogoproto.customname) = "CPUTime"


pkg/sql/sessionphase/session_phase.go line 59 at r1 (raw file):

	PlannerEndExecStmt

	PlannerEndExecStmtCpuTime

nit: PlannerEndExecStmtCPUTime

@jordanlewis
Copy link
Member Author

Thanks to @DrewKimball, this is obviated by #93952!

@jordanlewis jordanlewis deleted the cpu-time branch January 20, 2023 19:35
@DrewKimball
Copy link
Collaborator

I used this as my starting point BTW

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.

4 participants