-
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
executor: show operators' memory consumption in results of EXPLAIN ANALYZE
#11334
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11334 +/- ##
================================================
+ Coverage 81.3024% 81.3303% +0.0279%
================================================
Files 423 423
Lines 90616 90591 -25
================================================
+ Hits 73673 73678 +5
+ Misses 11619 11596 -23
+ Partials 5324 5317 -7 |
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
executor/distsql.go
Outdated
@@ -415,14 +417,16 @@ func (e *IndexLookUpExecutor) startIndexWorker(ctx context.Context, kvRanges []k | |||
e.dagPB.CollectExecutionSummaries = &collExec | |||
} | |||
|
|||
tracker := memory.NewTracker(stringutil.StringerStr("InnerWorker"), e.ctx.GetSessionVars().MemQuotaIndexLookupReader) |
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.
s/ InnerWorker/ IndexWorker ?
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.
fixed.
executor/distsql.go
Outdated
@@ -486,7 +488,8 @@ func (e *IndexLookUpExecutor) startTableWorker(ctx context.Context, workCh <-cha | |||
keepOrder: e.keepOrder, | |||
handleIdx: e.handleIdx, | |||
isCheckOp: e.isCheckOp, | |||
memTracker: memory.NewTracker(tableWorkerLabel, -1), | |||
memTracker: memory.NewTracker(stringutil.StringerStr(fmt.Sprintf("worker_%v", i)), |
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.
s/ worker/ TableWorker ?
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.
fixed.
executor/distsql.go
Outdated
@@ -486,7 +488,8 @@ func (e *IndexLookUpExecutor) startTableWorker(ctx context.Context, workCh <-cha | |||
keepOrder: e.keepOrder, | |||
handleIdx: e.handleIdx, | |||
isCheckOp: e.isCheckOp, | |||
memTracker: memory.NewTracker(tableWorkerLabel, -1), | |||
memTracker: memory.NewTracker(stringutil.StringerStr(fmt.Sprintf("worker_%v", i)), |
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.
maybe better not alloc this for normal execution
stringutil.MemoizeStr(func() string { return "worker_" + strconv.Itoa(i) })
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.
updated.
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.
What about the other stringutil.StringerStr(xx)
?
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.
that seems cannot be avoided :D
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.
The other stringutil.StringerStr(xx)
has no sprintf
, so it's OK? 🤔
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
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 |
cherry pick to release-2.1 failed |
It seems that, not for sure, we failed to cherry-pick this commit to release-2.1. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @qw4990 PTAL. |
What problem does this PR solve?
Add a new column named
memory
in the results ofEXPLAIN ANALYZE
to display memory consumption of operators.After this PR, you can see:
Pipeliner operators like
Selection
orProjection
have no memory consumption.Cop-tasks' memory consumption will be added by another PR.
What is changed and how it works?
Main changes:
ResetContextOfStmt
when the next query comes;memory
in results ofEXPLAIN ANALYZE
;Check List
Tests