Skip to content

[SPARK-30074][SQL] The maxNumPostShufflePartitions config should obey reducePostShufflePartitions enabled - #26664

Closed
xuanyuanking wants to merge 3 commits into
apache:masterfrom
xuanyuanking:SPARK-9853-follow
Closed

[SPARK-30074][SQL] The maxNumPostShufflePartitions config should obey reducePostShufflePartitions enabled#26664
xuanyuanking wants to merge 3 commits into
apache:masterfrom
xuanyuanking:SPARK-9853-follow

Conversation

@xuanyuanking

@xuanyuankingxuanyuanking commented Nov 25, 2019•
edited
Loading

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

  1. Make maxNumPostShufflePartitions config obey reducePostShfflePartitions config.
  2. Update the description for all the SQLConf affected by spark.sql.adaptive.enabled.

Why are the changes needed?

Make the relation between these confs clearer.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing UT.

@xuanyuankingxuanyuanking mentioned this pull request Nov 25, 2019
Closed
cloud-fan
cloud-fan reviewed Nov 25, 2019
View reviewed changes
Comment threadsql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated
buildConf("spark.sql.adaptive.shuffle.targetPostShuffleInputSize")
.doc("The target post-shuffle input size in bytes of a task.")
.doc("The target post-shuffle input size in bytes of a task. This configuration only has " +
s"an effect when '${ADAPTIVE_EXECUTION_ENABLED.key}' is enabled.")

@cloud-fancloud-fanNov 25, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and also when spark.sql.adaptive.shuffle.reducePostShufflePartitions.enabled is true?

@xuanyuankingxuanyuankingNov 25, 2019

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fix the doc of SHUFFLE_MIN_NUM_POSTSHUFFLE_PARTITIONS and SHUFFLE_TARGET_POSTSHUFFLE_INPUT_SIZE.

@SparkQA

SparkQA commented Nov 25, 2019

Copy link
Copy Markdown

Test build #114407 has finished for PR 26664 at commit cd71aac.

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

@SparkQA

SparkQA commented Nov 25, 2019

Copy link
Copy Markdown

Test build #114423 has finished for PR 26664 at commit 81c536d.

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

cloud-fan
cloud-fan reviewed Nov 26, 2019
View reviewed changes
Comment threadsql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated
cloud-fan
cloud-fan reviewed Nov 26, 2019
View reviewed changes
Comment threadsql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated
@xuanyuankingxuanyuanking changed the title [SPARK-9853][Doc][Follow-up] Update the description for all the SQLConf affected by spark.sql.adaptive.enabled[SPARK-30074][SQL] The maxNumPostShufflePartitions config should obey reducePostShufflePartitions enabledNov 29, 2019
@SparkQA

SparkQA commented Nov 29, 2019

Copy link
Copy Markdown

Test build #114602 has finished for PR 26664 at commit 69070d7.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@xuanyuanking

xuanyuanking commented Nov 29, 2019

Copy link
Copy Markdown
MemberAuthor

retest this please.

@SparkQA

SparkQA commented Nov 29, 2019

Copy link
Copy Markdown

Test build #114617 has finished for PR 26664 at commit 69070d7.

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

@cloud-fan

cloud-fan commented Dec 2, 2019

Copy link
Copy Markdown
Contributor

thanks, merging to master!

@xuanyuanking

xuanyuanking commented Dec 2, 2019

Copy link
Copy Markdown
MemberAuthor

Thanks!

@xuanyuanking
xuanyuanking deleted the SPARK-9853-follow branch December 2, 2019 04:54
manuzhang
manuzhang reviewed Dec 2, 2019
View reviewed changes
Comment threadsql/core/src/main/scala/org/apache/spark/sql/execution/exchange/EnsureRequirements.scala
case class EnsureRequirements(conf: SQLConf) extends Rule[SparkPlan] {
private def defaultNumPreShufflePartitions: Int =
if (conf.adaptiveExecutionEnabled) {
if (conf.adaptiveExecutionEnabled && conf.reducePostShufflePartitionsEnabled) {

@manuzhangmanuzhangDec 2, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xuanyuanking is there any UT verifying the number of shuffle partitions dependent on values of these two configurations ?

attilapiros pushed a commit to attilapiros/spark that referenced this pull request Dec 6, 2019
@xuanyuanking@attilapiros
[SPARK-30074][SQL] The maxNumPostShufflePartitions config should obey…
21eae40
… reducePostShufflePartitions enabled
### What changes were proposed in this pull request?
1. Make maxNumPostShufflePartitions config obey reducePostShfflePartitions config.
2. Update the description for all the SQLConf affected by `spark.sql.adaptive.enabled`.
### Why are the changes needed?
Make the relation between these confs clearer.
### Does this PR introduce any user-facing change?
No
### How was this patch tested?
Existing UT.
Closesapache#26664 from xuanyuanking/SPARK-9853-follow.
Authored-by: Yuanjian Li <xyliyuanjian@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cloud-fancloud-fancloud-fan left review comments
+1 more reviewer
@manuzhangmanuzhangmanuzhang left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

SPARK CORESQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@xuanyuanking@SparkQA@cloud-fan@manuzhang@dongjoon-hyun