ui: fix stmt not being found #71596
ui: fix stmt not being found #71596
Conversation
When creating a transaction fingerprint text based on statement ids, if the statement was not found, it was crashing and making the Transactions Page to not load. This commit handles the case. Release note (bug fix): Transaction page no longer crashes when a statement is not found.
|
This change is |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @maryliag)
There was a problem hiding this comment.
I don't know for sure, could be a situation where the statement was cleared (or not persisted properly) after the transaction was finished but before the UI needed the info for calculating the text on that particular function (which may explain why sometimes the page was loading normally for me and sometimes not). Once I cleared the stats on that cluster the page loaded normally. It's not something that should be happening (the reason why we never saw this before), so we will continue to investigate but at least this fix makes it so the page doesn't crash
Reviewable status:
|
It's hard to tell. My first instincts was that the the statements stats needed by thye transaction stats got GC'd, but looking at the system table there's so little stats stored there's no way a GC would even be triggering. (Or did we reset SQL Stats already on that cluster?) |
|
@Azhng I did reset the stats (to make the page load again and people would be able to use it until the fix is merged) |
|
Actually now I'm thinking, there might be another culprit: We run an hourly flush async task here. This is new in 21.2 We also run an hourly SQL Stats Reset async task that dumps in-memory stuff into telemetry here. This exists since 21.1 If telemetry job gets to reset the stats before flusher, then the stats will be permanently removed from both I'm voting to just get rid of the telemetry async task all together, and make the flusher async task to dump the stats into the telemetry reporting pool. |
|
The flusher is already doing telemetry dump anyway. |
|
bors r+ |
|
Build succeeded: |
71731: sql: consistent aggregated timestamp when flushing sql stats r=matthewtodd a=Azhng Previously, when SQL Stats are flushed to system table, the aggregatedTs column for SQL Stats is calculated for each stats entry individually. This means that if a flush starts near the end of each hour, it is possible that different stats rows will be assigned two different aggregated timestamp. Currently, flusher flushes statement statistics first, and only when statement statistics are flushed, transaction statistics will be flushed into system table. This means that it is likely the transaction statistics will get assigned a different aggregatedTs than the statement statistics. Consequentially, when the frontend fetches SQL Stats through the CombinedStmtStats handler, the frontend default performs range scan at 1 hour interval. This triggers a range scan on the system table for that 1 hour range. This causes the statement statisitcs that got assigned a different aggregatedTs to be omitted from the result. This commit changed the flusher to only compute aggregatedTs once before the flush actually happen, and assign that aggregatedTs too *all* stmt/txn stats rows. Statements executed in the same aggregation interval can be looked up by the corresponding statement fingerprint ID stored in transaction stats metadata. Follow up to #71596 Release note: None 71897: ui:app filter is has multi select option r=maryliag a=maryliag Previously you could only select one App on the filter for Transaction and Statement page. This commits introduces a multi select option, making possible for the user select several apps at the same time and exclude internal results. This commits also properly sets the filter value for database with no values to (unset). Partially addresses #70544 Not included in this commit: - Select all apps except internal by default - Add app filter to Session tab Before <img width="308" alt="Screen Shot 2021-10-22 at 6 27 25 PM" src="https://user-images.githubusercontent.com/1017486/138529700-012a3530-a7ef-4bbe-bd81-0a0a49c5be61.png"> <img width="278" alt="Screen Shot 2021-10-22 at 6 29 37 PM" src="https://user-images.githubusercontent.com/1017486/138529713-0c9c309d-5255-4fe9-be75-5edb5bdf0580.png"> After <img width="273" alt="Screen Shot 2021-10-22 at 6 27 37 PM" src="https://user-images.githubusercontent.com/1017486/138529626-1966a7c1-fba5-4167-9c17-351cc0f8da3d.png"> <img width="273" alt="Screen Shot 2021-10-22 at 6 29 51 PM" src="https://user-images.githubusercontent.com/1017486/138529726-39ff6c0b-97ec-4ea4-af9f-aa2d3d15ac80.png"> Release note (ui change): App filter on Transaction and Statement pages is now multi select. Co-authored-by: Azhng <[email protected]> Co-authored-by: Marylia Gutierrez <[email protected]>
When creating a transaction fingerprint text based on
statement ids, if the statement was not found, it was
crashing and making the Transactions Page to not load.
This commit handles the case.
Release note (bug fix): Transaction page no longer crashes when
a statement is not found.