kv: add conflicting txn_meta to error message #111937
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained
-- commits line 4 at r1:
It looks like this:
TransactionRetryWithProtoRefreshError: TransactionRetryError: retry txn (RETRY_SERIALIZABLE - failed preemptive refresh due to conflicting locks on "a" [reason=wait_policy] - conflicting txn: meta={id=628cc31a key="a" iso=Serializable pri=0.04958338 epo=0 ts=1696616855.689312000,0 min=1696616855.689312000,0 seq=1}): "original txn" meta={id=5f395c8f key="b" iso=Serializable pri=0.03612870 epo=0 ts=1696616855.689312000,1 min=1696616855.689310000,0 seq=1} lock=true stat=PENDING rts=1696616855.689310000,0 wto=false gul=1696616856.189310000,0
There was a problem hiding this comment.
The Cluster Observability team requires this error message to also include the conflicting txn_id for surfacing this error up to the client dashboard.
Is the db console parsing the error message instead of using the structured TransactionRetryWithProtoRefreshError proto?
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:
pkg/kv/kvclient/kvcoord/dist_sender_server_test.go line 4660 at r1 (raw file):
require.NotNil(t, tErr.ConflictingTxn) require.Equal(t, txn2.ID(), tErr.ConflictingTxn.ID) require.True(t, strings.Contains(tErr.Error(), tErr.ConflictingTxn.String()))
Would require.Regexp be a better fit?
This patch adds back the conflicting txn_meta to the error message printed out from a `TransactionRetryError` if available. Fixes cockroachdb#110689. Release note: None
aadityasondhi
force-pushed
the
20231004.contention-txn-id-error-msg
branch
from
390810a to
131572b
Compare
There was a problem hiding this comment.
I believe this is being used to display the error as-is to the user. It also allows us to show conflicting txn in the SQL shell if this error is thrown. I would expect db console to use the actual field in the structured error for any other purpose. @xinhaoz can probably confirm.
Reviewable status:
pkg/kv/kvclient/kvcoord/dist_sender_server_test.go line 4660 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Would
require.Regexpbe a better fit?
yeah, made the change. Thanks!
|
TFTR! bors r=nvanbenschoten |
|
@aadityasondhi Yes, that's right, we won't be parsing this value at all and are reading from |
There was a problem hiding this comment.
Reviewable status:
|
Build succeeded: |
This patch adds back the conflicting txn_meta to the error message printed out from a
TransactionRetryErrorif available.Fixes #110689.
Release note: None