remove "failed" as part of the fingerprint creation #97176
Assignees
Labels
A-sql-observability
Related to observability of the SQL layer
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-observability
Projects
Comments
maryliag
added
C-enhancement
|
I'm going to move this to backlog for now to evaluate for 23.2 since there are broader implications (and improvements) we can consider. Related issue: #69785 |
abarganier
added a commit
to abarganier/cockroach
that referenced
this issue
Mar 15, 2024
In the SQL stats feature, we consider whether the statement failed as part of the statement key/statement fingerprint ID, which is used to bucket statement statistics aggregations. This contributes to the cardinality of statement statistics by a constant factor of 2x. A statement that has failed will show up twice in the SQL Activity page, once for the successful executions, and once for the failed executions. Despite not being very useful, the contributions it makes to cardinality are detrimental to the SQL activity feature, as it has a non-negligible impact on data volume. To improve upon this, we should remove the failure bool from the statement key, and instead include failure counts as part of the statistics object. This patch does the following: * Removes the failure boolean from the statement key * Removes failure counts from the statement statistics metadata * Moves the failure counts into the aggrgeated statement statistics * Updates the Statement Details page to display "Failure Count" instead of "Failed?", with the associated numeric count instead of a bool val. Addresses: cockroachdb#97176 Release note (ui change): In the DB Console's SQL Activity page, the Statement Details page will now show "Failure Count" in place of "Failed?", showing the number of failed executions for the given statement fingerprint instead of a "Yes"/"No" value. Furthermore, previously in the SQL Activity table the same statement fingeprint would appear twice - once for failed executions, and once for successful executions. With these changes, we no longer consider these to be separate rows. Instead, we combine them into a single statement fingerprint. Release note (sql change): A new column has been added to `crdb_internal.node_statement_statistics`, `failure_count INT NOT NULL`. It represents the number of recorded statement execution failures for the given statement, as a new component of the overall statistics.
craig bot
pushed a commit
that referenced
this issue
Mar 15, 2024
120236: pkg/sql/sqlstats: move failures out of stmt key/meta, into stats r=dhartunian a=abarganier Addresses: #97176 Epic: CRDB-24527 In the SQL stats feature, we consider whether the statement failed as part of the statement key/statement fingerprint ID, which is used to bucket statement statistics aggregations. This contributes to the cardinality of statement statistics by a constant factor of 2x. A statement that has failed will show up twice in the SQL Activity page, once for the successful executions, and once for the failed executions. Despite not being very useful, the contributions it makes to cardinality are detrimental to the SQL activity feature, as it has a non-negligible impact on data volume. To improve upon this, we should remove the failure bool from the statement key, and instead include failure counts as part of the statistics object. This patch does the following: * Removes the failure boolean from the statement key * Removes failure counts from the statement statistics metadata * Moves the failure counts into the aggrgeated statement statistics * Updates the Statement Details page to display "Failure Count" instead of "Failed?", with the associated numeric count instead of a bool val. Release note (ui change): In the DB Console's SQL Activity page, the Statement Details page will now show "Failure Count" in place of "Failed?", showing the number of failed executions for the given statement fingerprint instead of a "Yes"/"No" value. Furthermore, previously in the SQL Activity table the same statement fingeprint would appear twice - once for failed executions, and once for successful executions. With these changes, we no longer consider these to be separate rows. Instead, we combine them into a single statement fingerprint. Release note (sql change): A new column has been added to `crdb_internal.node_statement_statistics`, `failure_count INT NOT NULL`. It represents the number of recorded statement execution failures for the given statement, as a new component of the overall statistics. 120517: dev: if a `PKG` is supplied, only build that package with lints enabled r=celiala a=rickystewart The idea here is that if you supply a package, you want to see lint results for that package, regardless of whether you've passed in `--short`. Closes #119927. Epic: none Release note: None Co-authored-by: Alex Barganier <abarganier@cockroachlabs.com> Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
exalate-issue-sync
bot
added
T-sql-observability
T-observability
and removed
T-observability
T-sql-observability
labels
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We should not use "failed" as part of the key for a fingerprint, so we can easily see all status for the same fingerprint together. This would also reduce the amount of different fingerprints
Jira issue: CRDB-24560
Epic: CRDB-24527
The text was updated successfully, but these errors were encountered: