-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release-23.1: server: fix total latency time on sql stats call #117496
release-23.1: server: fix total latency time on sql stats call #117496
Conversation
For the top activity tables, we have a column `execution_total_cluster_seconds` with the total latency of all executions of that period. This value is important because the top activity table have up to 500 rows per aggregated timestamp, so if we add all latencies from it, we won't have the total, only the total of the top. We then have a column that has that total of that hour which is based on the main table. Previously, when making a request to the top activity table we were returning only of of those values as the total, which works as expected if your select is in a single aggregation period. The majority of requests won't be this way, so we need to add the values from all aggregated period selects. This commit update the query for the total select to get one value from each aggregated_ts and then adding those. Part Of CRDB-34884 Release note (bug fix): Fix value used for the total runtime on sql stats, which was using the wrong value previously, causing the UI to display values with more than 100%.
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @abarganier)
Backport 1/1 commits from #116976.
/cc @cockroachdb/release
For the top activity tables, we have a column
execution_total_cluster_seconds
with the total latency of all executions of that period. This value is important because the top activity table have up to 500 rows per aggregated timestamp, so if we add all latencies from it, we won't have the total, only the total of the top. We then have a column that has that total of that hour which is based on the main table.Previously, when making a request to the top activity table we were returning only of of those values as the total, which works as expected if your select is in a single aggregation period. The majority of requests won't be this way, so we need to add the values from all aggregated period selects.
This commit update the query for the total select to get one value from each aggregated_ts and then adding those.
Part Of CRDB-34884
Release note (bug fix): Fix value used for the total runtime on sql stats, which was using the wrong value previously, causing the UI to display values with more than 100%.
Release justification: bug fix