ui: Transaction page inconsistent grouping of statement stats

[sheaffej]

I saw this while working with a customer, but have not been able to reproduce the behavior.

On the Transactions page in 20.2, we see a transaction with 2 similar statements. In the statement stats list (bottom of screen shot) the statement is listed twice, however the first listing has an execution count of 2 and the second has an execution count of 0. So it seems like some of the grouping and/or display logic in the statement stats list is mixing up the stats...or is displaying the statement twice when it should only be listed one with the aggregate stats.

Epic: CRDB-9867

[awoods187]

This seems related to https://github.com/cockroachlabs/support/issues/793

[Azhng]

Currently started with the approach where we create a temporary in-memory SQL Stats to record all statements within an ongoing transaction, then flush the transaction to the actual SQL Stats once transaction is either committed or aborted. (This is when transaction fingerprint ID becomes available).

I realized an issue with the approach with using the temp SQL Stats approach while implementing it.

SET application_name = "app1";

BEGIN;

SELECT 1;                      -- stmt1
SELECT 1,1;                    -- stmt2  

SET application_name = "app2"; -- stmt3

SELECT 1,1,1;                  -- stmt4

COMMIT;

Our current behaviour would result stmt1 and stmt2 stored with application_name = app1. stmt3 and stmt4 are stored under application_name = app2. This is because we would swap out the application statistics object from app1 to app2 when the SET application_name is executed.

This introduce a problem for a new approach, when we swap out the application statistics object, we still don't have all the information to compute transaction fingerprint ID yet. This leaves us with few options:

1. we do nothing, and we flush the stats from the Temp SQL Stats to the actual SQL Stats once the transaction is done executing. This is the easiest approach, but alters our behaviour when dealing with SET application_name = ...
2. we maintain a list of application statistics objects in the Temp SQL Stats, and we keep track which application name is each statement executed under. We flush them to the corresponding application statistics object when the transaction is done executing. Personally, I'm not a fan of this option, it's very complicated and we need a lot of book keeping to ensure the correctness.
3. we try a new approach: we don't create Temp SQL Stats at all. We would directly write all statement statistics into the main SQL Stats with a temporary transaction fingerprint ID. When the transaction is done executing, since we know the statement fingerprint IDs associated with that transaction and we know the transaction fingerprint ID, we go back to look up each individual statement statistics containing the new transaction fingerprint ID and we update the transaction fingerprint ID field. However, this introduces problems when the statement statistics are flushed to disk before we can come back to perform updates. Then we now just have inconsistent data. This can be addressed via some sort of special marker to inform the flusher to not flush statement statistics with temporary transaction fingerprint ID, but it's just introducing a more complexity.

My personal take: I would go with the first option. Since application_name is mostly passed in through connection string, using the SET application_name = ... statements within a transaction is an extremely weird thing to do. I personally feel like spending time and effort to support the backward compatibility with this weird usage of SET application_name = ... within a transaction. I would be really surprised to find users who are depending on that old behaviour and I'd argue it is fine for us to break the backward compatibility.