kv-client: use worker pool in region worker #1481

amyangfei · 2021-03-03T11:24:34Z

What problem does this PR solve?

Part of #1393

To increase the throughput of kv client, we should use multiple goroutines in region worker

What is changed and how it works?

Reuse github.com/pingcap/ticdc/pkg/workerpool

Check List

Tests

Unit test
Integration test

Release note

No release note

amyangfei · 2021-04-16T10:27:44Z

/run-all-tests

amyangfei · 2021-04-20T02:26:08Z

/run-all-tests

amyangfei · 2021-04-20T08:05:55Z

/run-all-tests

amyangfei · 2021-04-20T08:33:40Z

/run-all-tests

amyangfei · 2021-04-27T07:42:59Z

PTAL @liuzix @leoppro @overvenus

zier-one · 2021-04-28T02:33:06Z

cdc/kv/client_test.go

-func Test(t *testing.T) { check.TestingT(t) }
+func Test(t *testing.T) {
+	conf := config.GetDefaultServerConfig()
+	config.StoreGlobalServerConfig(conf)


should we set a default config in init function?

zier-one · 2021-04-28T02:37:39Z

cdc/server.go

@@ -263,6 +264,10 @@ func (s *Server) run(ctx context.Context) (err error) {
 		return sorter.RunWorkerPool(cctx)
 	})

