planner, executor: support broadcast join for tiflash engine. #17232

hanfei1991 · 2020-05-15T06:19:08Z

What problem does this PR solve?

Support Broadcast Join for TiFlash Engine.

What is changed and how it works?

Change Details and Description can be referred from : https://docs.google.com/document/d/1RXW4kEKhxVS1hRsKA9Vlr-Mn94X8dMNexbuiJqfrt4A/edit?usp=sharing

Related changes

Tipb and parser is also changed:

Check List

Side effects

Breaking backward compatibility

TiFlash must also be updated before update for TiDB

Release note

Support broadcast join for tiflash.

…join-merge

…into hanfei/join-merge

fzhedu · 2020-07-20T13:41:37Z

planner/core/testdata/integration_serial_suite_out.json

+          "StreamAgg_32 1.00 root  funcs:count(Column#14)->Column#11",
+          "└─TableReader_33 1.00 root  data:StreamAgg_13",
+          "  └─StreamAgg_13 1.00 cop[tiflash]  funcs:count(1)->Column#14",
+          "    └─TypeBroadcastJoin_31 8.00 cop[tiflash]  ",


TypeBroadCastJoin -> BroadCastJoin ?

fzhedu · 2020-07-20T13:43:11Z

planner/util/path.go

@@ -50,6 +50,9 @@ type AccessPath struct {

 	IsDNFCond bool

+	// IsGlobalRead indicates whether this path is a remote read path for tiflash
+	IsGlobalRead bool


IsGlobalRead is not obvious, it is not related to tiflash.

Done, changed to IsTiFlashGlobalRead

fzhedu · 2020-07-20T13:45:08Z

sessionctx/variable/varsutil.go

@@ -451,6 +451,11 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string, scope Sc
 			return "ON", nil
 		}
 		return value, ErrWrongValueForVar.GenWithStackByArgs(name, value)
