add retry info to request source #953
Conversation
Signed-off-by: you06 <[email protected]>
|
@crazycs520 PTAL |
internal/locate/region_request.go
Outdated
| return nil, nil, retryTimes, err | ||
| } | ||
| rpcCtx.contextPatcher.applyTo(&req.Context) | ||
| patchRequestSource(req, requestSource, readType, retryTimes) |
There was a problem hiding this comment.
If finally "nil, nil" is returned, the next round RegionRequestSender.SendReqCtx retry, stale flag would not be considered again, in this case it seems the readType may not represent the original read type.
There was a problem hiding this comment.
I change the implemention and it now handles the retry from upper layer.
Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
| // The initial read type. | ||
| ReadType string | ||
| // InputRequestSource is the input source of the request, if it's not empty, the final RequestSource sent to store will be attached with the retry info. | ||
| InputRequestSource string |
There was a problem hiding this comment.
Would the InputRequestSource of coprocessor related requests be set accordingly in tidb?
There was a problem hiding this comment.
Coprocessor code also need to be updated.
There was a problem hiding this comment.
Ok, let's merge the client-go part first
Signed-off-by: you06 <[email protected]>
|
The retry flag should also be handled when key error is returned to the |
Signed-off-by: you06 <[email protected]>
Updated and added the test result in the end of description. |
|
@you06 |
Yes, if there is no "stale" word, it's not a stale read request. There are two figures in the description, the first one is tested with stale read and the second one is not. |
* add retry info to request source Signed-off-by: you06 <[email protected]> * handle upper layer retry Signed-off-by: you06 <[email protected]> * stabilize test Signed-off-by: you06 <[email protected]> * retry in 3 dimension Signed-off-by: you06 <[email protected]> * record and restore req.ReadType Signed-off-by: you06 <[email protected]> --------- Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
|
can we adjust to like : |
Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
* add retry info to request source (#953) Signed-off-by: you06 <[email protected]> * set the request source at the last section (#960) Signed-off-by: you06 <[email protected]> --------- Signed-off-by: you06 <[email protected]>
| @@ -1431,8 +1430,17 @@ func (s *RegionRequestSender) SendReqCtx( | |||
| } | |||
| } | |||
|
|
|||
| if e := tikvrpc.SetContext(req, rpcCtx.Meta, rpcCtx.Peer); e != nil { | |||
| return nil, nil, retryTimes, err | |||
There was a problem hiding this comment.
Should it return e rather than err?
As the image shows, this PR will change the request srouce when there is retry.
The first request's request source will be set to
{request source}-{retry}_{stale}_{replica type}.The retried request's request soruce will be set to:
{request source}-{replica type}for the first non-stale request.{request source}-stale_{replica type}for the first stale request.{request source}-retry_{first replica type}_{replica type}for the retried non-stale request, the first tried replica type will be recorded.{request source}-retry_stale_{first replica type}for the retried stale request, the first tried replica type will be recorded. Even its stale flag is removed, the retry info still contain it.e.g.
external_Select-stale_leaderstands for first request with stale read sent to leader.external_Select-stale_followerstands for first request with stale read sent to follower.external_Select-retry_stale_follower_leaderstands for retried stale request to leader read, and it's fallback from follower. In the figure, because leader's safe ts is fresh enough to server the stale read, so the retried requests are fallback from followe.When meeting key error, it the retry information is also recorded, the get and batch get test with lock error injection: