txn: seperate the prewrite and commit details information to make it clear #37158
Conversation
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
added
do-not-merge/work-in-progress
|
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/6181d72f71f8fd609ad2034f672fc74503ea3f15 |
cfzjywxk
force-pushed
the
commit_detail
branch
from
db9971a
to
0272d5d
Compare
cfzjywxk
force-pushed
the
commit_detail
branch
from
0272d5d
to
12df785
Compare
ti-chi-bot
removed
the
do-not-merge/work-in-progress
|
/run-check_dev |
| fields = append(fields, zap.String("backoff_types", fmt.Sprintf("%v", commitDetails.Mu.BackoffTypes))) | ||
| if len(commitDetails.Mu.PrewriteBackoffTypes) > 0 { | ||
| fields = append(fields, zap.String("Prewrite_"+BackoffTypesStr, fmt.Sprintf("%v", commitDetails.Mu.PrewriteBackoffTypes))) | ||
| } |
There was a problem hiding this comment.
Do we also need to add CommitBackoffTypes to String and ToZapFields?
There was a problem hiding this comment.
Thanks for the reminding, they're added.
| SlowestCommitRPCDetailStr = "Slowest_commit_rpc_detail" | ||
| // SlowestPrewriteRPCDetailStr means the details of the slowest RPC during the transaction 2pc prewrite process. | ||
| SlowestPrewriteRPCDetailStr = "Slowest_prewrite_rpc_detail" | ||
| // CommitPrimaryRPCDetailStr means the details of the slowest RPC during the transaction 2pc commit process. |
There was a problem hiding this comment.
For the query latency calculation of 2pc committed transactions, the primary key commit is enough.
There was a problem hiding this comment.
I mean the comment message is slightly inaccurate.
| fields = append(fields, zap.String(CommitPrimaryRPCDetailStr, "{total:"+strconv.FormatFloat(commitDetails.Mu.SlowestPrewrite.ReqTotalTime.Seconds(), 'f', 3, 64)+ | ||
| "s, region_id: "+strconv.FormatUint(commitDetails.Mu.SlowestPrewrite.Region, 10)+ | ||
| ", store: "+commitDetails.Mu.SlowestPrewrite.StoreAddr+ | ||
| ", "+commitDetails.Mu.SlowestPrewrite.ExecDetails.String()+"}")) |
There was a problem hiding this comment.
Shall we extract a function for coverting ReqDetailInfo to string? (maybe better to impl String() method for ReqDetailInfo)
There was a problem hiding this comment.
Better to abstract a string function for ReqDetailInfo, it needs to update client-go first maybe we could do it in the later prs.
ti-chi-bot
added
status/LGT2
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 69f97c9
|
ti-chi-bot
added
the
status/can-merge
TiDB MergeCI notify🔴 Bad News! New failing [1] after this pr merged.
|
What problem does this PR solve?
Issue Number: ref #34106
Problem Summary:
Separate the prewrite and commit details in commit information to make it clear.
Depends on the kv client change tikv/client-go#567.
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.