-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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: calibrate resource support tpch10 #47095
Conversation
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #47095 +/- ##
================================================
- Coverage 73.0374% 72.6152% -0.4223%
================================================
Files 1338 1361 +23
Lines 399306 408150 +8844
================================================
+ Hits 291643 296379 +4736
- Misses 88826 93042 +4216
+ Partials 18837 18729 -108
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
Signed-off-by: guo-shaoge <shaoge1994@163.com>
// read bytes: 401799161689.0 / 20 / 20 = 1004497904.22 | ||
const cpuTimePerCPUPerSec float64 = 263.74 | ||
const readBytesPerCPUPerSec float64 = 1004497904.22 | ||
ruPerCPU := float64(ruCfg.CPUMsCost)*cpuTimePerCPUPerSec + float64(ruCfg.ReadBytesCost)*readBytesPerCPUPerSec |
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.
An additional question: Is the cost factor used by Tiflash to calculate RU obtained from PD?
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.
No, the number is by checking tiflash log(105494.666484 and 401799161689.0)
Signed-off-by: guo-shaoge <shaoge1994@163.com>
/test unit-test |
@CabinfeverB: 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/test-infra repository. |
Signed-off-by: guo-shaoge <shaoge1994@163.com>
/run-unit-test |
[LGTM Timeline notifier]Timeline:
|
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.
rest LGTM!
- Do we need to update dashboard and doc-cn?
https://github.com/pingcap/tidb-dashboard/blob/53563e713a2acce53a480e959f5d5c221caac993/ui/packages/tidb-dashboard-lib/src/apps/ResourceManager/translations/zh.yaml#L27C1-L30 - please paste a SQL statement in the pr description :)
return nil | ||
} | ||
|
||
func (e *Executor) getTiDBQuota(ctx context.Context, exec sqlexec.RestrictedSQLExecutor, startTs, endTs time.Time) (float64, error) { |
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.
Seems like it includes TiKV Quota, does it make sense to be called getTiDBQuota
?
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 getTiDBAndTiKVQuota
?
quota := sum / float64(upperBound-lowerBound) | ||
req.AppendUint64(0, uint64(quota)) | ||
return nil | ||
return sum / float64(upperBound-lowerBound), nil |
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.
Could upperBound-lowerBound
be zero?
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.
It cannot be zero.
because len(quotas) >= 2
. So len(quotas)
must be greater than round(0.1*len(quotas))
/approve There is no sysvar change. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CabinfeverB, easonn7, nolouch, wshwsh12, XuHuaiyu 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 |
/retest |
1 similar comment
/retest |
/hold |
/retest |
3 similar comments
/retest |
/retest |
/retest |
/rebuild |
/retest |
What problem does this PR solve?
Issue Number: close #47094
Problem Summary: part of tiflash resource control
What is changed and how it works?
RU_PER_SEC = RU/(cpu_usage/total_logical_core)
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.