executor: add concurrency limit on union executor #19827

wshwsh12 · 2020-09-07T03:56:24Z

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:

Use system variable tidb_executor_concurrency to control the concurrency on union executor.
Create n worker to pull result from executors.
Close children executors that has been opened.

How it Works:

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch 4.0

Check List

Tests

Unit test

Side effects

Performance regression

Release note

Support adjust the concurrency on union executor.

XuHuaiyu

rest LGTM

executor/executor.go

XuHuaiyu · 2020-09-08T06:58:30Z

LGTM

executor/executor.go

lysu · 2020-09-08T08:00:04Z

executor/executor.go

+				return
+			case result.chk = <-e.resourcePools[workerID]:
+			}
+			result.err = Next(ctx, e.children[childID], result.chk)


and can we make children[childID]'s Open call be lazy to the time before first invokes Next?

#16815 (comment)

we also have a problem that limit 1 but buildRange for all partitions

I have a try to open children executor in resultPuller, but get race in test.
Any idea to fix it?

[2020-09-07T06:00:55.991Z] WARNING: DATA RACE [2020-09-07T06:00:55.991Z] Write at 0x00c0936a8b30 by goroutine 517: [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/store/tikv.(*tikvSnapshot).SetOption() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/store/tikv/snapshot.go:468 +0x3e8 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*BatchPointGetExec).Open() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/batch_point_get.go:111 +0x8b3 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*UnionExec).resultPuller() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:1470 +0x775 [2020-09-07T06:00:55.991Z] [2020-09-07T06:00:55.991Z] Previous read at 0x00c0936a8b30 by goroutine 1012: [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/store/tikv.(*tikvSnapshot).batchGetSingleRegion() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/store/tikv/snapshot.go:243 +0x178 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/store/tikv.(*tikvSnapshot).batchGetKeysByRegions() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/store/tikv/snapshot.go:214 +0xa2a [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/store/tikv.(*tikvSnapshot).BatchGet() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/store/tikv/snapshot.go:140 +0x715 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/kv.(*BufferBatchGetter).BatchGet() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/kv/utils.go:102 +0x99b [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*BatchPointGetExec).initialize() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/batch_point_get.go:301 +0xc40 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*BatchPointGetExec).Next() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/batch_point_get.go:141 +0x819 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.Next() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:268 +0x27d [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*ProjectionExec).unParallelExecute() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/projection.go:187 +0x1db [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*ProjectionExec).Next() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/projection.go:173 +0xa9 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.Next() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:268 +0x27d [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*UnionExec).resultPuller() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:1485 +0x4df [2020-09-07T06:00:55.991Z] [2020-09-07T06:00:55.991Z] Goroutine 517 (running) created at: [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*UnionExec).initialize() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:1442 +0x548 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*UnionExec).Next() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:1503 +0x520 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.Next() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:268 +0x27d [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/executor.(*recordSet).Next() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/adapter.go:127 +0x110 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/session.(*execStmtResult).Next() [2020-09-07T06:00:55.991Z] <autogenerated>:1 +0x84 [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/session.GetRows4Test() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:282 +0x35a [2020-09-07T06:00:55.991Z] github.com/pingcap/tidb/session.ResultSetToStringSlice() [2020-09-07T06:00:55.991Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:300 +0xb8 [2020-09-07T06:00:55.992Z] [2020/09/07 14:00:55.774 +08:00] [INFO] [ddl.go:517] ["[ddl] DDL job is finished"] [jobID=73] [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/util/testkit.(*TestKit).ResultSetToResultWithCtx() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:311 +0xa8 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/util/testkit.(*TestKit).MustQuery() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:306 +0x4e4 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor_test.(*testPointGetSuite).TestClusterIndexCBOPointGet() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/point_get_test.go:586 +0x4b4 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor_test.(*testPointGetSuite).TestClusterIndexCBOPointGet() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/point_get_test.go:572 +0x1cf [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor_test.(*testPointGetSuite).TestClusterIndexCBOPointGet() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/point_get_test.go:571 +0x198 [2020-09-07T06:00:55.992Z] [2020/09/07 14:00:55.779 +08:00] [INFO] [domain.go:646] ["performing DDL change, must reload"] [2020-09-07T06:00:55.992Z] runtime.call32() [2020-09-07T06:00:55.992Z] /usr/local/go/src/runtime/asm_amd64.s:539 +0x3a [2020-09-07T06:00:55.992Z] reflect.Value.Call() [2020-09-07T06:00:55.992Z] /usr/local/go/src/reflect/value.go:321 +0xd3 [2020-09-07T06:00:55.992Z] github.com/pingcap/check.(*suiteRunner).forkTest.func1() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:850 +0x9aa [2020-09-07T06:00:55.992Z] github.com/pingcap/check.(*suiteRunner).forkCall.func1() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:739 +0x113 [2020-09-07T06:00:55.992Z] [2020-09-07T06:00:55.992Z] Goroutine 1012 (running) created at: [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor.(*UnionExec).initialize() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:1442 +0x548 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor.(*UnionExec).Next() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:1503 +0x520 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor.Next() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:268 +0x27d [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor.(*recordSet).Next() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/adapter.go:127 +0x110 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/session.(*execStmtResult).Next() [2020-09-07T06:00:55.992Z] <autogenerated>:1 +0x84 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/session.GetRows4Test() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:282 +0x35a [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/session.ResultSetToStringSlice() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:300 +0xb8 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/util/testkit.(*TestKit).ResultSetToResultWithCtx() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:311 +0xa8 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/util/testkit.(*TestKit).MustQuery() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:306 +0x4e4 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor_test.(*testPointGetSuite).TestClusterIndexCBOPointGet() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/point_get_test.go:586 +0x4b4 [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor_test.(*testPointGetSuite).TestClusterIndexCBOPointGet() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/point_get_test.go:572 +0x1cf [2020-09-07T06:00:55.992Z] github.com/pingcap/tidb/executor_test.(*testPointGetSuite).TestClusterIndexCBOPointGet() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/point_get_test.go:571 +0x198 [2020-09-07T06:00:55.992Z] runtime.call32() [2020-09-07T06:00:55.992Z] /usr/local/go/src/runtime/asm_amd64.s:539 +0x3a [2020-09-07T06:00:55.992Z] reflect.Value.Call() [2020-09-07T06:00:55.992Z] /usr/local/go/src/reflect/value.go:321 +0xd3 [2020-09-07T06:00:55.992Z] github.com/pingcap/check.(*suiteRunner).forkTest.func1() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:850 +0x9aa [2020-09-07T06:00:55.992Z] github.com/pingcap/check.(*suiteRunner).forkCall.func1() [2020-09-07T06:00:55.992Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:739 +0x113

oh...- - maybe we could solve this problem later..

lysu

LGTM

AilinKid · 2020-09-08T10:49:54Z

/merge

ti-srebot · 2020-09-08T10:49:56Z

@AilinKid Oops! auto merge is restricted to Committers of the SIG.See the corresponding SIG page for more information. Related SIG: execution(slack).

wshwsh12 · 2020-09-08T10:56:57Z

/merge

ti-srebot · 2020-09-08T10:57:06Z

/run-all-tests

ti-srebot · 2020-09-08T11:03:26Z

@wshwsh12 merge failed.

wshwsh12 · 2020-09-08T11:12:36Z

/run-mybatis-test

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2020-09-08T11:17:21Z

cherry pick to release-3.0 in PR #19885

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2020-09-08T11:18:41Z

cherry pick to release-4.0 in PR #19886

* cherry pick #19827 to release-3.0 Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

wshwsh12 added 6 commits September 4, 2020 18:21

fix

8f09fe3

reuse chunk

ec23495

open executor serially

92dfd39

fix

586251c

fix

a5d35ce

add ut

3bb20ca

wshwsh12 requested a review from a team as a code owner September 7, 2020 03:56

wshwsh12 requested review from XuHuaiyu and removed request for a team September 7, 2020 03:56

github-actions bot added the sig/execution SIG execution label Sep 7, 2020

wshwsh12 added 4 commits September 7, 2020 13:58

make test happy

bd54dee

Merge remote-tracking branch 'upstream/master' into union

9f155ff

fix

5fda8e1

Merge remote-tracking branch 'upstream/master' into union

a450ca1

wshwsh12 force-pushed the union branch from 4fadc37 to a450ca1 Compare September 7, 2020 07:46

XuHuaiyu reviewed Sep 8, 2020

View reviewed changes

executor/executor.go Outdated Show resolved Hide resolved

results

b0985c3

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 8, 2020

wshwsh12 requested review from tiancaiamao and lysu September 8, 2020 07:00

wshwsh12 added the needs-cherry-pick-4.0 label Sep 8, 2020

lysu reviewed Sep 8, 2020

View reviewed changes

executor/executor.go Show resolved Hide resolved

lysu reviewed Sep 8, 2020

View reviewed changes

ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 8, 2020

ti-srebot approved these changes Sep 8, 2020

View reviewed changes

ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 8, 2020

Merge branch 'master' into union

2b7fa20

AilinKid added the needs-cherry-pick-3.0 label Sep 8, 2020

AilinKid mentioned this pull request Sep 8, 2020

executor: add concurrency limit on the union executor #16815

Closed

tiancaiamao added the priority/release-blocker This issue blocks a release. Please solve it ASAP. label Sep 8, 2020

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 8, 2020

wshwsh12 merged commit 354f399 into pingcap:master Sep 8, 2020

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Sep 8, 2020

cherry pick pingcap#19827 to release-3.0

80425b2

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Sep 8, 2020

executor: add concurrency limit on union executor (#19827) #19885

Merged

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Sep 8, 2020

cherry pick pingcap#19827 to release-4.0

8054dd4

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Sep 8, 2020

executor: add concurrency limit on union executor (#19827) #19886

Merged

SunRunAway pushed a commit that referenced this pull request Sep 9, 2020

executor: add concurrency limit on union executor (#19827) (#19886)

be055a3

zhouqiang-cl pushed a commit that referenced this pull request Sep 17, 2020

executor: add concurrency limit on union executor (#19827) (#19885)

179c131

* cherry pick #19827 to release-3.0 Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

lysu mentioned this pull request Dec 2, 2020

Support lazy open union children #21441

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

executor: add concurrency limit on union executor #19827

executor: add concurrency limit on union executor #19827

wshwsh12 commented Sep 7, 2020 •

edited

Loading

XuHuaiyu left a comment

XuHuaiyu commented Sep 8, 2020

lysu Sep 8, 2020

wshwsh12 Sep 8, 2020

lysu Sep 8, 2020

lysu left a comment

AilinKid commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

wshwsh12 commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

wshwsh12 commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

executor: add concurrency limit on union executor #19827

executor: add concurrency limit on union executor #19827

Conversation

wshwsh12 commented Sep 7, 2020 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

XuHuaiyu left a comment

Choose a reason for hiding this comment

XuHuaiyu commented Sep 8, 2020

lysu Sep 8, 2020

Choose a reason for hiding this comment

wshwsh12 Sep 8, 2020

Choose a reason for hiding this comment

lysu Sep 8, 2020

Choose a reason for hiding this comment

lysu left a comment

Choose a reason for hiding this comment

AilinKid commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

wshwsh12 commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

wshwsh12 commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

ti-srebot commented Sep 8, 2020

wshwsh12 commented Sep 7, 2020 •

edited

Loading