[SPARK-32995][SQL] CostBasedJoinReorder optimizer rule should be idempotent #29871

tanelk · 2020-09-25T16:54:49Z

What changes were proposed in this pull request?

Mark the "Join Reorder" batch as idempotent.
The CostBasedJoinReorder needed a slight modification to satisfy this rule.

There are some changes in the plan stability suite. They are caused by changing the iteration order in CostBasedJoinReorder.

Why are the changes needed?

Idempotence check on optimizer batches is invaluable for developers. It can avoid us bugs in the future.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing UTs mostly cover it, also added one extra

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/CostBasedJoinReorder.scala

tanelk · 2020-09-25T17:15:33Z

@LuciferYang, it seems you were working on similar issue, but you closed it - #29638

SparkQA · 2020-09-25T17:40:12Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33736/

SparkQA · 2020-09-25T17:47:33Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33737/

SparkQA · 2020-09-25T18:04:35Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33736/

SparkQA · 2020-09-25T18:09:42Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33737/

tanelk · 2020-09-25T19:33:59Z

Closing for now, because there is some issue with the golden files

SparkQA · 2020-09-25T20:27:49Z

Test build #129118 has finished for PR 29871 at commit 7a44562.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-09-25T21:02:33Z

Test build #129116 has finished for PR 29871 at commit d58966c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

LuciferYang · 2020-09-27T02:24:52Z

If there are multiple candidate plans with same cost, I think different inputs produce different outputs is reasonable，so only need ensure the same behavior in Scala 2.12 and Scala 2.13

This reverts commit d58966c

tanelk · 2020-09-27T20:55:12Z

If there are multiple candidate plans with same cost, I think different inputs produce different outputs is reasonable，so only need ensure the same behavior in Scala 2.12 and Scala 2.13

With this change optimally ordered input doesn't get reordered into another plan with the same cost - this makes CostBasedJoinReorder idempotent, that can help catch bugs.

I had a wrong approach to solving this earlier, now it should stable.

SparkQA · 2020-09-27T20:57:04Z

Test build #129154 has finished for PR 29871 at commit c9028ad.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-09-27T21:40:19Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33769/

SparkQA · 2020-09-27T21:44:40Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33770/

SparkQA · 2020-09-27T22:04:19Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33769/

SparkQA · 2020-09-27T22:05:23Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33770/

SparkQA · 2020-09-28T01:31:27Z

Test build #129155 has finished for PR 29871 at commit 4619acc.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

LuciferYang · 2020-09-28T02:57:15Z

Does this PR affect the behavior in Scala 2.13? Can you help verify it offline?

maropu · 2020-09-28T06:35:45Z

Have you checked actuall performance changes with sf=100? I think it would be better to check them for avoiding accidental performance regression.

SparkQA · 2021-01-05T12:14:12Z

Test build #133658 has finished for PR 29871 at commit 6248e4a.

This patch passes all tests.
This patch does not merge cleanly.
This patch adds no public classes.

tanelk · 2021-01-05T13:38:54Z

Now that #30965 is merged, this changes the plans a lot less.
But there is still idempotency issue with plans with equal costs.

SparkQA · 2021-01-05T15:02:21Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38274/

SparkQA · 2021-01-05T15:35:48Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38274/

SparkQA · 2021-01-05T18:22:15Z

Test build #133685 has finished for PR 29871 at commit f815feb.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-01-16T19:31:43Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38731/

SparkQA · 2021-01-16T20:01:12Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38731/

SparkQA · 2021-01-16T23:41:03Z

Test build #134148 has finished for PR 29871 at commit f7744db.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-01-26T18:15:58Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39101/

SparkQA · 2021-01-26T18:27:02Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39102/

SparkQA · 2021-01-26T18:31:52Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39102/

SparkQA · 2021-01-26T18:48:47Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39101/

SparkQA · 2021-01-26T21:56:07Z

Test build #134517 has finished for PR 29871 at commit 5fe5f3b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-03-20T08:07:40Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40862/

SparkQA · 2021-03-20T08:18:06Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40862/

SparkQA · 2021-03-20T11:29:04Z

Test build #136280 has finished for PR 29871 at commit d6522d9.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-04-02T09:56:20Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41428/

SparkQA · 2021-04-02T09:56:21Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41428/

SparkQA · 2021-04-02T12:39:14Z

Test build #136850 has finished for PR 29871 at commit d2a6128.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

github-actions · 2021-07-12T00:07:57Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

tanelk added 2 commits September 24, 2020 08:30

CBO once

3628972

make CostBasedJoinReorder idempotent

d58966c

probot-autolabeler bot added the SQL label Sep 25, 2020

tanelk commented Sep 25, 2020

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/CostBasedJoinReorder.scala Outdated Show resolved Hide resolved

tanelk commented Sep 25, 2020

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/CostBasedJoinReorder.scala Outdated Show resolved Hide resolved

Add CostBasedJoinReorder idempotence check to UTs

7a44562

Merge branch 'master' into SPARK-32995

1aec54b

tanelk closed this Sep 25, 2020

tanelk added 2 commits September 27, 2020 19:17

Revert "make CostBasedJoinReorder idempotent"

c995703

This reverts commit d58966c

make CostBasedJoinReorder idempotent

c9028ad

tanelk reopened this Sep 27, 2020

style

4619acc

Tanel Kiis added 3 commits January 5, 2021 15:15

Merge branch 'master' into SPARK-32995

fe7da48

Rerun PlanStabilitySuite

09c5da8

Rerun PlanStabilitySuite

f815feb

Merge branch 'master' into SPARK-32995_idempotent_cbo

f7744db

tanelk added 2 commits January 26, 2021 19:14

rerun plan stability

d09abd1

rerun plan stability

5fe5f3b

Rerun plan stability suite

d6522d9

Rerun plan stability suite

d2a6128

github-actions bot added the Stale label Jul 12, 2021

github-actions bot closed this Jul 13, 2021

[SPARK-32995][SQL] CostBasedJoinReorder optimizer rule should be idempotent #29871

[SPARK-32995][SQL] CostBasedJoinReorder optimizer rule should be idempotent #29871

Uh oh!

Conversation

tanelk commented Sep 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

Uh oh!

tanelk commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 25, 2020

Uh oh!

tanelk commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 25, 2020

Uh oh!

LuciferYang commented Sep 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tanelk commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 28, 2020

Uh oh!

LuciferYang commented Sep 28, 2020

Uh oh!

maropu commented Sep 28, 2020

Uh oh!

SparkQA commented Jan 5, 2021

Uh oh!

tanelk commented Jan 5, 2021

Uh oh!

SparkQA commented Jan 5, 2021

Uh oh!

SparkQA commented Jan 5, 2021

Uh oh!

SparkQA commented Jan 5, 2021

Uh oh!

SparkQA commented Jan 16, 2021

Uh oh!

SparkQA commented Jan 16, 2021

Uh oh!

SparkQA commented Jan 16, 2021

Uh oh!

SparkQA commented Jan 26, 2021

Uh oh!

SparkQA commented Jan 26, 2021

Uh oh!

SparkQA commented Jan 26, 2021

Uh oh!

SparkQA commented Jan 26, 2021

Uh oh!

SparkQA commented Jan 26, 2021

Uh oh!

SparkQA commented Mar 20, 2021

Uh oh!

SparkQA commented Mar 20, 2021

Uh oh!

tanelk commented Sep 25, 2020 •

edited

Loading

LuciferYang commented Sep 27, 2020 •

edited

Loading