insights: record more txn insights#93079
Conversation
|
This change is |
maryliag
left a comment
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @xinhaoz)
pkg/sql/executor_statement_metrics.go line 211 at r1 (raw file):
EndTime: phaseTimes.GetSessionPhaseTime(sessionphase.PlannerStartExecStmt).Add(svcLatRaw), FullScan: fullScan, SessionData: planner.SessionData(),
I don't see the commit mentions the removal of the sessions data, is this what you mean by user/app/database?
|
Previously, maryliag (Marylia Gutierrez) wrote…
Yes that's correct, that info was coming from the presence of the session data in the statements record. |
398fed3 to
7129c3b
Compare
matthewtodd
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go line 6697 at r2 (raw file):
tree.NewDString(insight.Transaction.User), tree.NewDString(insight.Transaction.ApplicationName), tree.NewDString(insight.Transaction.Database),
Great! Do you want to add the new values to the table, too, or is that for another commit?
|
Previously, matthewtodd (Matthew Todd) wrote…
I'm going to split the table in the next PR. This is just to ensure the behaviour of the current exec insights doesn't change in this PR (and it seems fine to keep these fields in the stmt table version in addition to adding them to the incoming txns table, LMK if you think differently). |
maryliag
left a comment
There was a problem hiding this comment.
Reviewed 2 of 3 files at r2, all commit messages.
Reviewable status:
j82w
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go line 6697 at r2 (raw file):
tree.NewDString(insight.Transaction.User), tree.NewDString(insight.Transaction.ApplicationName), tree.NewDString(insight.Transaction.Database),
Is it possible for a single transaction to span multiple dbs?
|
Previously, j82w wrote…
Hm I believe so, so this is probably more of a stmt-level stat. I'll move it back |
matthewtodd
left a comment
There was a problem hiding this comment.
Reviewable status:
072bf51 to
ebd73c0
Compare
Closes cockroachdb#93076 This commit adds the following fields at the txn level when recording insights in the sql insights system: - contention: total txn contention time - start_time: txn start time - end_time: txn end time - auto_retry_reason: last reason for auto txn retry - retry_count - rows_written - rows_read In addition, the following fields have been moved from recording at the stmt level to recording at the txn level for insights: - user - application_name Release note: None
ebd73c0 to
69f16c6
Compare
|
bors r+ |
|
Build succeeded: |
Closes #93076
This commit adds the following fields at the txn level when recording
insights in the sql insights system:
In addition, the following fields have been moved from recording at
the stmt level to recording at the txn level for insights:
Release note: None