+	wg.Go(func() error {


maybe we can run worker pool in init function?

Yep either run it in init function or explicit invocation works, but IMHO the init function is a little dangerous in some scenarios, which may make initialization step unclear (such as init sequence, unexpected init when we import a package but don't want it to init, etc)
such as the discussion in this article: https://medium.com/higher-order-functions/golang-how-to-sensibly-use-init-function-8077a4ec15e5

overvenus · 2021-05-18T09:04:15Z

/lgtm

ti-chi-bot · 2021-05-18T09:04:18Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

leoppro
overvenus

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

3AceShowHand · 2021-05-18T10:54:39Z

/run-unit-tests

3AceShowHand · 2021-05-18T11:21:04Z

cdc/kv/region_worker.go

+	regionWorkerPool workerpool.WorkerPool
+	workerPoolOnce   sync.Once
+	// TODO: unified channel buffer mechanism
+	regionWorkerInputChanSize = 128000


can you add a comment about this magic 128000?

added some comments

amyangfei · 2021-05-19T07:10:14Z

/merge

ti-chi-bot · 2021-05-19T07:10:17Z

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

Commit hash: 862f695

purelind · 2021-05-19T07:14:34Z

/run-all-tests

amyangfei · 2021-05-19T13:06:00Z

/run-kafka-tests

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

ti-chi-bot · 2021-05-19T13:34:20Z

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

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

ti-chi-bot · 2021-05-19T13:34:57Z

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

…-5.0 (#1922)

amyangfei added status/WIP component/kv-client TiKV kv log client component. needs-cherry-pick-release-4.0 Should cherry pick this PR to release-4.0 branch. labels Mar 3, 2021

amyangfei added this to the v5.0.0 milestone Mar 3, 2021

amyangfei force-pushed the kv-client-optimization-region-worker-pool branch from 34ed9e2 to f46e923 Compare March 5, 2021 08:31

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2021

amyangfei modified the milestones: v5.0.0, v5.0.1 Apr 9, 2021

amyangfei force-pushed the kv-client-optimization-region-worker-pool branch from 9d0912b to 531d440 Compare April 15, 2021 03:03

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 15, 2021

amyangfei added the needs-cherry-pick-release-5.0 Should cherry pick this PR to release-5.0 branch. label Apr 15, 2021

amyangfei force-pushed the kv-client-optimization-region-worker-pool branch 2 times, most recently from d6da339 to 168a20b Compare April 16, 2021 10:27

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 Apr 20, 2021

amyangfei added 3 commits April 20, 2021 15:49

kv-client: use worker pool in region worker

fda2c25

add server config for region worker

951c7cb

fix unit test

4dc2f83

amyangfei force-pushed the kv-client-optimization-region-worker-pool branch from 3c6d569 to 4dc2f83 Compare April 20, 2021 07:49

amyangfei added status/ptal Could you please take a look? and removed status/wip labels Apr 20, 2021

amyangfei modified the milestones: v5.0.1, v5.0.2 Apr 22, 2021

zier-one approved these changes Apr 28, 2021

View reviewed changes

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 May 18, 2021

3AceShowHand reviewed May 18, 2021

View reviewed changes

add comments

862f695

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

Merge branch 'master' into kv-client-optimization-region-worker-pool

7b26536

Merge branch 'master' into kv-client-optimization-region-worker-pool

28ead63

ti-chi-bot merged commit 075a31d into pingcap:master May 19, 2021

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

This is an automated cherry-pick of pingcap#1481

c4f6043

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

ti-chi-bot mentioned this pull request May 19, 2021

kv-client: use worker pool in region worker (#1481) #1802

Closed

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

This is an automated cherry-pick of pingcap#1481

10cbc4e

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

ti-chi-bot mentioned this pull request May 19, 2021

kv-client: use worker pool in region worker (#1481) #1803

Closed

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

kv-client: use worker pool in region worker (pingcap#1481)

3ae40e9

amyangfei mentioned this pull request Jun 3, 2021

kv/client: cherry pick #1591,#1674,#1682,#1481,#1847,#1903 to release-5.0 #1922

Merged

ti-chi-bot pushed a commit that referenced this pull request Jun 10, 2021

kv/client: cherry pick #1591,#1674,#1682,#1481,#1847,#1903 to release…

fc4adf4

…-5.0 (#1922)

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

kv-client: use worker pool in region worker (pingcap#1481)

f251bef

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: use worker pool in region worker (pingcap#1481)

4bb5502

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

kv-client: use worker pool in region worker (pingcap#1481)

062941c

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

kv-client: use worker pool in region worker (pingcap#1481)

2a724a6

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

kv-client: use worker pool in region worker (#1481)

bb813a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kv-client: use worker pool in region worker #1481

kv-client: use worker pool in region worker #1481

amyangfei commented Mar 3, 2021

amyangfei commented Apr 16, 2021

amyangfei commented Apr 20, 2021

amyangfei commented Apr 20, 2021

amyangfei commented Apr 20, 2021

amyangfei commented Apr 27, 2021

zier-one Apr 28, 2021

zier-one Apr 28, 2021

amyangfei Apr 28, 2021 •

edited

Loading

overvenus commented May 18, 2021

ti-chi-bot commented May 18, 2021

3AceShowHand commented May 18, 2021

3AceShowHand May 18, 2021

amyangfei May 19, 2021

amyangfei commented May 19, 2021

ti-chi-bot commented May 19, 2021

purelind commented May 19, 2021

amyangfei commented May 19, 2021

ti-chi-bot commented May 19, 2021

ti-chi-bot commented May 19, 2021

kv-client: use worker pool in region worker #1481

kv-client: use worker pool in region worker #1481

Conversation

amyangfei commented Mar 3, 2021

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

amyangfei commented Apr 16, 2021

amyangfei commented Apr 20, 2021

amyangfei commented Apr 20, 2021

amyangfei commented Apr 20, 2021

amyangfei commented Apr 27, 2021

zier-one Apr 28, 2021

Choose a reason for hiding this comment

zier-one Apr 28, 2021

Choose a reason for hiding this comment

amyangfei Apr 28, 2021 • edited Loading

Choose a reason for hiding this comment

overvenus commented May 18, 2021

ti-chi-bot commented May 18, 2021

3AceShowHand commented May 18, 2021

3AceShowHand May 18, 2021

Choose a reason for hiding this comment

amyangfei May 19, 2021

Choose a reason for hiding this comment

amyangfei commented May 19, 2021

ti-chi-bot commented May 19, 2021

purelind commented May 19, 2021

amyangfei commented May 19, 2021

ti-chi-bot commented May 19, 2021

ti-chi-bot commented May 19, 2021

amyangfei Apr 28, 2021 •

edited

Loading