resource_control: add ru details in ExecDetails #1070
Merged
Conversation
This file contains 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
|
@nolouch @bufferflies PTAL |
13 tasks
|
/cc @JmPotato As we add ru metrics into ExecDetails, seems we can let |
bufferflies
reviewed
Dec 1, 2023
| @@ -115,13 +116,14 @@ func buildResourceControlInterceptor( | |||
| if reqInfo.Bypass() || resourceControlInterceptor.IsBackgroundRequest(ctx, resourceGroupName, req.RequestSource) { | |||
| return next(target, req) | |||
| } | |||
|
|
|||
| consumption, penalty, priority, err := resourceControlInterceptor.OnRequestWait(ctx, resourceGroupName, reqInfo) | |||
| consumption, penalty, waitDuration, priority, err := resourceControlInterceptor.OnRequestWait(ctx, resourceGroupName, reqInfo) | |||
There was a problem hiding this comment.
how about use one struct ?
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)) |
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 |
There was a problem hiding this comment.
where does it use in this pr?
There was a problem hiding this comment.
See: pingcap/tidb#49067
bufferflies
reviewed
Dec 1, 2023
| return | ||
| } | ||
| if stmtExec := ctx.Value(util.ExecDetailsKey); stmtExec != nil { | ||
| detail := stmtExec.(*util.ExecDetails) |
There was a problem hiding this comment.
Does it maybe nil in some scenario？
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) { |
There was a problem hiding this comment.
what‘s the different if remove this condition? I think zero doesn't need to treat alone.
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.
|
Rest LGTM, you need to sync the upstream. |
bufferflies
approved these changes
Dec 4, 2023
nolouch
reviewed
Dec 4, 2023
util/execdetails.go
Outdated
| @@ -329,6 +329,10 @@ type ExecDetails struct { | |||
| BackoffDuration int64 | |||
| WaitKVRespDuration int64 | |||
| WaitPDRespDuration int64 | |||
| WaitRUDuration int64 | |||
| // total read ans write ru in 1/1000 | |||
| MilliRRU int64 | |||
There was a problem hiding this comment.
Is Milli for precision? I think it correspond to milliseconds, but this is multiplied by 1000.
There was a problem hiding this comment.
Yes. naming similar to millisecond, not sure if it is the correct naming.
Signed-off-by: glorv <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> Signed-off-by: iosmanthus <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> * use latest pd-client of release-7.5 Signed-off-by: nolouch <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Signed-off-by: nolouch <[email protected]> Co-authored-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> Signed-off-by: iosmanthus <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> Signed-off-by: iosmanthus <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> Signed-off-by: iosmanthus <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> * use latest pd-client of release-7.5 Signed-off-by: nolouch <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Signed-off-by: nolouch <[email protected]> Co-authored-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> Signed-off-by: zzm <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> Signed-off-by: iosmanthus <[email protected]> Signed-off-by: zzm <[email protected]>
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 <[email protected]> * add ruWaitDuration to RUDetails and update pd-client Signed-off-by: glorv <[email protected]> --------- Signed-off-by: zzm <[email protected]> Signed-off-by: glorv <[email protected]> Co-authored-by: zzm <[email protected]> Signed-off-by: iosmanthus <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.