resource_control: add ru details in ExecDetails#1070
Merged
bufferflies merged 2 commits intotikv:masterfrom Dec 4, 2023
Merged
resource_control: add ru details in ExecDetails#1070bufferflies merged 2 commits intotikv:masterfrom
bufferflies merged 2 commits intotikv:masterfrom
Conversation
Contributor
Author
|
@nolouch @bufferflies PTAL |
13 tasks
Contributor
Author
|
/cc @JmPotato As we add ru metrics into ExecDetails, seems we can let |
bufferflies
reviewed
Dec 1, 2023
| } | ||
|
|
||
| consumption, penalty, priority, err := resourceControlInterceptor.OnRequestWait(ctx, resourceGroupName, reqInfo) | ||
| consumption, penalty, waitDuration, priority, err := resourceControlInterceptor.OnRequestWait(ctx, resourceGroupName, reqInfo) |
Contributor
There was a problem hiding this comment.
how about use one struct ?
Contributor
Author
There was a problem hiding this comment.
Not sure if there is a best practice. The different between tuple and struct is not large, I see many other functions return multiple parameters.
bufferflies
reviewed
Dec 1, 2023
| if stmtExec := ctx.Value(util.ExecDetailsKey); stmtExec != nil { | ||
| detail := stmtExec.(*util.ExecDetails) | ||
| atomic.AddInt64(&detail.MilliRRU, int64(consumption.RRU*1000.0)) | ||
| atomic.AddInt64(&detail.MilliRRU, int64(consumption.RRU*1000.0)) |
Contributor
There was a problem hiding this comment.
why it needs to retry again?
bufferflies
reviewed
Dec 1, 2023
util/execdetails.go
Outdated
| WaitRUDuration int64 | ||
| // total read ans write ru in 1/1000 | ||
| MilliRRU int64 | ||
| MilliWRU int64 |
Contributor
There was a problem hiding this comment.
where does it use in this pr?
bufferflies
reviewed
Dec 1, 2023
| return | ||
| } | ||
| if stmtExec := ctx.Value(util.ExecDetailsKey); stmtExec != nil { | ||
| detail := stmtExec.(*util.ExecDetails) |
Contributor
There was a problem hiding this comment.
Does it maybe nil in some scenario？
Contributor
Author
There was a problem hiding this comment.
No. If the value exists, it should not be nil, otherwise the context value should not exist.
bufferflies
reviewed
Dec 1, 2023
| detail := stmtExec.(*util.ExecDetails) | ||
| atomic.AddInt64(&detail.MilliRRU, int64(consumption.RRU*1000.0)) | ||
| atomic.AddInt64(&detail.MilliWRU, int64(consumption.WRU*1000.0)) | ||
| if waitDuration > time.Duration(0) { |
Contributor
There was a problem hiding this comment.
what‘s the different if remove this condition? I think zero doesn't need to treat alone.
Contributor
Author
There was a problem hiding this comment.
Because the Duration is always 0 in OnResponse call, it's a very frequent value, so not need to do the atomic operation in this case. Though, the performance difference should be quite low.
Contributor
|
Rest LGTM, you need to sync the upstream. |
bufferflies
approved these changes
Dec 4, 2023
nolouch
reviewed
Dec 4, 2023
util/execdetails.go
Outdated
| WaitPDRespDuration int64 | ||
| WaitRUDuration int64 | ||
| // total read ans write ru in 1/1000 | ||
| MilliRRU int64 |
Contributor
There was a problem hiding this comment.
Is Milli for precision? I think it correspond to milliseconds, but this is multiplied by 1000.
Contributor
Author
There was a problem hiding this comment.
Yes. naming similar to millisecond, not sure if it is the correct naming.
…ikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com>
Signed-off-by: glorv <glorvs@163.com>
iosmanthus
pushed a commit
to iosmanthus/client-go
that referenced
this pull request
Dec 21, 2023
* collecting the RU information by pasing point through context.Value (tikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
This was referenced Feb 1, 2024
Merged
Closed
nolouch
pushed a commit
to nolouch/client-go
that referenced
this pull request
Feb 2, 2024
* collecting the RU information by pasing point through context.Value (tikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com>
glorv
added a commit
that referenced
this pull request
Feb 18, 2024
…#1151) * resource_control: add ru details in ExecDetails (#1070) * collecting the RU information by pasing point through context.Value (#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> * use latest pd-client of release-7.5 Signed-off-by: nolouch <nolouch@gmail.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Signed-off-by: nolouch <nolouch@gmail.com> Co-authored-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com>
zeminzhou
added a commit
to zeminzhou/client-go
that referenced
this pull request
Feb 28, 2024
* collecting the RU information by pasing point through context.Value (tikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
zeminzhou
added a commit
to zeminzhou/client-go
that referenced
this pull request
Feb 28, 2024
* collecting the RU information by pasing point through context.Value (tikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
zeminzhou
added a commit
to zeminzhou/client-go
that referenced
this pull request
Feb 28, 2024
* collecting the RU information by pasing point through context.Value (tikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
zeminzhou
added a commit
to zeminzhou/client-go
that referenced
this pull request
Feb 28, 2024
) (tikv#1151) * resource_control: add ru details in ExecDetails (tikv#1070) * collecting the RU information by pasing point through context.Value (tikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> * use latest pd-client of release-7.5 Signed-off-by: nolouch <nolouch@gmail.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Signed-off-by: nolouch <nolouch@gmail.com> Co-authored-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> Signed-off-by: zzm <zhouzemin@pingcap.com>
zeminzhou
added a commit
to zeminzhou/client-go
that referenced
this pull request
Feb 28, 2024
* collecting the RU information by pasing point through context.Value (tikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> Signed-off-by: iosmanthus <myosmanthustree@gmail.com> Signed-off-by: zzm <zhouzemin@pingcap.com>
zeminzhou
added a commit
to zeminzhou/client-go
that referenced
this pull request
Mar 13, 2024
* collecting the RU information by pasing point through context.Value (tikv#1032) Signed-off-by: zzm <zhouzemin@pingcap.com> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <glorvs@163.com> --------- Signed-off-by: zzm <zhouzemin@pingcap.com> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: zzm <zhouzemin@pingcap.com> Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cherry-pick #1032 to master and add
RUWaitDurationto RUDetails. RUDetails is used in recording executing log and slow log in tidb.