kv/client: add incremental scan region count limit #1899

amyangfei · 2021-05-31T14:48:45Z

What problem does this PR solve?

This PR adds a token limiter to kv client, in order to control the concurrency of incremental scan region, for some reasons:

A large scale incremental scan tasks could cause pressure in TiKV server, which could lead to OOM in TiKV.
TiKV could also add concurrency limit of incremental scan regions and prevents too much memory consumption, however it will cause too many uninitialized pending regions in TiCDC kv clients. Region reconnect could happen before the TiKV can serve them.

What is changed and how it works?

Add a token based region router.
- When the used token doesn't reach upper limit, redirect the region request as soon as possible.
- Otherwise buffer the region request, and check 100ms to see whether there exists new token opportunity.
- The token is grouped by store address
Separate some logic from dispatchRequest into requestRegionToStore
- dispatchRequest only reads singleRegionInfo from region channel, try to get gRPC context and sends the singleRegionInfo(which has been filled in with gRPC context) into region router
- requestRegionToStore reads singleRegionInfo from the output channel of region based region router, and sends real gRPC request to TiKV store.

Check List

Tests

Unit test
Integration test

Related changes

Need to cherry-pick to the release branch
Need to update key monitor metrics in both TiCDC document and official document

Release note

Add concurrency limit to the region incremental scan in kv client.

amyangfei · 2021-05-31T14:49:32Z

/run-all-tests

amyangfei · 2021-05-31T15:33:16Z

/run-all-tests

amyangfei · 2021-05-31T16:03:25Z

/run-integration-tests

codecov-commenter · 2021-05-31T16:30:06Z

Codecov Report

Merging #1899 (2f32548) into master (99ee8fb) will increase coverage by 0.8089%.
The diff coverage is 73.0769%.

@@               Coverage Diff                @@
##             master      #1899        +/-   ##
================================================
+ Coverage   53.4083%   54.2173%   +0.8089%     
================================================
  Files           154        161         +7     
  Lines         16166      16764       +598     
================================================
+ Hits           8634       9089       +455     
- Misses         6608       6704        +96     
- Partials        924        971        +47

ben1009 · 2021-06-01T03:26:26Z

cdc/kv/metrics.go

@@ -67,6 +67,13 @@ var (
 			Name:      "channel_size",
 			Help:      "size of each channel in kv client",
 		}, []string{"id", "channel"})
