ui: fix statement diag reports when min exec latency is null #139342
Conversation
|
This change is |
|
Before fix (point at drt-chaos with |
kyle-a-wong
added
backport-24.3.x
kyle-a-wong
force-pushed
the
fix_statement_bundle
branch
from
402ec45 to
b1d462d
Compare
There was a problem hiding this comment.
TY for fixing! Can you also add to the stmt diags cypress test so we can catch this whenn we eventually enable the test on the roachprod cypress tests?
This bug is very interesting because the API has always returned "0s" when the min_execution_latency col is null. I wonder if something else changed recently in how we decode the protobuf to js.
kyle-a-wong
force-pushed
the
fix_statement_bundle
branch
from
b1d462d to
7e362bc
Compare
Good idea, I'll add this as a separate PR, since it doesn't need to be back ported.
Yeah that's still unclear to me. I'm working on identifying when this started happening |
A bug in db console was resulting in statement diagnostics reports to not work as intended. As a result, activating diagnostics didn't result in the intended state change which showed a user that a diagnostics report is running or downloadble. This was happening in edge cases where reports "minExecutionLatency" response field was null, but the db console expected it to be populated. Now, db console should handle this edge case. This commit also adds nanosecond granularity to the returned StatementDiagnosticsResponse.min_execution_latency field. Previously only the seconds portion of the statement's min_execution_latency was converted into the returned objects duration, but often times this latency will be sub 1 second, resulting in "O" to be returned. Fixes: cockroachdb#139340 Epic: none Release note (bug fix): Fixes a bug where sometimes activating diagnostics for sql activity appears unresponsive, with no state or status update upon activating. Now, the status should always reflect that diagnosticsa are active or that a statement bundle is downloadable.
kyle-a-wong
force-pushed
the
fix_statement_bundle
branch
from
7e362bc to
e84396c
Compare
|
TFTR bors r+ |
139342: ui: fix statement diag reports when min exec latency is null r=kyle-a-wong a=kyle-a-wong A bug in db console was resulting in statement diagnostics reports to not work as intended. As a result, activating diagnostics didn't result in the intended state change which showed a user that a diagnostics report is running or downloadble. This was happening in edge cases where reports "minExecutionLatency" response field was null, but the db console expected it to be populated. Now, db console should handle this edge case. Fixes: #139340 Epic: none Release note (bug fix): Fixes a bug where sometimes activating diagnostics for sql activity appears unresponsive, with no state or status update upon activating. Now, the status should always reflect that diagnosticsa are active or that a statement bundle is downloadable. 139487: crosscluster/logical: permanent job errors should fail LDR job r=kev-cao a=msbutler Previously, permanent job errors would pause the LDR job, like PCR. Since LDR doesn't have a cutover step, we should instead fail the job to provide a clearer UX to the user. Epic: none Release note: none 139491: crosscluster/physical: wait for sip shutdown before cutover r=kev-cao a=msbutler Informs #136588 Release note: none Co-authored-by: Kyle Wong <[email protected]> Co-authored-by: Michael Butler <[email protected]>
|
Build failed (retrying...): |
|
bors retry |
|
Already running a review |
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches. Issue #139340: branch-release-24.3, branch-release-25.1. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
After investigation, it was determined that this issue was caused as a result of this PR: #137479, where a change was introduced to not serialize zero-valued durations. This results in the decoding of the duration field to be null on the client side |
A bug in db console was resulting in statement diagnostics reports to not work as intended. As a result, activating diagnostics didn't result in the intended state change which showed a user that a diagnostics report is running or downloadble.
This was happening in edge cases where reports "minExecutionLatency" response field was null, but the db console expected it to be populated. Now, db console should handle this edge case.
Fixes: #139340
Epic: none
Release note (bug fix): Fixes a bug where sometimes activating diagnostics for sql activity appears unresponsive, with no state or status update upon activating. Now, the status should always reflect that diagnosticsa are active or that a statement bundle is downloadable.