sql: add waiting stmt info to transaction_contention_events #95061
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 5 of 6 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @j82w and @msirek)
pkg/sql/contentionpb/contention.go line 95 at r2 (raw file):
// since uint64 is 8-byte. This is why we decode the byte array twice and add // the resulting uint64 into the fnv each time. func hashClusterUniqueID(id clusterunique.ID, hash *util.FNV64) {
You might want to update the comment for this function. It looks like it was copied over from hashUUID().
There was a problem hiding this comment.
Reviewable status:
-- commits line 11 at r3:
nit: the format should be Release note (sql change): adds .... IIUC the format matters because we have an automated script that docs writers use to populate the release notes that we send out, and if a release note in the commit message doesn't match the expected format, then the docs writer will need to do more manual stuff.
pkg/sql/crdb_internal.go line 6348 at r3 (raw file):
contention_duration INTERVAL NOT NULL, contending_key BYTES NOT NULL,
nit: looks like dangling spaces.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/contentionpb/contention_test.go line 19 at r3 (raw file):
"github.com/cockroachdb/cockroach/pkg/util/uuid" "github.com/stretchr/testify/require" "testing"
nit: Re-order "testing" import to the top of import list.
There was a problem hiding this comment.
Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status:
pkg/sql/contentionpb/contention.go line 93 at r4 (raw file):
// hashClusterUniqueID adds the hash of the clusterunique.ID into the fnv. // A clusterunique.ID is an uint128. To hash we treat it as two uint64 integers, // since uint128 is has a lo and hi uint64.
"since uint128 is has" -> "since uint128 has"
There was a problem hiding this comment.
Reviewed 2 of 6 files at r1, 3 of 4 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status:
There was a problem hiding this comment.
Reviewable status:
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: the format should be
Release note (sql change): adds .... IIUC the format matters because we have an automated script that docs writers use to populate the release notes that we send out, and if a release note in the commit message doesn't match the expected format, then the docs writer will need to do more manual stuff.
Also, if you are to have a release note, please make it a fuller sentence that a user can understand and make use of.
I would not add a note.
-- commits line 4 at r6:
what's the difference between a statement ID and a fingerprint ID ?
pkg/sql/clusterunique/id.go line 96 at r6 (raw file):
return err } id.Hi = uint128.Hi
*id.Uint128 = uint128 ?
Previously, andreimatei (Andrei Matei) wrote…
A statement fingerprint represents one or more SQL statements by replacing literal values (e.g., numbers and strings) with underscores (_). This can help you quickly identify frequently executed SQL statements and their latencies. https://www.cockroachlabs.com/docs/stable/ui-statements-page.html#sql-statement-fingerprints Statement ID is a unique identifier for the specific statement execution. |
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal_test.go line 1002 at r6 (raw file):
const defaultIdString = "0x0000000000000000" if blockingTxnId == defaultIdString { return fmt.Errorf("transaction_contention_events had default txn id %s, waiting txn id %s", blockingTxnId, waitingTxnId)
Change to: "transaction_contention_events had default blocking txn id..."
There was a problem hiding this comment.
Reviewable status:
Previously, andreimatei (Andrei Matei) wrote…
Also, if you are to have a release note, please make it a fuller sentence that a user can understand and make use of.
I would not add a note.
I removed the note as suggested.
pkg/sql/clusterunique/id.go line 96 at r6 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
*id.Uint128 = uint128?
Done.
pkg/sql/contentionpb/contention.go line 95 at r2 (raw file):
Previously, gtr (Gerardo Torres Castro) wrote…
You might want to update the comment for this function. It looks like it was copied over from
hashUUID().
Done.
pkg/sql/contentionpb/contention.go line 93 at r4 (raw file):
Previously, msirek (Mark Sirek) wrote…
"since uint128 is has" -> "since uint128 has"
Done.
pkg/sql/crdb_internal_test.go line 1002 at r6 (raw file):
Previously, msirek (Mark Sirek) wrote…
Change to: "transaction_contention_events had default blocking txn id..."
Done.
This adds the 'waiting_stmt_id' and 'waiting_stmt_fingerprint_id' to the transaction_contention_events. This makes it easier for users to figure out what the issue is when dealing with large transactions. part of: #91665 Release note: none
|
bors r+ |
|
Build succeeded: |
This adds the 'waiting_stmt_id' and 'waiting_stmt_fingerprint_id' to the transaction_contention_events. This makes it easier for users to figure out what the issue is when dealing with large transactions.
part of: #91665
Release note: sql: adds the 'waiting_stmt_id' and
'waiting_stmt_fingerprint_id' to the transaction_contention_events