Skip to content
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

sql, sqlstats: execution statistics count for explicit transactions are inflated #123692

Open
xinhaoz opened this issue May 6, 2024 · 0 comments
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-observability

Comments

@xinhaoz
Copy link
Member

xinhaoz commented May 6, 2024

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 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 check the value of instrumentationHelper.collectExecStats 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-37544

@xinhaoz xinhaoz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-observability labels May 6, 2024
@xinhaoz xinhaoz changed the title sql, sqlstats: last sampled time not inaccurately tracked for COMMIT TRANSACTION sql, sqlstats: last sampled time inaccurately tracked for COMMIT TRANSACTION May 6, 2024
@xinhaoz xinhaoz changed the title sql, sqlstats: last sampled time inaccurately tracked for COMMIT TRANSACTION sql, sqlstats: execution statistics count for explicit transactions are inflated May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-observability
Projects
None yet
Development

No branches or pull requests

2 participants