*: Log SQL statement when coprocessor encounteres lock #27735

MyonKeminta · 2021-09-01T12:07:04Z

What problem does this PR solve?

Issue Number: close #27718

Problem Summary: The debug log on coprocessor encounters lock doesn't print out the statement, which makes it difficult to know what statement was affected by the lock. This PR tries to add the log.

What is changed and how it works?

What's Changed: Added a callback parameter to the Send method of kv.Client that accepts a new TransactionEvent object. This avoids to pass the SQL statement deep down to the coprocessor client part. And I think this callback and events can also be reused by Lock View.

How it Works:

Check List

Tests

Manual test (add detailed scripts or steps below) (TODO)

Side effects

Performance regression: Consumes more CPU

Documentation

None

Release note

None.

ti-chi-bot · 2021-09-01T12:07:06Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

cfzjywxk
sticnarf

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

…nta/tidb into m/log-stmt-on-cop-error

…tmt-on-cop-error

longfangsong · 2021-09-08T02:51:02Z

util/trxevents/trx_events.go

+// TransactionEvent represents a transaction event that may belong to any of the possible types.
+type TransactionEvent struct {
+	eventType EventType
+	ptr       unsafe.Pointer


Why we have to use unsafe.Pointer here? (I don't know which one is better, unsafe.Pointer or interface{})

It seems either can be used, but interface{} looks better. I thought about avoiding the RTTI (though I'm not sure if there is) but I think perhaps that's a bad idea. I'll change it to interface{} then, please wait a minute

longfangsong

Just a small question. Rest LGTM

ti-chi-bot · 2021-09-08T02:53:45Z

@longfangsong: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

Just a small question. Rest LGTM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

longfangsong · 2021-09-08T02:55:52Z

@longfangsong: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

PTAL pingcap/community#544

MyonKeminta · 2021-09-08T05:34:37Z

@sticnarf @cfzjywxk Also PTAL, thanks

MyonKeminta · 2021-09-08T08:07:38Z

/merge

MyonKeminta · 2021-09-08T08:48:59Z

/run-check_dev_2

MyonKeminta · 2021-09-08T09:46:35Z

/run-check_dev_2

ti-srebot · 2021-09-09T05:34:58Z

cherry pick to release-5.2 in PR #27924

ti-srebot · 2021-09-09T05:35:43Z

cherry pick to release-5.1 in PR #27925

ti-srebot · 2021-09-09T05:36:21Z

cherry pick to release-4.0 in PR #27926

ti-srebot · 2021-09-09T05:37:04Z

cherry pick to release-5.0 in PR #27927

Signed-off-by: ti-srebot <[email protected]>

*: Log SQL statement when coprocessor encounteres lock

096ea39

MyonKeminta added the sig/transaction SIG:Transaction label Sep 1, 2021

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 1, 2021

MyonKeminta mentioned this pull request Sep 1, 2021

*: Log SQL statement when coprocessor encounteres lock #27737

Closed

2 tasks

MyonKeminta marked this pull request as ready for review September 6, 2021 09:37

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 6, 2021

MyonKeminta and others added 4 commits September 6, 2021 17:37

Merge branch 'master' into m/log-stmt-on-cop-error

a62a561

remove redact

110da2f

Merge branch 'm/log-stmt-on-cop-error' of https://github.com/MyonKemi…

671cf27

…nta/tidb into m/log-stmt-on-cop-error

Refactor to use callbacks

b6136fc

ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 7, 2021

Merge branch 'master' of https://github.com/pingcap/tidb into m/log-s…

ad8599d

…tmt-on-cop-error

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 7, 2021

MyonKeminta requested review from longfangsong and sticnarf September 7, 2021 09:22

MyonKeminta and others added 2 commits September 7, 2021 17:53

fix ci

a3ae7e2

Merge branch 'master' into m/log-stmt-on-cop-error

329ef63

longfangsong reviewed Sep 8, 2021

View reviewed changes

longfangsong approved these changes Sep 8, 2021

View reviewed changes

Address comments

0e61dbb

MyonKeminta requested a review from cfzjywxk September 8, 2021 05:34

cfzjywxk approved these changes Sep 8, 2021

View reviewed changes

Merge branch 'master' into m/log-stmt-on-cop-error

ef54d74

ti-chi-bot merged commit 649ed6a into pingcap:master Sep 8, 2021

MyonKeminta deleted the m/log-stmt-on-cop-error branch September 9, 2021 05:11

MyonKeminta added needs-cherry-pick-4.0 needs-cherry-pick-release-5.0 needs-cherry-pick-release-5.1 needs-cherry-pick-release-5.2 labels Sep 9, 2021

ti-srebot mentioned this pull request Sep 9, 2021

*: Log SQL statement when coprocessor encounteres lock (#27735) #27924

Merged

2 tasks

ti-srebot mentioned this pull request Sep 9, 2021

*: Log SQL statement when coprocessor encounteres lock (#27735) #27925

Merged

2 tasks

ti-srebot mentioned this pull request Sep 9, 2021

*: Log SQL statement when coprocessor encounteres lock (#27735) #27926

Merged

2 tasks

ti-srebot mentioned this pull request Sep 9, 2021

*: Log SQL statement when coprocessor encounteres lock (#27735) #27927

Merged

2 tasks

MyonKeminta added a commit to ti-srebot/tidb that referenced this pull request Sep 15, 2021

cherry pick pingcap#27735 to release-5.1

15c02c1

Signed-off-by: ti-srebot <[email protected]>

ti-chi-bot pushed a commit that referenced this pull request Sep 16, 2021

*: Log SQL statement when coprocessor encounteres lock (#27735) (#27925)

deab9fb

MyonKeminta added a commit to ti-srebot/tidb that referenced this pull request Sep 23, 2021

cherry pick pingcap#27735 to release-5.2

aa43a78

Signed-off-by: ti-srebot <[email protected]>

MyonKeminta added a commit to ti-srebot/tidb that referenced this pull request Sep 23, 2021

cherry pick pingcap#27735 to release-5.1

9ea7bb7

Signed-off-by: ti-srebot <[email protected]>

MyonKeminta added a commit to ti-srebot/tidb that referenced this pull request Sep 23, 2021

cherry pick pingcap#27735 to release-5.1

b8ba4f9

Signed-off-by: ti-srebot <[email protected]>

ti-chi-bot pushed a commit that referenced this pull request Oct 19, 2021

*: Log SQL statement when coprocessor encounteres lock (#27735) (#27924)

333edd8

ti-chi-bot pushed a commit that referenced this pull request Nov 19, 2021

*: Log SQL statement when coprocessor encounteres lock (#27735) (#27926)

7521d32

ti-chi-bot pushed a commit that referenced this pull request Dec 16, 2021

*: Log SQL statement when coprocessor encounteres lock (#27735) (#27927)

8dd600b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: Log SQL statement when coprocessor encounteres lock #27735

*: Log SQL statement when coprocessor encounteres lock #27735

MyonKeminta commented Sep 1, 2021 •

edited

Loading

ti-chi-bot commented Sep 1, 2021 •

edited

Loading

longfangsong Sep 8, 2021 •

edited

Loading

MyonKeminta Sep 8, 2021

longfangsong left a comment

ti-chi-bot commented Sep 8, 2021

longfangsong commented Sep 8, 2021

MyonKeminta commented Sep 8, 2021

MyonKeminta commented Sep 8, 2021

MyonKeminta commented Sep 8, 2021

MyonKeminta commented Sep 8, 2021

ti-srebot commented Sep 9, 2021

ti-srebot commented Sep 9, 2021

ti-srebot commented Sep 9, 2021

ti-srebot commented Sep 9, 2021

*: Log SQL statement when coprocessor encounteres lock #27735

*: Log SQL statement when coprocessor encounteres lock #27735

Conversation

MyonKeminta commented Sep 1, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Sep 1, 2021 • edited Loading

longfangsong Sep 8, 2021 • edited Loading

Choose a reason for hiding this comment

MyonKeminta Sep 8, 2021

Choose a reason for hiding this comment

longfangsong left a comment

Choose a reason for hiding this comment

ti-chi-bot commented Sep 8, 2021

longfangsong commented Sep 8, 2021

MyonKeminta commented Sep 8, 2021

MyonKeminta commented Sep 8, 2021

MyonKeminta commented Sep 8, 2021

MyonKeminta commented Sep 8, 2021

ti-srebot commented Sep 9, 2021

ti-srebot commented Sep 9, 2021

ti-srebot commented Sep 9, 2021

ti-srebot commented Sep 9, 2021

MyonKeminta commented Sep 1, 2021 •

edited

Loading

ti-chi-bot commented Sep 1, 2021 •

edited

Loading

longfangsong Sep 8, 2021 •

edited

Loading