*: configurable kv_read_timeout should not affect write request (#978) (

#983) * *: configurable kv_read_timeout should not affect write request (#978) --------- Signed-off-by: crazycs520 <[email protected]> * add test Signed-off-by: crazycs520 <[email protected]> --------- Signed-off-by: crazycs520 <[email protected]>
tikv · Sep 18, 2023 · 32c4ef5 · 32c4ef5
1 parent 9126d07
commit 32c4ef5
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 13 deletions.
diff --git a/internal/locate/region_request.go b/internal/locate/region_request.go
@@ -897,13 +897,25 @@ func (s *replicaSelector) onSendFailure(bo *retry.Backoffer, err error) {
 	s.state.onSendFailure(bo, s, err)
 }
 
-func (s *replicaSelector) onDeadlineExceeded() {
-	if target := s.targetReplica(); target != nil {
-		target.deadlineErrUsingConfTimeout = true
+func (s *replicaSelector) onReadReqConfigurableTimeout(req *tikvrpc.Request) bool {
+	if req.MaxExecutionDurationMs >= uint64(client.ReadTimeoutShort.Milliseconds()) {
+		// Configurable timeout should less than `ReadTimeoutShort`.
+		return false
 	}
-	if accessLeader, ok := s.state.(*accessKnownLeader); ok {
-		// If leader return deadline exceeded error, we should try to access follower next time.
-		s.state = &tryFollower{leaderIdx: accessLeader.leaderIdx, lastIdx: accessLeader.leaderIdx}
+	switch req.Type {
+	case tikvrpc.CmdGet, tikvrpc.CmdBatchGet, tikvrpc.CmdScan,
+		tikvrpc.CmdCop, tikvrpc.CmdBatchCop, tikvrpc.CmdCopStream:
+		if target := s.targetReplica(); target != nil {
+			target.deadlineErrUsingConfTimeout = true
+		}
+		if accessLeader, ok := s.state.(*accessKnownLeader); ok {
+			// If leader return deadline exceeded error, we should try to access follower next time.
+			s.state = &tryFollower{leaderIdx: accessLeader.leaderIdx, lastIdx: accessLeader.leaderIdx}
+		}
+		return true
+	default:
+		// Only work for read requests, return false for non-read requests.
+		return false
 	}
 }
 
@@ -1553,9 +1565,8 @@ func (s *RegionRequestSender) onSendFail(bo *retry.Backoffer, ctx *RPCContext, r
 		return errors.WithStack(err)
 	} else if LoadShuttingDown() > 0 {
 		return errors.WithStack(tikverr.ErrTiDBShuttingDown)
-	} else if isCauseByDeadlineExceeded(err) && req.MaxExecutionDurationMs < uint64(client.ReadTimeoutShort.Milliseconds()) {
-		if s.replicaSelector != nil {
-			s.replicaSelector.onDeadlineExceeded()
+	} else if isCauseByDeadlineExceeded(err) {
+		if s.replicaSelector != nil && s.replicaSelector.onReadReqConfigurableTimeout(req) {
 			return nil
 		}
 	}
@@ -1776,8 +1787,9 @@ func (s *RegionRequestSender) onRegionError(bo *retry.Backoffer, ctx *RPCContext
 
 	if serverIsBusy := regionErr.GetServerIsBusy(); serverIsBusy != nil {
 		if s.replicaSelector != nil && strings.Contains(serverIsBusy.GetReason(), "deadline is exceeded") {
-			s.replicaSelector.onDeadlineExceeded()
-			return true, nil
+			if s.replicaSelector.onReadReqConfigurableTimeout(req) {
+				return true, nil
+			}
 		}
 		logutil.Logger(bo.GetCtx()).Warn(
 			"tikv reports `ServerIsBusy` retry later",
@@ -1900,8 +1912,8 @@ func (s *RegionRequestSender) onRegionError(bo *retry.Backoffer, ctx *RPCContext
 		return true, nil
 	}
 
-	if isDeadlineExceeded(regionErr) && s.replicaSelector != nil {
-		s.replicaSelector.onDeadlineExceeded()
+	if isDeadlineExceeded(regionErr) && s.replicaSelector != nil && s.replicaSelector.onReadReqConfigurableTimeout(req) {
+		return true, nil
 	}
 
 	logutil.Logger(bo.GetCtx()).Debug(

diff --git a/internal/locate/region_request3_test.go b/internal/locate/region_request3_test.go
@@ -1308,6 +1308,22 @@ func (s *testRegionRequestToThreeStoresSuite) TestSendReqFirstTimeout() {
 			s.Equal(0, bo.GetTotalBackoffTimes())                                // no backoff since fast retry.
 		}
 	}
+
+	// Test for write request.
+	tf := func(s *Store, bo *retry.Backoffer) livenessState {
+		return reachable
+	}
+	s.regionRequestSender.regionCache.testingKnobs.mockRequestLiveness.Store((*livenessFunc)(&tf))
+	resetStats()
+	req := tikvrpc.NewRequest(tikvrpc.CmdPrewrite, &kvrpcpb.PrewriteRequest{}, kvrpcpb.Context{})
+	req.ReplicaReadType = kv.ReplicaReadLeader
+	loc := getLocFn()
+	bo = retry.NewBackoffer(context.Background(), 1000)
+	resp, _, err := s.regionRequestSender.SendReqCtx(bo, req, loc.Region, time.Millisecond, tikvrpc.TiKV)
+	s.Nil(resp)
+	s.Equal(context.DeadlineExceeded, err)
+	backoffTimes := bo.GetBackoffTimes()
+	s.True(backoffTimes["tikvRPC"] > 0) // write request timeout won't do fast retry, so backoff times should be more than 0.
 }
 
 func (s *testRegionRequestToThreeStoresSuite) TestStaleReadTryFollowerAfterTimeout() {