[DNM] support `GetRegion` via CSE `sync_region` API #745

iosmanthus · 2023-03-22T11:14:29Z

No description provided.

internal/locate/cse.go

Signed-off-by: iosmanthus <[email protected]>

internal/locate/cse/cse.go

Signed-off-by: iosmanthus <[email protected]>

…ient Signed-off-by: iosmanthus <[email protected]>

Signed-off-by: iosmanthus <[email protected]>

…ient Signed-off-by: iosmanthus <[email protected]>

Signed-off-by: iosmanthus <[email protected]>

zeminzhou · 2023-04-07T06:22:35Z

internal/locate/cse/breaker.go

+
+func ifMostFailures(counts gobreaker.Counts) bool {
+	failureRatio := float64(counts.TotalFailures) / float64(counts.Requests)
+	return counts.Requests >= 5 && failureRatio >= 0.4


5 and 0.4 should be a constant to improve readability. BTW, is it necessary to change them to configuration?

I could provide an option for Fallback and CSEClient, with a default ReadyToTrip.

zeminzhou · 2023-04-07T06:48:08Z

internal/locate/cse/fallback.go

+	_ pd.Client = &Fallback{}
+)
+
+type Fallback struct {


How about changing Fallback to ClientWithFallback?

Make sense.

zeminzhou · 2023-04-07T06:55:27Z

internal/locate/cse/fallback.go

+func probePD(name string, client pd.Client, timeout time.Duration) error {
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+	_, err := client.GetRegionByID(ctx, 1)


PD Client has a goutine to check leader status. Maybe we can use it, like this.

I try to probe PD with GetRegionByID because the health status of PD is hard to define since there will be more services in the PD microservice. We just probe the PD with the API we care about, a.k.a the region meta service.

zeminzhou · 2023-04-07T07:01:46Z

internal/locate/cse/cse.go

+
+var (
+	StoresRefreshInterval = time.Second * 5
+	SyncRegionTimeout     = time.Second * 2


how about time.Second * 10?

This duration might be too long for the newly created cluster.

zeminzhou · 2023-04-07T07:08:13Z

internal/locate/cse/cse.go

+	// Close the idle connections.
+	c.httpClient.CloseIdleConnections()
+	// Close the stores refresh goroutine.
+	c.done <- struct{}{}


how about close(c.done) ? That will not block the current goroutinue.

This will not block the current goroutine since the channel is init with: done: make(chan struct{}, 1)

zeminzhou · 2023-04-07T07:15:57Z

internal/locate/cse/cse.go

+				Name:          fmt.Sprintf("store-%d", s.GetId()),
+				Interval:      5 * time.Second,
+				Timeout:       1 * time.Second,
+				ProbeInterval: 1 * time.Second,


5, 1 and 1 should be a constant to improve readability

Signed-off-by: iosmanthus <[email protected]>

ystaticy · 2023-04-10T03:13:24Z

internal/locate/cse/fallback.go

+
+type ClientWithFallback struct {
+	pd.Client
+	cse     pd.Client


Why is it called cse？

Why is it called cse？

This field tends to store the CSEClient, which is an implementation of the interface PDClient.

zeminzhou

LGTM～

Co-authored-by: MyonKeminta <[email protected]> Co-authored-by: disksing <[email protected]> Co-authored-by: MyonKeminta <[email protected]> Co-authored-by: Violin <[email protected]> Co-authored-by: Smilencer <[email protected]> Co-authored-by: you06 <[email protected]> Co-authored-by: Hu# <[email protected]> Co-authored-by: Connor <[email protected]> Co-authored-by: zyguan <[email protected]> fix case typo in comment. (#778) fix goroutine leak (#784) fix TestRURuntimeStatsCleanUp (#787) Fix wrong resource group name for some requests (#788) resolver: support verifying primary for check_txn_status (#777) resolver: handle pessimistic locks in BatchResolveLocks (#794) resolved ts (#793) ResolveLocks for unistore (#807)

…ient Signed-off-by: iosmanthus <[email protected]>

…e-region-client Signed-off-by: iosmanthus <[email protected]>

* support remote coprocessor

Signed-off-by: nolouch <[email protected]>

Signed-off-by: iosmanthus <[email protected]>

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: disksing <[email protected]> Co-authored-by: David <[email protected]> Co-authored-by: Hu# <[email protected]> Co-authored-by: Weizhen Wang <[email protected]> Co-authored-by: you06 <[email protected]> Co-authored-by: ShuNing <[email protected]> Co-authored-by: Hu# <[email protected]> Co-authored-by: zzm <[email protected]> Co-authored-by: Yongbo Jiang <[email protected]> Co-authored-by: crazycs <[email protected]> Co-authored-by: glorv <[email protected]> Co-authored-by: zyguan <[email protected]> ResolvedTS error just write in debug log (#814) ResolvedTS error just write in debug log (#825) fix ci (#835) fix rpc interceptor data race (#845) resolver: let getTxnStatusFromLock return error when backoff timeout (#847)

…ient Signed-off-by: iosmanthus <[email protected]>

* client-go: add some key range info to error when PD returned no region (#862) Signed-off-by: Chao Wang <[email protected]> * *: refine non-global stale-read request retry logic (#863) Signed-off-by: crazycs520 <[email protected]> * 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 <[email protected]> * Fix mysteriously shown up thing that makes compilation failed Signed-off-by: MyonKeminta <[email protected]> * Fix test effectiveness (forgot to set txn2 to pessimistic txn); add more strict checks Signed-off-by: MyonKeminta <[email protected]> * Address comments Signed-off-by: MyonKeminta <[email protected]> --------- Signed-off-by: MyonKeminta <[email protected]> Co-authored-by: MyonKeminta <[email protected]> * add explicit request source type to label the external request like lightning/br (#868) Signed-off-by: nolouch <[email protected]> * use '%d' instead of '%q' for some int values in error message (#875) Signed-off-by: Chao Wang <[email protected]> * format key in error message in method `scanRegions` (#876) Signed-off-by: Chao Wang <[email protected]> * make cop request timeout a config paramter (#865) * update Signed-off-by: Spade A <[email protected]> * update Signed-off-by: Spade A <[email protected]> * update Signed-off-by: Spade A <[email protected]> * update Signed-off-by: Spade A <[email protected]> --------- Signed-off-by: Spade A <[email protected]> * region_cache: support check pending tiflash peer (#821) Signed-off-by: guo-shaoge <[email protected]> Co-authored-by: disksing <[email protected]> * *: add `SnapshotIterReverse` and make `iterReverse` supports `lowerBound` (#883) Signed-off-by: Jason Mo <[email protected]> * *: fix stale read ops metric (#878) (#889) Signed-off-by: crazycs520 <[email protected]> Co-authored-by: disksing <[email protected]> * add gc options (#828) Signed-off-by: weedge <[email protected]> Co-authored-by: disksing <[email protected]> * reload region cache when store is resolved from invalid status (#843) Signed-off-by: you06 <[email protected]> Co-authored-by: disksing <[email protected]> * ci: update setup-go action (#904) Signed-off-by: disksing <[email protected]> * 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 <[email protected]> * fix test Signed-off-by: crazycs520 <[email protected]> --------- Signed-off-by: crazycs520 <[email protected]> * resource_manager: ignore ru metrics for background request (#872) Signed-off-by: husharp <[email protected]> Co-authored-by: disksing <[email protected]> * add more log for diagnose (#915) * add more log for diagnose Signed-off-by: crazycs520 <[email protected]> * fix Signed-off-by: crazycs520 <[email protected]> * add more log for diagnose Signed-off-by: crazycs520 <[email protected]> * add more log Signed-off-by: crazycs520 <[email protected]> * address comment Signed-off-by: crazycs520 <[email protected]> --------- Signed-off-by: crazycs520 <[email protected]> * use context logger as much as possible (#908) * use context logger as much as possible Signed-off-by: crazycs520 <[email protected]> * refine Signed-off-by: crazycs520 <[email protected]> --------- Signed-off-by: crazycs520 <[email protected]> * 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 <[email protected]> * add cancel Signed-off-by: cfzjywxk <[email protected]> --------- Signed-off-by: cfzjywxk <[email protected]> * request_source: remove default label (#890) * request_source: remove default label Signed-off-by: nolouch <[email protected]> * add a function to set request source task type (#925) * add a function to set request source task type Signed-off-by: glorv <[email protected]> * ci: update go version (#936) * ci: update go version Signed-off-by: crazycs520 <[email protected]> * fix test Signed-off-by: crazycs520 <[email protected]> --------- Signed-off-by: crazycs520 <[email protected]> * 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 <[email protected]> * fix ci Signed-off-by: crazycs520 <[email protected]> * fix ci Signed-off-by: crazycs520 <[email protected]> * fix ci Signed-off-by: crazycs520 <[email protected]> * fix ci Signed-off-by: crazycs520 <[email protected]> * fix ci Signed-off-by: crazycs520 <[email protected]> * update comment Signed-off-by: crazycs520 <[email protected]> * refine test Signed-off-by: crazycs520 <[email protected]> --------- Signed-off-by: crazycs520 <[email protected]> * [pick] resource_control: bypass some internal urgent request (#938) * resource_control: bypass some internal urgent request (#884) Signed-off-by: nolouch <[email protected]> * resourcecontrol: fix nil pointer (#900) Signed-off-by: nolouch <[email protected]> --------- Signed-off-by: nolouch <[email protected]> --------- Signed-off-by: Chao Wang <[email protected]> Signed-off-by: crazycs520 <[email protected]> Signed-off-by: MyonKeminta <[email protected]> Signed-off-by: nolouch <[email protected]> Signed-off-by: Spade A <[email protected]> Signed-off-by: guo-shaoge <[email protected]> Signed-off-by: Jason Mo <[email protected]> Signed-off-by: weedge <[email protected]> Signed-off-by: you06 <[email protected]> Signed-off-by: disksing <[email protected]> Signed-off-by: husharp <[email protected]> Signed-off-by: cfzjywxk <[email protected]> Signed-off-by: glorv <[email protected]> Signed-off-by: iosmanthus <[email protected]> Co-authored-by: 王超 <[email protected]> Co-authored-by: crazycs <[email protected]> Co-authored-by: MyonKeminta <[email protected]> Co-authored-by: MyonKeminta <[email protected]> Co-authored-by: ShuNing <[email protected]> Co-authored-by: Spade A <[email protected]> Co-authored-by: guo-shaoge <[email protected]> Co-authored-by: disksing <[email protected]> Co-authored-by: Hangjie Mo <[email protected]> Co-authored-by: weedge <[email protected]> Co-authored-by: you06 <[email protected]> Co-authored-by: Hu# <[email protected]> Co-authored-by: cfzjywxk <[email protected]> Co-authored-by: glorv <[email protected]>

…ient

…1032) * ru detail Signed-off-by: zzm <[email protected]> * remove unused code Signed-off-by: zzm <[email protected]> * reduce waitgroup Signed-off-by: zzm <[email protected]> * fix ut Signed-off-by: zzm <[email protected]> * make lint Signed-off-by: zzm <[email protected]> * fix ci Signed-off-by: zzm <[email protected]> --------- Signed-off-by: zzm <[email protected]>

cloud-storage-engine has larger region size, we don't want to split regions into 32MB size on large transaction write. Signed-off-by: Evan Zhou <[email protected]>

Co-authored-by: cfzjywxk <[email protected]> Co-authored-by: cfzjywxk <[email protected]> Co-authored-by: disksing <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: zzm <[email protected]> Co-authored-by: husharp <[email protected]> Co-authored-by: you06 <[email protected]> Co-authored-by: buffer <[email protected]> Co-authored-by: 3pointer <[email protected]> Co-authored-by: buffer <[email protected]> Co-authored-by: husharp <[email protected]> Co-authored-by: crazycs520 <[email protected]> Co-authored-by: Smilencer <[email protected]> Co-authored-by: ShuNing <[email protected]> Co-authored-by: zyguan <[email protected]> Co-authored-by: Jack Yu <[email protected]> Co-authored-by: Weizhen Wang <[email protected]> Co-authored-by: lucasliang <[email protected]> Co-authored-by: healthwaite <[email protected]> Co-authored-by: xufei <[email protected]> Co-authored-by: JmPotato <[email protected]> Co-authored-by: ekexium <[email protected]> Co-authored-by: 山岚 <[email protected]> Co-authored-by: glorv <[email protected]> Co-authored-by: Yongbo Jiang <[email protected]> resolve locks interface for tidb gc_worker (#945) fix some issues of replica selector (#910) (#942) fix some issues of replica selector (#910) fix issue of configure kv timeout not work when disable batch client (#980) fix batch-client wait too long and add some metrics (#973) fix batch-client wait too long and add some metrics (#973)" (#984) fix data race at the aggressiveLockingDirty (#913) fix MinSafeTS might be set to MaxUint64 permanently (#994) fix: fix invalid nil pointer when trying to record Store.SlownessStat. (#1017) Fix batch client batchSendLoop panic (#1021) fix request source tag unset (#1025) Fix comment of `SuspendTime` (#1057)

…ient Signed-off-by: iosmanthus <[email protected]>

zeminzhou reviewed Mar 23, 2023

View reviewed changes

internal/locate/cse.go Outdated Show resolved Hide resolved

zeminzhou reviewed Mar 23, 2023

View reviewed changes

internal/locate/cse.go Outdated Show resolved Hide resolved

iosmanthus force-pushed the cse-region-client branch from 4c02bd6 to 89e30b3 Compare March 24, 2023 13:23

refactor fanout logic of cse region client

97cea40

Signed-off-by: iosmanthus <[email protected]>

iosmanthus force-pushed the cse-region-client branch from 89e30b3 to 97cea40 Compare March 28, 2023 05:49

add todos about region info resp from cse

4da4647

Signed-off-by: iosmanthus <[email protected]>

iosmanthus commented Mar 28, 2023

View reviewed changes

internal/locate/cse/cse.go Outdated Show resolved Hide resolved

iosmanthus added 11 commits March 28, 2023 18:27

support get store for CSEClient

7ab0132

Signed-off-by: iosmanthus <[email protected]>

update comment for cse test

de2f0f4

Signed-off-by: iosmanthus <[email protected]>

remove logs

5bd7f71

Signed-off-by: iosmanthus <[email protected]>

Merge branch 'master' of github.com:tikv/client-go into cse-region-cl…

4054b80

…ient Signed-off-by: iosmanthus <[email protected]>

add store prober

05b48d2

Signed-off-by: iosmanthus <[email protected]>

fix compile

e05be61

Signed-off-by: iosmanthus <[email protected]>

remove logs

a9ad5f5

Signed-off-by: iosmanthus <[email protected]>

add cb for Fallback and CSEClient

e3a2ba2

Signed-off-by: iosmanthus <[email protected]>

goimports -w ./

70cc79e

Signed-off-by: iosmanthus <[email protected]>

introduce asyncBreaker for Fallback and CSEClient

f8201f7

Signed-off-by: iosmanthus <[email protected]>

Merge branch 'master' of github.com:tikv/client-go into cse-region-cl…

e2fdd40

…ient Signed-off-by: iosmanthus <[email protected]>

iosmanthus changed the title ~~WIP: support GetRegion via CSE sync_region API~~ support GetRegion via CSE sync_region API Apr 6, 2023

iosmanthus added 5 commits April 6, 2023 16:53

bump kvproto

ccbfa16

Signed-off-by: iosmanthus <[email protected]>

goimports -w ./

bca479e

Signed-off-by: iosmanthus <[email protected]>

fix lint

f6b9f97

Signed-off-by: iosmanthus <[email protected]>

go mod tidy for integration tests

e8cf959

Signed-off-by: iosmanthus <[email protected]>

introduce EnableCSERegionClient for TiKVClient

8b68569

Signed-off-by: iosmanthus <[email protected]>

zeminzhou reviewed Apr 7, 2023

View reviewed changes

iosmanthus added 2 commits April 7, 2023 16:58

refine interface comments and options

770e723

Signed-off-by: iosmanthus <[email protected]>

bump kvproto to master

01caebd

Signed-off-by: iosmanthus <[email protected]>

disksing approved these changes Apr 7, 2023

View reviewed changes

ystaticy reviewed Apr 10, 2023

View reviewed changes

zeminzhou approved these changes Apr 10, 2023

View reviewed changes

rleungx and others added 30 commits May 17, 2023 12:07

*: Add GetMinTS (#801)

31152f5

Use a new way to create client for v2 (#806)

d5baf4c

Count only one tiflash replica in disaggregated mode. (#805)

a2f93a3

Merge branch 'master' of github.com:tikv/client-go into cse-region-cl…

35937f2

…ient Signed-off-by: iosmanthus <[email protected]>

Merge branch 'cse-region-client' of github.com:tikv/client-go into cs…

d8ce633

…e-region-client Signed-off-by: iosmanthus <[email protected]>

Add get min ts implementation (#809)

a6a7be2

*: fix the lint and add CI (#813)

1d62208

keyspace gc cse client (#803)

388baf3

support remote coprocessor (#808)

4499cc5

* support remote coprocessor

cse: add replica number (#824)

a07600f

Signed-off-by: nolouch <[email protected]>

add logs for cse region client (#838)

390b11b

filter out removed node state (#849)

d2d449f

Signed-off-by: iosmanthus <[email protected]>

polish comments for cse region client (#850)

a246fae

*: update pd client version (#840)

136f833

Signed-off-by: lhy1024 <[email protected]>

refactor cse region client as plugin (#853)

d5a7044

Merge branch 'master' of github.com:tikv/client-go into cse-region-cl…

f04046a

…ient Signed-off-by: iosmanthus <[email protected]>

resource_control: bypass some internal urgent request (#884)

19ef437

pdclient: get all keyspace (#892)

c22bb29

resourcecontrol: fix nil pointer (#900)

b27cb07

prefix safepoint kv with keyspace name (#928)

2ad441f

Merge branch 'master' of github.com:tikv/client-go into cse-region-cl…

e93b07c

…ient

increase large transaction preSplitSizeThreashold (#1059)

e1d4f84

cloud-storage-engine has larger region size, we don't want to split regions into 32MB size on large transaction write. Signed-off-by: Evan Zhou <[email protected]>

Merge branch 'master' of github.com:tikv/client-go into cse-region-cl…

4ffb53a

…ient Signed-off-by: iosmanthus <[email protected]>

Merge branch 'master' into cse-region-client

aaa66ef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DNM] support `GetRegion` via CSE `sync_region` API #745

[DNM] support `GetRegion` via CSE `sync_region` API #745

iosmanthus commented Mar 22, 2023

zeminzhou Apr 7, 2023

iosmanthus Apr 7, 2023

zeminzhou Apr 7, 2023

iosmanthus Apr 7, 2023

zeminzhou Apr 7, 2023

iosmanthus Apr 7, 2023

zeminzhou Apr 7, 2023

iosmanthus Apr 7, 2023

zeminzhou Apr 7, 2023

iosmanthus Apr 7, 2023

zeminzhou Apr 7, 2023

ystaticy Apr 10, 2023

iosmanthus Apr 10, 2023

zeminzhou left a comment

[DNM] support GetRegion via CSE sync_region API #745

Are you sure you want to change the base?

[DNM] support GetRegion via CSE sync_region API #745

Conversation

iosmanthus commented Mar 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeminzhou left a comment

Choose a reason for hiding this comment

[DNM] support `GetRegion` via CSE `sync_region` API #745

[DNM] support `GetRegion` via CSE `sync_region` API #745