sql: keep track of statement error code and error message in crdb_internal.execution_insights #95856
Conversation
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
There was a problem hiding this comment.
Looking good! Can we add some tests? Also just a small question on naming - do we need the prefix Last here? I feel like it's appropriate here in just shortening them to error and errorCode.
Reviewed 1 of 8 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @gtr)
pkg/sql/sqlstats/insights/registry.go line 129 at r1 (raw file):
} insight.Transaction.Problems = addProblem(insight.Transaction.Problems, s.Problem) // TODO(gerardo): bubble up stmt failed execution error codes
Is this TODO going to be addressed in this PR? If not maybe we can file a ticket for it and remove this TODO, as there's no additions here you need to come back and change.
There was a problem hiding this comment.
Thanks! Working on adding tests now. Also, good suggestion on simplifying the names.
Reviewable status:
pkg/sql/sqlstats/insights/registry.go line 129 at r1 (raw file):
Previously, xinhaoz (Xin Hao Zhang) wrote…
Is this TODO going to be addressed in this PR? If not maybe we can file a ticket for it and remove this TODO, as there's no additions here you need to come back and change.
I don't think it'll be addressed in this PR, so I'll go ahead and remove it and file a ticket.
crdb_internal.execution_insights Part of: cockroachdb#87785, cockroachdb#94381. Previously, the insights subsystem did not keep track of error code and error messages for failed executions and only contained a `FailedExecution` value for the `problem` field. This commit adds the `last_error_code` and `last_error_msg` columns to the `crdb_internal.cluster_execution_insights` virtual table. The next PR will focus on displaying these values in the UI. Release note (sql change): Added `last_error_code` and `last_error_msg` columns to the `crdb_internal.cluster_execution_insights` virtual table which contain the error code and error message for a failed statement execution, respectively.
|
As discussed offline, focus one PR on adding the information to statement and transaction statistics, then another PR on adding the information to insights, then another focusing on capturing all failed executions. This way is more clear, easy for you to develop and easier to review. |
Added the information to the statement statistics table on this PR. Closing this one and saving these changes for later. Thanks! |
Part of: #87785, #94381.
Previously, the insights subsystem did not keep track of error code and error messages for failed executions and only contained a
FailedExecutionvalue for theproblemfield. This commit adds theerror_codeanderror_msgcolumns to thecrdb_internal.cluster_execution_insightsvirtual table. The next PR will focus on displaying these values in the UI.Example:
Release note (sql change): Added
error_codeanderror_msgcolumns to thecrdb_internal.cluster_execution_insightsvirtual table which contain the error code and error message for a failed statement execution, respectively.