sql/contention: add blocking statement to contention event tables#172178
sql/contention: add blocking statement to contention event tables#172178parkerswy wants to merge 2 commits into
Conversation
The contention event tables identify the blocking transaction (txn_id) but not the statement within that transaction that held the lock. This plumbs a blocking statement fingerprint through to the transaction contention event surface. Plumbing only: - Adds `blocking_stmt_fingerprint_id` to the `ContentionEvent` proto. - Adds a `blocking_stmt_fingerprint_id` column to `crdb_internal.transaction_contention_events`. Epic: CRDB-65143 Fixes: cockroachdb#172008 Release note: None
parkerswy
requested review from
angles-n-daemons,
himanshu-shrivastava-crl and
kyle-a-wong
and removed request for
a team
Release note: None
kyle-a-wong
left a comment
kyle-a-wong
left a comment
There was a problem hiding this comment.
I see that you have a second commit to fix some lint issues. Typically we want commits in a PR to land as atomically, so if there is a bug, lint error, etc., you should fix that commit instead of creating a new commit on top of it to fix it. To do this, you would make the fix, and then do a git commit --amend, which amends the staged changes to the HEAD of your branch. One thing to note about this is that it replaces the old commit, so doing a git push fork branching-stmt-fingerprint-id will be rejected and you will need to do a force push (include the -f flag.
| expected gosql.NullString | ||
| }{ | ||
| { | ||
| name: "non-null", |
There was a problem hiding this comment.
nit: lets name these something like "blocking_stmt_fingerprint_not_exists" and "blocking_stmt_fingerprint_exists"
| srv, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{ | ||
| DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant, | ||
| }) |
There was a problem hiding this comment.
nit: im not sure if this DefaultTestTenant is needed. I think you can just do serverutils.StartServer(t, base.TestServerArgs{})
| tree.DBytes(sqlstatsutil.EncodeUint64ToBytes(uint64(resp.Events[i].BlockingTxnFingerprintID)))) | ||
|
|
||
| blockingStmtFingerprintID := tree.DNull | ||
| if resp.Events[i].BlockingEvent.BlockingStmtFingerprintID != 0 { |
There was a problem hiding this comment.
nit: you can pull this out into a variable to make the if block a little more readable
2 participants
The contention event tables identify the blocking transaction (txn_id) but not the statement within that transaction that held the lock. This plumbs a blocking statement fingerprint through to the transaction contention event surface.
Plumbing only:
blocking_stmt_fingerprint_idto theContentionEventproto.blocking_stmt_fingerprint_idcolumn tocrdb_internal.transaction_contention_events.Epic: CRDB-65143
Fixes: #172008
Release note: None