sql: add cpu nanos to insights tables

[maryliag]

Part Of #87213

Add cpu_sql_nanos to
crdb_internal.{cluster,nodes}_execution_insights and crdb_internal.{cluster,nodes}_txn_execution_insights.

Release note: None

[cockroach-teamcity]

This change is 

[j82w]

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @maryliag)

---

pkg/sql/crdb_internal.go line 6957 at r1 (raw file):

  causes                     STRING[] NOT NULL,
  stmt_execution_ids         STRING[] NOT NULL,
  cpu_nanos                  INT8

Should this be sql_cpu_nanos or something more specific since it doesn't cover cpu time of the entire query like kv layer?

---

pkg/sql/sqlstats/insights/integration/insights_test.go line 146 at r1 (raw file):

		var startInsights, endInsights time.Time
		var implicitTxn bool
		var cpuNanos int64

Add a check to verify cpu is greater than 0 and less than some large number that should be incorrect.

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @j82w)

---

pkg/sql/crdb_internal.go line 6957 at r1 (raw file):

Previously, j82w (Jake) wrote…

Should this be sql_cpu_nanos or something more specific since it doesn't cover cpu time of the entire query like kv layer?

I'm keeping consistent with the name on the system/stats tables. We will be adding the remaining info to it, so better to keep like this to avoid a renaming in the future

---

pkg/sql/sqlstats/insights/integration/insights_test.go line 146 at r1 (raw file):

Previously, j82w (Jake) wrote…

Add a check to verify cpu is greater than 0 and less than some large number that should be incorrect.

Do you have any suggestion about what the large number should be?

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @j82w)

---

pkg/sql/sqlstats/insights/integration/insights_test.go line 146 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

Do you have any suggestion about what the large number should be?

Just realize this is for a pg_sleep, so I can use that as a base for the time here

[j82w]

pkg/sql/sqlstats/insights/integration/insights_test.go line 146 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

Just realize this is for a pg_sleep, so I can use that as a base for the time here

It shouldn't exceed the elapsed time which can be calculated from the end_time-start_time. I would add 10ms buffer to be on the safe.

[j82w]

pkg/sql/sqlstats/insights/integration/insights_test.go line 146 at r1 (raw file):

Previously, j82w (Jake) wrote…

It shouldn't exceed the elapsed time which can be calculated from the end_time-start_time. I would add 10ms buffer to be on the safe.

Is the actual sleep time included in the cpu time? That seems like possibly a bug if it is because I'm hoping it doesn't hold a cpu that entire time.

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @j82w)

---

pkg/sql/sqlstats/insights/integration/insights_test.go line 146 at r1 (raw file):

Previously, j82w (Jake) wrote…

Is the actual sleep time included in the cpu time? That seems like possibly a bug if it is because I'm hoping it doesn't hold a cpu that entire time.

is not included, which made me think that the cpu had to be smaller, but your idea is better. Will add the changes

[j82w]

pkg/sql/crdb_internal.go line 6957 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

I'm keeping consistent with the name on the system/stats tables. We will be adding the remaining info to it, so better to keep like this to avoid a renaming in the future

Wouldn't it be better to keep the cpu stats separate to avoid ambiguity? For example if it just says 10.1seconds it would be very different issue if 10s in sql layer and .1s is kv layer vs .1s in sql and 10 sin kv.

[j82w]

pkg/ccl/backupccl/testdata/backup-restore/alter-schedule/backup-options line 41 at r2 (raw file):

with schedules as (show schedules) select id, command->'backup_statement' from schedules where label='datatest' order by command->>'backup_type' asc;
----
835701674603675649 "BACKUP INTO 'nodelocal://1/example-schedule' WITH revision_history = false, detached"

Is this right? I don't see anything in the PR touching backup options.

[j82w]

Reviewed 9 of 9 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @maryliag)

[j82w]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @maryliag)

[maryliag]

TFTR!
bors r+

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[craig]

Build failed:

• Bazel Essential CI (Cockroach)

[maryliag]

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)