Resume the max retry check for stale read processing #903

cfzjywxk · 2023-07-24T08:42:42Z

Ref: #902

Allow only another extra leader retry when processing the DataIsNotReady error.

cfzjywxk · 2023-07-24T08:42:50Z

/cc @crazycs520

Signed-off-by: cfzjywxk <lsswxrxr@163.com>

crazycs520

LGTM

crazycs520 · 2023-07-25T08:42:00Z

internal/locate/region_request3_test.go

@@ -1018,3 +1019,58 @@ func (s *testRegionRequestToThreeStoresSuite) TestAccessFollowerAfter1TiKVDown()
 		s.Equal(0, bo.GetTotalBackoffTimes())
 	}
 }
+
+func (s *testRegionRequestToThreeStoresSuite) TestStaleReadFallback() {


This test was also passed before this PR.

It would fail after removing the !state.option.leaderOnly check which is the early version.

you06 · 2023-07-26T05:09:23Z

internal/locate/region_request3_test.go

+				DataIsNotReady: &errorpb.DataIsNotReady{},
+			}}}, nil
+		}
+		return &tikvrpc.Response{Resp: &kvrpcpb.GetResponse{Value: value}}, nil


It tests the max retry works, may this mock client looks like:

1st time returns data-is-not-ready error

2nd time returns other errors like unavailable store

3rd time fail the test because there should be leader retry limitation

Fail path test are added.

