-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
*: record execute runtime information in slow log #17573
Conversation
No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md |
… into slow-log-plan-info
Signed-off-by: crazycs <crazycs520@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
/run-all-tests |
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #17573 +/- ##
===========================================
Coverage 79.4174% 79.4174%
===========================================
Files 520 520
Lines 139973 139973
===========================================
Hits 111163 111163
Misses 19819 19819
Partials 8991 8991 |
analyzeInfo = "time:0ns, loops:0" | ||
} | ||
switch p.(type) { | ||
case *PhysicalTableReader, *PhysicalIndexReader, *PhysicalIndexLookUpReader: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about PhysicalIndexMergeReader?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just extract those code from prepareOperatorInfo
, I'm not sure whether it is a bug.... @qw4990 PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can ignore this problem here since it's another thing, the PhysicalIndexMergeReader
doesn't support collecting read-stats now. @wjhuang2016 @crazycs520
Signed-off-by: crazycs <crazycs520@gmail.com>
Signed-off-by: crazycs <crazycs520@gmail.com>
What's the performance impact when we enable this by default? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: crazycs520 <crazycs520@gmail.com>
Sorry @wjhuang2016, you don't have permission to trigger auto merge event on this branch. You are not a committer for this part |
/run-all-tests |
1 similar comment
/run-all-tests |
/run-integration-ddl-test |
/run-cherry-picker |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
cherry pick to release-4.0 in PR #18072 |
Signed-off-by: crazycs crazycs520@gmail.com
What problem does this PR solve?
Close #15009
Record execute runtime information in slow log plan, such as below:
Plan
field value will like below, It will contain the plan runtime execute information.As you see, the execute information was contain in the slow-log
Plan
field now. Since the decode plan field was become too many, we had better add some header column name to more readable, such as below, but we can do this in next PR:What is changed and how it works?
Related changes
pingcap/docs
/pingcap/docs-cn
:Check List
Tests
Side effects
Release note