+	clientRegionTokenSize = prometheus.NewGaugeVec(
+		prometheus.GaugeOpts{
+			Namespace: "ticdc",


maybe extract the ticdc kvclient in

Namespace: "ticdc", Subsystem: "kvclient",

to const ?

Yep they are constant values, but I will not change them to constants in this PR, since too many literal constants are used in metric names, maybe in another PR we can address it

ben1009 · 2021-06-01T03:32:49Z

cdc/kv/token_region.go

+)
+
+const (
+	regionOutputChanSize = 16


could u add more comment explain why 16, thx

ben1009 · 2021-06-01T03:51:22Z

cdc/kv/token_region.go

+}
+
+func (r *sizedRegionRouter) Run(ctx context.Context) error {
+	ticker := time.NewTicker(time.Millisecond * 100)


maybe defined as const also?

overvenus · 2021-06-01T05:56:56Z

cdc/kv/client.go

 	// The channel to put the region that will be sent requests.
-	regionCh chan singleRegionInfo
+	regionRouter LimitRegionRouter


I think it's not a channel anymore, could you update the comment?

overvenus · 2021-06-01T05:59:38Z

cdc/kv/client.go

@@ -70,6 +70,9 @@ const (
 	// failed region will be reloaded via `BatchLoadRegionsWithKeyRange` API. So we
 	// don't need to force reload region any more.
 	regionScheduleReload = false
+
+	// defines the scan region limit for each table
+	regionScanLimitPerTable = 40


Is it possible to limit scan region for each store?

cdc/kv/token_region.go

liuzix · 2021-06-02T09:08:24Z

Can you comment on how your design is similar or different form a "cancellable semaphore"?

amyangfei · 2021-06-02T09:50:21Z

Can you comment on how your design is similar or different form a "cancellable semaphore"?

The semaphore counter will be used for fast check and metric counter

the fast check means we will check whether we have token available, which can be achieved by semaphore.TryAcquire(n) and semaphore.Release(n), but it is simpler with counter
golang native semaphore doesn't expose available counter

@liuzix

amyangfei · 2021-06-02T11:55:45Z

/run-all-tests /tidb=release-5.0 /tikv=release-5.0 /pd=release-5.0 /tiflash=release-5.0

amyangfei · 2021-06-02T14:07:12Z

/run-leak-tests

amyangfei · 2021-06-03T16:31:07Z

/merge

ti-chi-bot · 2021-06-03T16:31:11Z

This pull request has been accepted and is ready to merge.

Commit hash: 9a07fd2

amyangfei · 2021-06-04T01:18:03Z

/merge

ti-chi-bot · 2021-06-04T01:18:06Z

This pull request has been accepted and is ready to merge.

Commit hash: c01b492

amyangfei · 2021-06-04T01:38:21Z

/run-unit-tests

amyangfei · 2021-06-04T01:38:53Z

/run-kafka-tests

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2021-06-04T02:09:05Z

In response to a cherrypick label: new pull request created: #1930.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2021-06-04T02:09:42Z

In response to a cherrypick label: new pull request created: #1931.

Note pingcap#1926 has picked part of pingcap#1899, this PR picks the remaining change

Note #1926 has picked part of #1899, this PR picks the remaining change

amyangfei added component/kv-client TiKV kv log client component. needs-cherry-pick-release-4.0 Should cherry pick this PR to release-4.0 branch. needs-cherry-pick-release-5.0 Should cherry pick this PR to release-5.0 branch. labels May 31, 2021

amyangfei added this to the v5.0.3 milestone May 31, 2021

ti-chi-bot requested review from zier-one and overvenus May 31, 2021 14:48

ti-chi-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 31, 2021

amyangfei added the status/ptal Could you please take a look? label Jun 1, 2021

ben1009 reviewed Jun 1, 2021

View reviewed changes

overvenus reviewed Jun 1, 2021

View reviewed changes

cdc/kv/token_region.go Show resolved Hide resolved

amyangfei added 4 commits June 2, 2021 15:10

kv/client: add incremental scan region count limit

86c88b4

fix deadlock

187a39f

address comments

2c1fc81

change use token time

97ab2f4

amyangfei force-pushed the kv-client-region-scan-limit branch from f3f42d4 to 97ab2f4 Compare June 2, 2021 07:10

add store based token limit

7b9abac

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 2, 2021

amyangfei added 2 commits June 2, 2021 18:46

support store as id in token region limitter

4b15edc

fix unit test

8967443

fix make check

0e40541

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jun 3, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 3, 2021

ti-chi-bot and others added 2 commits June 4, 2021 00:31

Merge branch 'master' into kv-client-region-scan-limit

015f1e1

fix test case

c01b492

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jun 4, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 4, 2021

Merge branch 'master' into kv-client-region-scan-limit

8e22891

ti-chi-bot merged commit f7ab5ba into pingcap:master Jun 4, 2021

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Jun 4, 2021

This is an automated cherry-pick of pingcap#1899

747bd1d

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jun 4, 2021

kv/client: add incremental scan region count limit (#1899) #1930

Closed

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Jun 4, 2021

This is an automated cherry-pick of pingcap#1899

41001a7

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jun 4, 2021

kv/client: add incremental scan region count limit (#1899) #1931

Closed

amyangfei deleted the kv-client-region-scan-limit branch June 4, 2021 02:10

amyangfei mentioned this pull request Jun 8, 2021

kv/client: add incremental scan region count limit #1981

Merged

amyangfei mentioned this pull request Jun 21, 2021

kv/client: fix token counter in region scan limiter (#1979) #2037

Merged

amyangfei added a commit to amyangfei/tiflow that referenced this pull request Jun 24, 2021

kv/client: add incremental scan region count limit (pingcap#1899)

b712e95

Note pingcap#1926 has picked part of pingcap#1899, this PR picks the remaining change

amyangfei mentioned this pull request Jun 24, 2021

kv/client: cherry pick kv client v2 into release-4.0 #2149

Merged

amyangfei added a commit to amyangfei/tiflow that referenced this pull request Jun 24, 2021

kv/client: add incremental scan region count limit (pingcap#1899)

13a817d

Note pingcap#1926 has picked part of pingcap#1899, this PR picks the remaining change

amyangfei added a commit to amyangfei/tiflow that referenced this pull request Jul 7, 2021

kv/client: add incremental scan region count limit (pingcap#1899)

66921e8

Note pingcap#1926 has picked part of pingcap#1899, this PR picks the remaining change

amyangfei added a commit to amyangfei/tiflow that referenced this pull request Jul 7, 2021

kv/client: add incremental scan region count limit (pingcap#1899)

8a05bfe

Note pingcap#1926 has picked part of pingcap#1899, this PR picks the remaining change

ti-chi-bot pushed a commit that referenced this pull request Jul 8, 2021

kv/client: add incremental scan region count limit (#1899)

c37854d

Note #1926 has picked part of #1899, this PR picks the remaining change

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kv/client: add incremental scan region count limit #1899

kv/client: add incremental scan region count limit #1899

amyangfei commented May 31, 2021 •

edited

Loading

amyangfei commented May 31, 2021

amyangfei commented May 31, 2021

amyangfei commented May 31, 2021

codecov-commenter commented May 31, 2021

ben1009 Jun 1, 2021

amyangfei Jun 1, 2021

ben1009 Jun 1, 2021

amyangfei Jun 1, 2021

ben1009 Jun 1, 2021

amyangfei Jun 1, 2021

overvenus Jun 1, 2021

amyangfei Jun 1, 2021

overvenus Jun 1, 2021

liuzix commented Jun 2, 2021

amyangfei commented Jun 2, 2021

amyangfei commented Jun 2, 2021

amyangfei commented Jun 2, 2021

amyangfei commented Jun 3, 2021

ti-chi-bot commented Jun 3, 2021

amyangfei commented Jun 4, 2021

ti-chi-bot commented Jun 4, 2021

amyangfei commented Jun 4, 2021

amyangfei commented Jun 4, 2021

ti-chi-bot commented Jun 4, 2021

ti-chi-bot commented Jun 4, 2021

kv/client: add incremental scan region count limit #1899

kv/client: add incremental scan region count limit #1899

Conversation

amyangfei commented May 31, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

amyangfei commented May 31, 2021

amyangfei commented May 31, 2021

amyangfei commented May 31, 2021

codecov-commenter commented May 31, 2021

Codecov Report

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

liuzix commented Jun 2, 2021

amyangfei commented Jun 2, 2021

amyangfei commented Jun 2, 2021

amyangfei commented Jun 2, 2021

amyangfei commented Jun 3, 2021

ti-chi-bot commented Jun 3, 2021

amyangfei commented Jun 4, 2021

ti-chi-bot commented Jun 4, 2021

amyangfei commented Jun 4, 2021

amyangfei commented Jun 4, 2021

ti-chi-bot commented Jun 4, 2021

ti-chi-bot commented Jun 4, 2021

amyangfei commented May 31, 2021 •

edited

Loading