sql: implement crdb_internal.transaction_statistics#69049
sql: implement crdb_internal.transaction_statistics#69049craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
|
This change is |
403eec8 to
22f30db
Compare
d79e334 to
263c92b
Compare
maryliag
left a comment
There was a problem hiding this comment.
Reviewed 15 of 15 files at r1, 14 of 14 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @Azhng)
ajwerner
left a comment
There was a problem hiding this comment.
Reviewed 15 of 15 files at r1, 5 of 14 files at r2, all commit messages.
Reviewable status:
pkg/sql/crdb_internal.go, line 5066 at r2 (raw file):
if err != nil { return nil, nil, err }
thoughts on adding the memory in use for these stats to the monitor here and then releasing it in the worker? It's not perfect but it's better than nothing.
pkg/sql/sqlstats/sslocal/temp_sql_stats.go, line 51 at r2 (raw file):
// Less implements the sort.Interface interface. func (t txnResponseList) Less(i, j int) bool { return strings.Compare(t[i].StatsData.App, t[j].StatsData.App) == -1
This is usually written as < 0 but also, why not t[i].StatsData.App < t[j].StatsData.App?
roachpb.CollectedTransactionStatistics Previously, roachpb.CollectedTransactionStatistics does not contain TransactionFingerprintID. This results in SQLStatusServer's /_status/statements endpoint not able to return transaction fingerprint id in the response. This commit adds transaction fingerprint id into CollectedTransactionStatistics and simplifies the API in multiple layers. Release note: None
This commit introduces crdb_internal.transaction_statistics virtual table that exposes both cluster-wide in-memory transaction statistics as well as persited transaction statistics. This new virtual table will be used to replace crdb_internal.node_transaction_statistics virtual table, which only surface node-local in-memory transaction statsitics. Follow up to cockroachdb#68715 Release note (sql change): introduced new crdb_internal.transaction_statistics virtual table that surfaces both cluster-wide in-memory transaction statistics as well as persisted transaction statistics. Release justification: Category 4
263c92b to
971bacd
Compare
Azhng
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go, line 5066 at r2 (raw file):
Previously, ajwerner wrote…
thoughts on adding the memory in use for these stats to the monitor here and then releasing it in the worker? It's not perfect but it's better than nothing.
Done. I think it's ok for a stopgap solution for now. Left a TODO here that reference the tracking issue.
pkg/sql/sqlstats/sslocal/temp_sql_stats.go, line 51 at r2 (raw file):
Previously, ajwerner wrote…
This is usually written as
< 0but also, why nott[i].StatsData.App < t[j].StatsData.App?
Done. TIL < works on strings.
|
TFTR! bors r=ajwerner,maryliag |
|
Build succeeded: |
Depends on #68715 and #69022
First Commit
roachpb,sql,server: add TransactionFingerprintID to
roachpb.CollectedTransactionStatistics
Previously, roachpb.CollectedTransactionStatistics does not contain
TransactionFingerprintID. This results in SQLStatusServer's
/_status/statements endpoint not able to return transaction
fingerprint id in the response.
This commit adds transaction fingerprint id into CollectedTransactionStatistics
and simplifies the API in multiple layers.
Release note: None
Second Commit
sql: introduce crdb_internal.transaction_statistics virtual table
This commit introduces crdb_internal.transaction_statistics virtual
table that exposes both cluster-wide in-memory transaction statistics
as well as persited transaction statistics. This new virtual table
will be used to replace crdb_internal.node_transaction_statistics
virtual table, which only surface node-local in-memory transaction
statsitics.
Follow up to #68715
Release justification: Category 4
Release note (sql change): introduced new crdb_internal.transaction_statistics
virtual table that surfaces both cluster-wide in-memory transaction statistics
as well as persisted transaction statistics.