executor: add more explain analyze info for hash join spill #59255
Conversation
ti-chi-bot
bot
added
do-not-merge/needs-linked-issue
release-note-none
|
Hi @xzhangxian1008. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. 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 kubernetes-sigs/prow repository. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #59255 +/- ##
================================================
+ Coverage 73.0409% 73.4542% +0.4133%
================================================
Files 1689 1689
Lines 467012 467435 +423
================================================
+ Hits 341110 343351 +2241
+ Misses 104920 103106 -1814
+ Partials 20982 20978 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/cc @windtalker @yibin87 |
|
please follow the common key:value pattern, and use UnderScoreCase naming method for key names, most execution info also follows json format. |
pkg/executor/join/hash_join_v2.go
Outdated
| } | ||
|
|
||
| round := e.spillHelper.round | ||
| for len(e.stats.spill.totalSpillBytesPerRound) < round+1 { |
There was a problem hiding this comment.
It looks a little strange for using "for loop" here, since in the following code, only the last round info are updated.
There was a problem hiding this comment.
It looks a little strange for using "for loop" here, since in the following code, only the last round info are updated.
okk, I will replace it with if
| @@ -202,6 +225,17 @@ func (e *hashJoinRuntimeStatsV2) String() string { | |||
| } | |||
| buf.WriteString("}") | |||
| } | |||
| if e.spill.round > 0 { | |||
There was a problem hiding this comment.
Is it possible that e.spill.round is very large? If so, I don't think all the data should be printed. Instead, some aggregated info may be more readable.
There was a problem hiding this comment.
Is it possible that e.spill.round is very large? If so, I don't think all the data should be printed. Instead, some aggregated info may be more readable.
It will not be very large.
pkg/executor/join/hash_join_stats.go
Outdated
| "github.com/pingcap/tidb/pkg/util/execdetails" | ||
| ) | ||
|
|
||
| func convertBytesStatsToString(bytes []int64) string { | ||
| info := "[" |
There was a problem hiding this comment.
Better use bytes.NewBuffer instead of raw strings here.
There was a problem hiding this comment.
Better use bytes.NewBuffer instead of raw strings here.
done
done |
pkg/executor/join/hash_join_stats.go
Outdated
| "github.com/pingcap/tidb/pkg/util/execdetails" | ||
| ) | ||
|
|
||
| func writeBytesStatsToString(buf *bytes.Buffer, convertedBytes []int64) string { |
There was a problem hiding this comment.
Seems no need to return string here.
There was a problem hiding this comment.
Seems no need to return string here.
deleted
| } | ||
|
|
||
| round := e.spillHelper.round | ||
| if len(e.stats.spill.totalSpillBytesPerRound) < round+1 { |
There was a problem hiding this comment.
Will it be possible that length + N < round here? If so, for loop makes sense
There was a problem hiding this comment.
Will it be possible that length + N < round here? If so, for loop makes sense
It's impossible.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yibin87 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
|
@xzhangxian1008: The following tests failed, say
Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: close #59264
Problem Summary:
What changed and how does it work?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.