distsql,util: Add executor open close time into its total consume time #56236
Conversation
ti-chi-bot
bot
added
release-note-none
|
Hi @yibin87. 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. |
yibin87
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #56236 +/- ##
================================================
+ Coverage 72.8488% 73.5566% +0.7078%
================================================
Files 1672 1672
Lines 462666 463743 +1077
================================================
+ Hits 337047 341114 +4067
+ Misses 104821 101851 -2970
+ Partials 20798 20778 -20
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Signed-off-by: yibin <huyibin@pingcap.cn>
Signed-off-by: yibin <huyibin@pingcap.cn>
Signed-off-by: yibin <huyibin@pingcap.cn>
Signed-off-by: yibin <huyibin@pingcap.cn>
Signed-off-by: yibin <huyibin@pingcap.cn>
yibin87
force-pushed
the
add_executor_open_close_time
branch
from
ead70f8 to
0bf614c
Compare
Signed-off-by: yibin <huyibin@pingcap.cn>
Signed-off-by: yibin <huyibin@pingcap.cn>
|
/cc @windtalker |
|
/cc @guo-shaoge |
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
Signed-off-by: yibin <huyibin@pingcap.cn>
| @@ -430,6 +430,10 @@ func Open(ctx context.Context, e Executor) (err error) { | |||
| err = util.GetRecoverError(r) | |||
| } | |||
| }() | |||
| if e.RuntimeStats() != nil { | |||
There was a problem hiding this comment.
why not just move this into the defer function in L428
There was a problem hiding this comment.
If e.RuntimeStats() is nil, startTime won't be recorded. If move in L428, then there will be two "e.RuntimeStats() is not nil" checks, one in normal, one in defer. Not too much differences.
| @@ -469,5 +473,9 @@ func Close(e Executor) (err error) { | |||
| err = util.GetRecoverError(r) | |||
| } | |||
| }() | |||
| if e.RuntimeStats() != nil { | |||
Signed-off-by: yibin <huyibin@pingcap.cn>
|
/ok-to-test |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
/test pull-integration-ddl-test |
|
@yibin87: The specified target(s) for
Use
In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: guo-shaoge, windtalker, winoros The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: close #56233 ref #56232
Problem Summary:
Some executors' open or close operation may take too much time, thus we'd probably record these time in execution summary.
#55957 provides an example that table reader operator's open takes much time while not included.
#50377 provides an example that limit operator's close takes much time while not included.
What changed and how does it work?
TPCH Q3 TiFlash
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.