+	case TiDBOptBCJ:
+		if (strings.EqualFold(value, "ON") || value == "1") && vars.AllowBatchCop == 0 {
+			return value, ErrWrongValueForVar.GenWithStackByArgs("Can't set BCJ to 1 but tidb_allow_batch_cop is 0, please active batch cop at first.")


BCJ -> BroadCastJoin is more direct.

fzhedu · 2020-07-20T13:45:30Z

sessionctx/variable/varsutil.go

@@ -575,11 +580,15 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string, scope Sc
 		if err != nil {
 			return value, ErrWrongTypeForVar.GenWithStackByArgs(name)
 		}
+		if v == 0 && vars.AllowBCJ {
+			return value, ErrWrongValueForVar.GenWithStackByArgs("Can't set batch cop 0 but tidb_opt_broadcast_join is 1, please set bcj 0 at first")


fzhedu · 2020-07-20T13:48:08Z

util/plancodec/id.go

@@ -52,6 +52,8 @@ const (
 	TypeLimit = "Limit"
 	// TypeHashJoin is the type of hash join.
 	TypeHashJoin = "HashJoin"
+	// TypeBroadcastJoin is the type of broad cast join.
+	TypeBroadcastJoin = "TypeBroadcastJoin"


TypeBroadcastJoin = "BroadcastJoin"

fzhedu · 2020-07-23T12:00:14Z

planner/core/exhaust_physical_plans.go

+	if prop.TaskTp == property.CopTiFlashGlobalReadTaskType {
+		childrenReqProps[1-preferredGlobalIndex] = &property.PhysicalProperty{TaskTp: property.CopTiFlashGlobalReadTaskType, ExpectedCnt: math.MaxFloat64}
+	} else {
+		childrenReqProps[1-preferredGlobalIndex] = &property.PhysicalProperty{TaskTp: property.CopTiFlashLocalReadTaskType, ExpectedCnt: math.MaxFloat64}
+	}


This if is not necessary depending on the prop.TaskTp would be either CopTiFlashGlobalReadTaskType or CopTiFlashLocalReadTaskType?
&property.PhysicalProperty{TaskTp: prop.TaskTP, ExpectedCnt: math.MaxFloat64} is ok?

It's not ok when prop.TaskTp is rootTask. When it requires rootTask, its child can return a TiFlashTask , close it and get a root task.

fzhedu · 2020-07-23T12:06:48Z

planner/core/exhaust_physical_plans.go

+	if prop.ExpectedCnt < p.stats.RowCount {
+		expCntScale := prop.ExpectedCnt / p.stats.RowCount
+		childrenReqProps[1-baseJoin.InnerChildIdx].ExpectedCnt = p.children[1-baseJoin.InnerChildIdx].statsInfo().RowCount * expCntScale
+	}


what is the meaning of these code? can you add some comments?

It's referred from

tidb/planner/core/exhaust_physical_plans.go

Line 345 in adc2d04

if prop.ExpectedCnt < p.stats.RowCount {

ExpectedCnt means expected returned rows before sql stops, sometimes it's different from stats.rowCount if and only if there is a limit in parent operators which makes it stop early.

The comments have existed in ExpectedCnt field, so it's no need to add it here

fzhedu · 2020-07-23T12:30:19Z

planner/core/task.go

+	numPairs := helper.estimate()
+	probeCost := numPairs * sessVars.CopCPUFactor
+	// should divided by the concurrency in tiflash, which should be the number of core in tiflash nodes.
+	probeCost /= float64(sessVars.CopTiFlashConcurrencyFactor)


does CopTiFlashConcurrencyFactor guarentee > 0?

Yes, it's check in varsutil.go

fzhedu

LGTM after solve comments

fzhedu

LGTM

lzmhhh123 · 2020-07-27T04:30:53Z

/merge

ti-srebot · 2020-07-27T04:31:00Z

/run-all-tests

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

ti-srebot · 2020-07-27T07:43:44Z

cherry pick to release-4.0 in PR #18801

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

hanfei1991 and others added 30 commits March 7, 2020 18:48

support batch cop for tiflash

9122e31

Merge branch 'master' into hanfei/batch_cop

8456ef3

support batch cop

460d5a8

support join push down to tiflash

7a147b6

refine

943863b

change pb

a654fda

push join

2d7e3c0

fix

7b6ad95

add hint

4aae81b

refine hint

7c49728

add ranges

a0a88f3

fix

e90731b

fix

239cc17

fix push down

00d417a

fix index

deb6fd2

enable distsql for join

55477c7

Merge branch 'master' into hanfei/join-merge

c01e9a5

add a session var to disable/enable broadcast join

1dbbb56

fix bug

c4b2e13

Merge branch 'master' of https://github.com/pingcap/tidb into hanfei/…

49ff997

…join-merge

Merge branch 'master' into hanfei/join-merge

35e66a1

fix bug

2c92a29

tiny fix

05ade28

enable cast decimal pushdown to tiflash

4f14acf

Merge branch 'master' of https://github.com/pingcap/tidb into hanfei/…

ea9a0f1

…join-merge

fix

cd7f225

merge master

bf8bf6e

fix bc join bug

04b4cf6

Merge branch 'hanfei/join-merge' of https://github.com/hanfei1991/tidb …

3d1c0ed

…into hanfei/join-merge

make broadcast plan stable

40e3990

Merge branch 'master' into hanfei/join-merge

83a1761

pingcap deleted a comment from hanfei1991 Jul 20, 2020

fzhedu reviewed Jul 20, 2020

View reviewed changes

address comments

84a3539

fzhedu reviewed Jul 23, 2020

View reviewed changes

fzhedu suggested changes Jul 23, 2020

View reviewed changes

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

fzhedu approved these changes Jul 25, 2020

View reviewed changes

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

Merge branch 'master' into hanfei/join-merge

e871df4

lzmhhh123 approved these changes Jul 27, 2020

View reviewed changes

ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 27, 2020

ti-srebot approved these changes Jul 27, 2020

View reviewed changes

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

ti-srebot merged commit 29178df into pingcap:master Jul 27, 2020

hanfei1991 added the needs-cherry-pick-4.0 label Jul 27, 2020

ti-srebot mentioned this pull request Jul 27, 2020

planner, executor: support broadcast join for tiflash engine. (#17232) #18801

Merged

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

cherry pick pingcap#17232 to release-4.0

6301f01

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

lidezhu mentioned this pull request Aug 11, 2020

Sqllogic test case(sqllogictest/test/select/select1.test) failed on tiflash pingcap/tiflash#999

Closed

ti-srebot added a commit that referenced this pull request Sep 1, 2020

planner, executor: support broadcast join for tiflash engine. (#17232) (

8b3ff32

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner, executor: support broadcast join for tiflash engine. #17232

planner, executor: support broadcast join for tiflash engine. #17232

hanfei1991 commented May 15, 2020

fzhedu Jul 20, 2020

hanfei1991 Jul 21, 2020

fzhedu Jul 20, 2020

hanfei1991 Jul 21, 2020

fzhedu Jul 20, 2020

hanfei1991 Jul 21, 2020

fzhedu Jul 20, 2020

hanfei1991 Jul 21, 2020

fzhedu Jul 20, 2020

hanfei1991 Jul 21, 2020

fzhedu Jul 23, 2020

hanfei1991 Jul 24, 2020

fzhedu Jul 23, 2020

hanfei1991 Jul 24, 2020

hanfei1991 Jul 24, 2020

fzhedu Jul 23, 2020

hanfei1991 Jul 24, 2020

fzhedu left a comment

fzhedu left a comment

lzmhhh123 commented Jul 27, 2020

ti-srebot commented Jul 27, 2020

ti-srebot commented Jul 27, 2020

planner, executor: support broadcast join for tiflash engine. #17232

planner, executor: support broadcast join for tiflash engine. #17232

Conversation

hanfei1991 commented May 15, 2020

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

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

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

fzhedu left a comment

Choose a reason for hiding this comment

fzhedu left a comment

Choose a reason for hiding this comment

lzmhhh123 commented Jul 27, 2020

ti-srebot commented Jul 27, 2020

ti-srebot commented Jul 27, 2020