Signed-off-by: cfzjywxk <lsswxrxr@163.com>

) (#911) * Resume max retry time check for stale read retry with leader option Signed-off-by: cfzjywxk <lsswxrxr@163.com> * add cancel Signed-off-by: cfzjywxk <lsswxrxr@163.com> --------- Signed-off-by: cfzjywxk <lsswxrxr@163.com>

* client-go: add some key range info to error when PD returned no region (#862) Signed-off-by: Chao Wang <cclcwangchao@hotmail.com> * *: refine non-global stale-read request retry logic (#863) Signed-off-by: crazycs520 <crazycs520@gmail.com> * Fix the issue that primary pessimistic lock may be left not cleared after GC (#866) * Fix the issue that primary pessimistic lock may be left not cleared after GC Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> * Fix mysteriously shown up thing that makes compilation failed Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> * Fix test effectiveness (forgot to set txn2 to pessimistic txn); add more strict checks Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> * Address comments Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> --------- Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com> * add explicit request source type to label the external request like lightning/br (#868) Signed-off-by: nolouch <nolouch@gmail.com> * use '%d' instead of '%q' for some int values in error message (#875) Signed-off-by: Chao Wang <cclcwangchao@hotmail.com> * format key in error message in method `scanRegions` (#876) Signed-off-by: Chao Wang <cclcwangchao@hotmail.com> * make cop request timeout a config paramter (#865) * update Signed-off-by: Spade A <u6748471@anu.edu.au> * update Signed-off-by: Spade A <u6748471@anu.edu.au> * update Signed-off-by: Spade A <u6748471@anu.edu.au> * update Signed-off-by: Spade A <u6748471@anu.edu.au> --------- Signed-off-by: Spade A <u6748471@anu.edu.au> * region_cache: support check pending tiflash peer (#821) Signed-off-by: guo-shaoge <shaoge1994@163.com> Co-authored-by: disksing <i@disksing.com> * *: add `SnapshotIterReverse` and make `iterReverse` supports `lowerBound` (#883) Signed-off-by: Jason Mo <mohangjie1995@gmail.com> * *: fix stale read ops metric (#878) (#889) Signed-off-by: crazycs520 <crazycs520@gmail.com> Co-authored-by: disksing <i@disksing.com> * add gc options (#828) Signed-off-by: weedge <weege007@gmail.com> Co-authored-by: disksing <i@disksing.com> * reload region cache when store is resolved from invalid status (#843) Signed-off-by: you06 <you1474600@gmail.com> Co-authored-by: disksing <i@disksing.com> * ci: update setup-go action (#904) Signed-off-by: disksing <i@disksing.com> * fix unexpected slow query during GC running after stop 1 tikv-server (#899) (#909) * fix unexpected slow query during GC running after stop 1 tikv-server Signed-off-by: crazycs520 <crazycs520@gmail.com> * fix test Signed-off-by: crazycs520 <crazycs520@gmail.com> --------- Signed-off-by: crazycs520 <crazycs520@gmail.com> * resource_manager: ignore ru metrics for background request (#872) Signed-off-by: husharp <jinhao.hu@pingcap.com> Co-authored-by: disksing <i@disksing.com> * add more log for diagnose (#915) * add more log for diagnose Signed-off-by: crazycs520 <crazycs520@gmail.com> * fix Signed-off-by: crazycs520 <crazycs520@gmail.com> * add more log for diagnose Signed-off-by: crazycs520 <crazycs520@gmail.com> * add more log Signed-off-by: crazycs520 <crazycs520@gmail.com> * address comment Signed-off-by: crazycs520 <crazycs520@gmail.com> --------- Signed-off-by: crazycs520 <crazycs520@gmail.com> * use context logger as much as possible (#908) * use context logger as much as possible Signed-off-by: crazycs520 <crazycs520@gmail.com> * refine Signed-off-by: crazycs520 <crazycs520@gmail.com> --------- Signed-off-by: crazycs520 <crazycs520@gmail.com> * Resume max retry time check for stale read retry with leader option(#903) (#911) * Resume max retry time check for stale read retry with leader option Signed-off-by: cfzjywxk <lsswxrxr@163.com> * add cancel Signed-off-by: cfzjywxk <lsswxrxr@163.com> --------- Signed-off-by: cfzjywxk <lsswxrxr@163.com> * request_source: remove default label (#890) * request_source: remove default label Signed-off-by: nolouch <nolouch@gmail.com> * add a function to set request source task type (#925) * add a function to set request source task type Signed-off-by: glorv <glorvs@163.com> * ci: update go version (#936) * ci: update go version Signed-off-by: crazycs520 <crazycs520@gmail.com> * fix test Signed-off-by: crazycs520 <crazycs520@gmail.com> --------- Signed-off-by: crazycs520 <crazycs520@gmail.com> * use tidb_kv_read_timeout as first kv request timeout (#919) * support tidb_kv_read_timeout as first round kv request timeout Signed-off-by: crazycs520 <crazycs520@gmail.com> * fix ci Signed-off-by: crazycs520 <crazycs520@gmail.com> * fix ci Signed-off-by: crazycs520 <crazycs520@gmail.com> * fix ci Signed-off-by: crazycs520 <crazycs520@gmail.com> * fix ci Signed-off-by: crazycs520 <crazycs520@gmail.com> * fix ci Signed-off-by: crazycs520 <crazycs520@gmail.com> * update comment Signed-off-by: crazycs520 <crazycs520@gmail.com> * refine test Signed-off-by: crazycs520 <crazycs520@gmail.com> --------- Signed-off-by: crazycs520 <crazycs520@gmail.com> * [pick] resource_control: bypass some internal urgent request (#938) * resource_control: bypass some internal urgent request (#884) Signed-off-by: nolouch <nolouch@gmail.com> * resourcecontrol: fix nil pointer (#900) Signed-off-by: nolouch <nolouch@gmail.com> --------- Signed-off-by: nolouch <nolouch@gmail.com> --------- Signed-off-by: Chao Wang <cclcwangchao@hotmail.com> Signed-off-by: crazycs520 <crazycs520@gmail.com> Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Signed-off-by: nolouch <nolouch@gmail.com> Signed-off-by: Spade A <u6748471@anu.edu.au> Signed-off-by: guo-shaoge <shaoge1994@163.com> Signed-off-by: Jason Mo <mohangjie1995@gmail.com> Signed-off-by: weedge <weege007@gmail.com> Signed-off-by: you06 <you1474600@gmail.com> Signed-off-by: disksing <i@disksing.com> Signed-off-by: husharp <jinhao.hu@pingcap.com> Signed-off-by: cfzjywxk <lsswxrxr@163.com> Signed-off-by: glorv <glorvs@163.com> Signed-off-by: iosmanthus <myosmanthustree@gmail.com> Co-authored-by: 王超 <cclcwangchao@hotmail.com> Co-authored-by: crazycs <crazycs520@gmail.com> Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com> Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Co-authored-by: ShuNing <nolouch@gmail.com> Co-authored-by: Spade A <71589810+SpadeA-Tang@users.noreply.github.com> Co-authored-by: guo-shaoge <shaoge1994@163.com> Co-authored-by: disksing <i@disksing.com> Co-authored-by: Hangjie Mo <mohangjie1995@gmail.com> Co-authored-by: weedge <weege007@gmail.com> Co-authored-by: you06 <you1474600@gmail.com> Co-authored-by: Hu# <jinhao.hu@pingcap.com> Co-authored-by: cfzjywxk <lsswxrxr@163.com> Co-authored-by: glorv <glorvs@163.com>

…ikv#903) (tikv#911) * Resume max retry time check for stale read retry with leader option Signed-off-by: cfzjywxk <lsswxrxr@163.com> * add cancel Signed-off-by: cfzjywxk <lsswxrxr@163.com> --------- Signed-off-by: cfzjywxk <lsswxrxr@163.com>

* reload region cache when store is resolved from invalid status (#843) Signed-off-by: you06 <you1474600@gmail.com> Co-authored-by: disksing <i@disksing.com> * fallback to follower when leader is busy (#916) (#923) * fallback to follower when leader is busy Signed-off-by: you06 <you1474600@gmail.com> Co-authored-by: cfzjywxk <cfzjywxk@gmail.com> Co-authored-by: cfzjywxk <lsswxrxr@163.com> * Resume max retry time check for stale read retry with leader option(#903) (#911) * Resume max retry time check for stale read retry with leader option Signed-off-by: cfzjywxk <lsswxrxr@163.com> * add cancel Signed-off-by: cfzjywxk <lsswxrxr@163.com> --------- Signed-off-by: cfzjywxk <lsswxrxr@163.com> * add region cache state test & fix some issues of replica selector (#910) Signed-off-by: you06 <you1474600@gmail.com> remove duplicate code Signed-off-by: you06 <you1474600@gmail.com> * enable workflow for tidb-7.1 Signed-off-by: you06 <you1474600@gmail.com> * update Signed-off-by: you06 <you1474600@gmail.com> update Signed-off-by: you06 <you1474600@gmail.com> fix test Signed-off-by: you06 <you1474600@gmail.com> fix test Signed-off-by: you06 <you1474600@gmail.com> * lint Signed-off-by: you06 <you1474600@gmail.com> * lint Signed-off-by: you06 <you1474600@gmail.com> * fix flaky test Signed-off-by: you06 <you1474600@gmail.com> --------- Signed-off-by: you06 <you1474600@gmail.com> Signed-off-by: cfzjywxk <lsswxrxr@163.com> Co-authored-by: disksing <i@disksing.com> Co-authored-by: cfzjywxk <cfzjywxk@gmail.com> Co-authored-by: cfzjywxk <lsswxrxr@163.com>

cfzjywxk requested review from zyguan, you06 and ekexium July 24, 2023 08:42

cfzjywxk force-pushed the tidb-6.5-max-retry branch from daca134 to 392ff23 Compare July 24, 2023 09:27

Resume max retry time check for stale read retry with leader option

e601a9d

Signed-off-by: cfzjywxk <lsswxrxr@163.com>

cfzjywxk force-pushed the tidb-6.5-max-retry branch from 392ff23 to e601a9d Compare July 24, 2023 09:28

crazycs520 approved these changes Jul 25, 2023

View reviewed changes

crazycs520 reviewed Jul 25, 2023

View reviewed changes

cfzjywxk mentioned this pull request Jul 26, 2023

Resume max retry time check for stale read retry with leader option(#903) #911

Merged

crazycs520 approved these changes Jul 26, 2023

View reviewed changes

you06 reviewed Jul 26, 2023

View reviewed changes

add fail path test

ecb880e

Signed-off-by: cfzjywxk <lsswxrxr@163.com>

you06 approved these changes Jul 26, 2023

View reviewed changes

cfzjywxk merged commit bf9390f into tikv:tidb-6.5 Jul 26, 2023
8 of 9 checks passed

cfzjywxk mentioned this pull request Aug 7, 2023

storage: backport the stale read enhancement and bug fix to release 6.5 pingcap/tidb#43481

Closed

cfzjywxk mentioned this pull request Feb 6, 2024

The try times are not limited when stale read is used #902

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resume the max retry check for stale read processing #903

Resume the max retry check for stale read processing #903

cfzjywxk commented Jul 24, 2023

cfzjywxk commented Jul 24, 2023

crazycs520 left a comment

crazycs520 Jul 25, 2023

cfzjywxk Jul 25, 2023

crazycs520 Jul 26, 2023

you06 Jul 26, 2023

cfzjywxk Jul 26, 2023

Resume the max retry check for stale read processing #903

Resume the max retry check for stale read processing #903

Conversation

cfzjywxk commented Jul 24, 2023

cfzjywxk commented Jul 24, 2023

crazycs520 left a comment

Choose a reason for hiding this comment

crazycs520 Jul 25, 2023

Choose a reason for hiding this comment

cfzjywxk Jul 25, 2023

Choose a reason for hiding this comment

crazycs520 Jul 26, 2023

Choose a reason for hiding this comment

you06 Jul 26, 2023

Choose a reason for hiding this comment

cfzjywxk Jul 26, 2023

Choose a reason for hiding this comment