cockroachdb / cockroachdb/cockroach
sql, sqlstats: execution statistics count for explicit transactions are inflated
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
We should investigate if setting up the instrumentation helper for `COMMIT TRANSACTION` statements is intentional. At the very least, the last sampled time is not being accurately tracked for these statements in the instrumentation helper setup, leading to incorrect execution statistics metrics for transactions.
The last sampled time for a stmt is tracked in a map within the sql stats container (stmt fingerprint -> last sampled time). This map is updated at the time of sql stats recording. During instrumentation setup, we check the [last sampled time](https://github.com/cockroachdb/cockroach/blob/6f998c90fef64c5ca4478d10970b2152819f9d55/pkg/sql/instrumentation.go#L449) to determine if this is the first time we're seeing this fingerprint, and sample the statement if so, however, we do not write any sql stats for `COMMIT TRANSACTION` statements.
This leads to always sampling `COMMIT TRANSACTION` statements. A consequence of this is that we are also inflating the count of the execution_statistics for explicit transactions. At the time of recording transaction level stats, we c[heck the value of instrumentationHelper.collectExecStats](https://github.com/cockroachdb/cockroach/blob/4a7029631bbec84e06e6325655019ac83d878120/pkg/sql/conn_executor_exec.go#L3395) to record another entry of exec stats, which is now incorrectly represented to be for every execution of the transaction.
Note: This behaviour may extend to other 'special case' statements other than COMMIT. Investigate to see if this is the case.
Jira issue: CRDB-38475
Epic CRDB-39776
Contributor guide
Assessment
This issue has not been assessed yet.