You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/06/04 08:39:08 UTC
[GitHub] [spark] wangyum opened a new pull request #32781: [SPARK-35650][SQL] Coalesce small output files through AQE
wangyum opened a new pull request #32781:
URL: https://github.com/apache/spark/pull/32781
### What changes were proposed in this pull request?
This pr add support coalesce small output files through AQE. The basic logic is: Spark first tries to coalesce output files, if it cannot be coalesced, then try to use the local shuffle reader to read data avoid exchange the data over the network.
### Why are the changes needed?
Avoid generating small files.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856034289
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139424/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647272371
##########
File path: docs/sql-performance-tuning.md
##########
@@ -227,6 +227,7 @@ The "REPARTITION_BY_RANGE" hint must have column names and a partition number is
SELECT /*+ REPARTITION(3, c) */ * FROM t
SELECT /*+ REPARTITION_BY_RANGE(c) */ * FROM t
SELECT /*+ REPARTITION_BY_RANGE(3, c) */ * FROM t
+ SELECT /*+ REPARTITION */ * FROM t
Review comment:
nit: move it to before `REPARTITION_BY_RANGE`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857603486
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44080/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857456902
**[Test build #139554 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139554/testReport)** for PR 32781 at commit [`1c8419b`](https://github.com/apache/spark/commit/1c8419bdcd208afc40fcd838d2f8c0622caa84cb).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856117239
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43946/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856672654
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43996/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857306692
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44039/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856678652
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43996/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857320602
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44039/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647274093
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -712,7 +712,9 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
case r: logical.Range =>
execution.RangeExec(r) :: Nil
case r: logical.RepartitionByExpression =>
- val shuffleOrigin = if (r.optNumPartitions.isEmpty) {
+ val shuffleOrigin = if (r.isUsedToCoalescePartitions) {
+ ENSURE_REQUIREMENTS
Review comment:
how about adding a new one `REPARTITION_WITHOUT_COL_AND_NUM`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647460029
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
cc @yaooqinn @JkSelf
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r646410590
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -1351,6 +1351,45 @@ object RepartitionByExpression {
}
}
+/**
+ * This method repartitions data using [[Expression]]s into `numShufflePartitions` defined in
+ * `SQLConf`, and could be coalesced by AQE. Usually used to merge small files.
+ */
+case class CoalescePartitions(
+ partitionExpressions: Seq[Expression],
+ child: LogicalPlan) extends RepartitionOperation {
+
+ override val numPartitions = conf.numShufflePartitions
+
+ override val partitioning: Partitioning = {
+ val (sortOrder, nonSortOrder) = partitionExpressions.partition(_.isInstanceOf[SortOrder])
+
+ require(sortOrder.isEmpty || nonSortOrder.isEmpty,
+ s"${getClass.getSimpleName} expects that either all its `partitionExpressions` are of type " +
+ "`SortOrder`, which means `RangePartitioning`, or none of them are `SortOrder`, which " +
+ "means `HashPartitioning`. In this case we have:" +
+ s"""
+ |SortOrder: $sortOrder
+ |NonSortOrder: $nonSortOrder
+ """.stripMargin)
+
+ if (numPartitions == 1) {
+ SinglePartition
+ } else if (sortOrder.nonEmpty) {
+ RangePartitioning(sortOrder.map(_.asInstanceOf[SortOrder]), numPartitions)
Review comment:
That's too far away, and we can probably just use the `Repartition` node.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce small output files through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-854502116
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43848/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856895028
**[Test build #139489 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139489/testReport)** for PR 32781 at commit [`d5cc086`](https://github.com/apache/spark/commit/d5cc086555d0fbfb381c919c612a27ccc0c9490f).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856649865
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43991/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856026555
**[Test build #139424 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139424/testReport)** for PR 32781 at commit [`ba75b09`](https://github.com/apache/spark/commit/ba75b09281f04e4376b6da0b402828c33075f8d1).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857682849
**[Test build #139563 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139563/testReport)** for PR 32781 at commit [`6424885`](https://github.com/apache/spark/commit/6424885eb94159f8f16ba3b3ed2afeb5565a11f0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856645836
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43991/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857281347
**[Test build #139514 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139514/testReport)** for PR 32781 at commit [`fe0ff1d`](https://github.com/apache/spark/commit/fe0ff1d0c05354de9b592c045bee15c5872e0514).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856609943
**[Test build #139467 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139467/testReport)** for PR 32781 at commit [`49422e1`](https://github.com/apache/spark/commit/49422e16f2e48c7a3511f4090c9c5009c330f126).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647997967
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -712,7 +712,9 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
case r: logical.Range =>
execution.RangeExec(r) :: Nil
case r: logical.RepartitionByExpression =>
- val shuffleOrigin = if (r.optNumPartitions.isEmpty) {
+ val shuffleOrigin = if (r.partitionExpressions.isEmpty && r.optNumPartitions.isEmpty) {
+ REPARTITION_BY_NONE
+ } else if (r.optNumPartitions.isEmpty) {
REPARTITION
Review comment:
Let's fully adopt the naming suggestion: REPARTITION_BY_COL, REPARTITION_BY_NUM, REPARTITION_BY_NONE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856649785
**[Test build #139473 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139473/testReport)** for PR 32781 at commit [`266d2f0`](https://github.com/apache/spark/commit/266d2f0af3324a765dc1fe6b85945f7b4a40e088).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647997967
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -712,7 +712,9 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
case r: logical.Range =>
execution.RangeExec(r) :: Nil
case r: logical.RepartitionByExpression =>
- val shuffleOrigin = if (r.optNumPartitions.isEmpty) {
+ val shuffleOrigin = if (r.partitionExpressions.isEmpty && r.optNumPartitions.isEmpty) {
+ REPARTITION_BY_NONE
+ } else if (r.optNumPartitions.isEmpty) {
REPARTITION
Review comment:
Let's fully adopt the naming suggestion: REPARTITION_BY_COL, REPARTITION_BY_NUM, REPARTITION_BY_COL_AND_NUM, REPARTITION_BY_NONE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857660022
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139554/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856867980
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44013/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647274944
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CoalesceShufflePartitions.scala
##########
@@ -48,15 +48,20 @@ case class CoalesceShufflePartitions(session: SparkSession) extends CustomShuffl
}
val shuffleStageInfos = collectShuffleStageInfos(plan)
+ val shuffles = shuffleStageInfos.map(_.shuffleStage.shuffle)
// ShuffleExchanges introduced by repartition do not support changing the number of partitions.
// We change the number of partitions in the stage only if all the ShuffleExchanges support it.
- if (!shuffleStageInfos.forall(s => supportCoalesce(s.shuffleStage.shuffle))) {
+ if (!shuffles.forall(supportCoalesce)) {
plan
} else {
// We fall back to Spark default parallelism if the minimum number of coalesced partitions
// is not set, so to avoid perf regressions compared to no coalescing.
- val minPartitionNum = conf.getConf(SQLConf.COALESCE_PARTITIONS_MIN_PARTITION_NUM)
- .getOrElse(session.sparkContext.defaultParallelism)
+ val minPartitionNum = if (shuffles.forall(isUsedToCoalescePartitions)) {
+ 1
Review comment:
why do we need this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855186194
**[Test build #139361 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139361/testReport)** for PR 32781 at commit [`e72dc75`](https://github.com/apache/spark/commit/e72dc75a86fc76d366badb22431da87946117d34).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class CoalescePartitions(`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r646407033
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -1351,6 +1351,45 @@ object RepartitionByExpression {
}
}
+/**
+ * This method repartitions data using [[Expression]]s into `numShufflePartitions` defined in
+ * `SQLConf`, and could be coalesced by AQE. Usually used to merge small files.
+ */
+case class CoalescePartitions(
+ partitionExpressions: Seq[Expression],
+ child: LogicalPlan) extends RepartitionOperation {
+
+ override val numPartitions = conf.numShufflePartitions
+
+ override val partitioning: Partitioning = {
+ val (sortOrder, nonSortOrder) = partitionExpressions.partition(_.isInstanceOf[SortOrder])
+
+ require(sortOrder.isEmpty || nonSortOrder.isEmpty,
+ s"${getClass.getSimpleName} expects that either all its `partitionExpressions` are of type " +
+ "`SortOrder`, which means `RangePartitioning`, or none of them are `SortOrder`, which " +
+ "means `HashPartitioning`. In this case we have:" +
+ s"""
+ |SortOrder: $sortOrder
+ |NonSortOrder: $nonSortOrder
+ """.stripMargin)
+
+ if (numPartitions == 1) {
+ SinglePartition
+ } else if (sortOrder.nonEmpty) {
+ RangePartitioning(sortOrder.map(_.asInstanceOf[SortOrder]), numPartitions)
Review comment:
Yes. Maybe we can support `SORTED BY` clause in `CREATE TABLE` statement in future.
https://issues.apache.org/jira/browse/SPARK-31470
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856735060
**[Test build #139467 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139467/testReport)** for PR 32781 at commit [`49422e1`](https://github.com/apache/spark/commit/49422e16f2e48c7a3511f4090c9c5009c330f126).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r648086922
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -86,11 +86,15 @@ case object ENSURE_REQUIREMENTS extends ShuffleOrigin
// Indicates that the shuffle operator was added by the user-specified repartition operator. Spark
// can still optimize it via changing shuffle partition number, as data partitioning won't change.
-case object REPARTITION extends ShuffleOrigin
+case object REPARTITION_BY_COL extends ShuffleOrigin
// Indicates that the shuffle operator was added by the user-specified repartition operator with
// a certain partition number. Spark can't optimize it.
-case object REPARTITION_WITH_NUM extends ShuffleOrigin
+case object REPARTITION_BY_NUM extends ShuffleOrigin
+
+// Indicates that the shuffle operator was added by the user-specified repartition operator. Spark
+// first tries to coalesce partitions, if it cannot be coalesced, then use the local shuffle reader.
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce small output files through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-854495823
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43848/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce small output files through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-854470002
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139326/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856678652
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43996/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856609943
**[Test build #139467 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139467/testReport)** for PR 32781 at commit [`49422e1`](https://github.com/apache/spark/commit/49422e16f2e48c7a3511f4090c9c5009c330f126).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856912084
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856762110
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139467/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855160068
**[Test build #139361 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139361/testReport)** for PR 32781 at commit [`e72dc75`](https://github.com/apache/spark/commit/e72dc75a86fc76d366badb22431da87946117d34).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857692355
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857658224
**[Test build #139554 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139554/testReport)** for PR 32781 at commit [`1c8419b`](https://github.com/apache/spark/commit/1c8419bdcd208afc40fcd838d2f8c0622caa84cb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856819576
**[Test build #139489 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139489/testReport)** for PR 32781 at commit [`d5cc086`](https://github.com/apache/spark/commit/d5cc086555d0fbfb381c919c612a27ccc0c9490f).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647491235
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -712,7 +712,9 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
case r: logical.Range =>
execution.RangeExec(r) :: Nil
case r: logical.RepartitionByExpression =>
- val shuffleOrigin = if (r.optNumPartitions.isEmpty) {
+ val shuffleOrigin = if (r.repartitionWithoutColAndNum) {
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r646684941
##########
File path: docs/sql-performance-tuning.md
##########
@@ -220,13 +220,15 @@ Coalesce hints allows the Spark SQL users to control the number of output files
tuning and reducing the number of output files. The "COALESCE" hint only has a partition number as a
parameter. The "REPARTITION" hint has a partition number, columns, or both of them as parameters.
The "REPARTITION_BY_RANGE" hint must have column names and a partition number is optional.
+The "COALESCE_PARTITION" hint do not has any parameter. It can merge output files by AQE.
Review comment:
We can have a bit more discussion about the API:
1. how about other names like `REPARTITION_BEFORE_INSERT`?
2. shall we introduce hint for the INSERT statement? It's a bit weird to have hint for write in the SELECT statement.
cc @viirya @maropu @hvanhovell
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r646403528
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -1351,6 +1351,45 @@ object RepartitionByExpression {
}
}
+/**
+ * This method repartitions data using [[Expression]]s into `numShufflePartitions` defined in
+ * `SQLConf`, and could be coalesced by AQE. Usually used to merge small files.
+ */
+case class CoalescePartitions(
+ partitionExpressions: Seq[Expression],
+ child: LogicalPlan) extends RepartitionOperation {
+
+ override val numPartitions = conf.numShufflePartitions
+
+ override val partitioning: Partitioning = {
+ val (sortOrder, nonSortOrder) = partitionExpressions.partition(_.isInstanceOf[SortOrder])
+
+ require(sortOrder.isEmpty || nonSortOrder.isEmpty,
+ s"${getClass.getSimpleName} expects that either all its `partitionExpressions` are of type " +
+ "`SortOrder`, which means `RangePartitioning`, or none of them are `SortOrder`, which " +
+ "means `HashPartitioning`. In this case we have:" +
+ s"""
+ |SortOrder: $sortOrder
+ |NonSortOrder: $nonSortOrder
+ """.stripMargin)
+
+ if (numPartitions == 1) {
+ SinglePartition
+ } else if (sortOrder.nonEmpty) {
+ RangePartitioning(sortOrder.map(_.asInstanceOf[SortOrder]), numPartitions)
Review comment:
do we want to allow ordering here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857296460
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44039/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647457345
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -712,7 +712,9 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
case r: logical.Range =>
execution.RangeExec(r) :: Nil
case r: logical.RepartitionByExpression =>
- val shuffleOrigin = if (r.optNumPartitions.isEmpty) {
+ val shuffleOrigin = if (r.repartitionWithoutColAndNum) {
Review comment:
We only call it once. Let's just write `partitionExpressions.isEmpty && optNumPartitions.isEmpty` here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855186403
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139361/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r648074672
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
See https://github.com/apache/spark/pull/32781#discussion_r647998499
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647582632
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
Are we use `_WITH_XXX` to indicate the point from which shuffle starts or comes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856649887
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856819576
**[Test build #139489 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139489/testReport)** for PR 32781 at commit [`d5cc086`](https://github.com/apache/spark/commit/d5cc086555d0fbfb381c919c612a27ccc0c9490f).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856034234
**[Test build #139424 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139424/testReport)** for PR 32781 at commit [`ba75b09`](https://github.com/apache/spark/commit/ba75b09281f04e4376b6da0b402828c33075f8d1).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class CoalescePartitions(child: LogicalPlan) extends RepartitionOperation `
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857379516
**[Test build #139514 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139514/testReport)** for PR 32781 at commit [`fe0ff1d`](https://github.com/apache/spark/commit/fe0ff1d0c05354de9b592c045bee15c5872e0514).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856117239
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43946/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857685133
thanks, merging to master!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857614283
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44080/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855186403
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139361/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855160068
**[Test build #139361 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139361/testReport)** for PR 32781 at commit [`e72dc75`](https://github.com/apache/spark/commit/e72dc75a86fc76d366badb22431da87946117d34).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857660022
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139554/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JkSelf commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r648082801
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala
##########
@@ -1782,4 +1782,36 @@ class AdaptiveQueryExecSuite
}
}
}
+
+ test("Coalesce number of partitions by AEQ") {
Review comment:
`test("SPARK-35650: Coalesce number of partitions by AEQ")`
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala
##########
@@ -1782,4 +1782,36 @@ class AdaptiveQueryExecSuite
}
}
}
+
+ test("Coalesce number of partitions by AEQ") {
+ withSQLConf(SQLConf.COALESCE_PARTITIONS_MIN_PARTITION_NUM.key -> "1") {
+ val query = "SELECT /*+ REPARTITION */ * FROM testData"
+ val (_, adaptivePlan) = runAdaptiveAndVerifyResult(query)
+ collect(adaptivePlan) {
+ case r: CustomShuffleReaderExec => r
+ } match {
+ case Seq(customShuffleReader) =>
+ assert(customShuffleReader.partitionSpecs.size === 1)
+ assert(!customShuffleReader.isLocalReader)
+ case _ =>
+ fail("There should be a CustomShuffleReaderExec")
+ }
+ }
+ }
+
+ test("Use local shuffle reader if can not coalesce number of partitions") {
Review comment:
test("SPARK-35650: Use local shuffle reader if can not coalesce number of partitions")
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856895871
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44013/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856650066
**[Test build #139473 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139473/testReport)** for PR 32781 at commit [`266d2f0`](https://github.com/apache/spark/commit/266d2f0af3324a765dc1fe6b85945f7b4a40e088).
* This patch **fails RAT tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856649785
**[Test build #139473 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139473/testReport)** for PR 32781 at commit [`266d2f0`](https://github.com/apache/spark/commit/266d2f0af3324a765dc1fe6b85945f7b4a40e088).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857582433
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44080/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce small output files through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-854502116
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43848/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647608543
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
yea, it describes the user intention and indicates how spark can optimize it without breaking user intention
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855165461
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855169500
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855173702
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JkSelf commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647952828
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
Here we have 4 cases for repartition: only num, only col, both num and col, neither num or col. Why we don't handle the both num and col case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce small output files through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-854466174
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857692355
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JkSelf commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
JkSelf commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857501209
LGTM. +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647619551
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
or `REPARTITION` -> `REPARTITION_BY_NONE`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857677696
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44090/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JkSelf commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r648083641
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -86,11 +86,15 @@ case object ENSURE_REQUIREMENTS extends ShuffleOrigin
// Indicates that the shuffle operator was added by the user-specified repartition operator. Spark
// can still optimize it via changing shuffle partition number, as data partitioning won't change.
-case object REPARTITION extends ShuffleOrigin
+case object REPARTITION_BY_COL extends ShuffleOrigin
// Indicates that the shuffle operator was added by the user-specified repartition operator with
// a certain partition number. Spark can't optimize it.
-case object REPARTITION_WITH_NUM extends ShuffleOrigin
+case object REPARTITION_BY_NUM extends ShuffleOrigin
+
+// Indicates that the shuffle operator was added by the user-specified repartition operator. Spark
+// first tries to coalesce partitions, if it cannot be coalesced, then use the local shuffle reader.
Review comment:
`first -> firstly`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647275744
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderRule.scala
##########
@@ -30,4 +31,12 @@ trait CustomShuffleReaderRule extends Rule[SparkPlan] {
* Returns the list of [[ShuffleOrigin]]s supported by this rule.
*/
def supportedShuffleOrigins: Seq[ShuffleOrigin]
+
+ /**
+ * Whether it is used to coalesce partitions through AQE.
+ */
+ def isUsedToCoalescePartitions(s: ShuffleExchangeLike): Boolean = {
Review comment:
This is too hacky. Let's add a new shuffle origin and include it in `OptimizeLocalShuffleReader.supportedShuffleOrigins`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647461185
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala
##########
@@ -1782,4 +1782,38 @@ class AdaptiveQueryExecSuite
}
}
}
+
+ test("Coalesce number of partitions by AEQ") {
+ withSQLConf(SQLConf.COALESCE_PARTITIONS_MIN_PARTITION_NUM.key -> "1") {
+ val df = sql("SELECT /*+ REPARTITION */ * FROM testData")
+ df.collect()
Review comment:
can we follow other tests?
```
val query = "SELECT /*+ REPARTITION */ * FROM testData"
val (_, adaptivePlan) = runAdaptiveAndVerifyResult(query)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857614283
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44080/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r648063132
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -712,7 +712,9 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
case r: logical.Range =>
execution.RangeExec(r) :: Nil
case r: logical.RepartitionByExpression =>
- val shuffleOrigin = if (r.optNumPartitions.isEmpty) {
+ val shuffleOrigin = if (r.partitionExpressions.isEmpty && r.optNumPartitions.isEmpty) {
+ REPARTITION_BY_NONE
+ } else if (r.optNumPartitions.isEmpty) {
REPARTITION
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856026555
**[Test build #139424 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139424/testReport)** for PR 32781 at commit [`ba75b09`](https://github.com/apache/spark/commit/ba75b09281f04e4376b6da0b402828c33075f8d1).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce small output files through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-854470002
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139326/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856649887
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43991/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857456902
**[Test build #139554 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139554/testReport)** for PR 32781 at commit [`1c8419b`](https://github.com/apache/spark/commit/1c8419bdcd208afc40fcd838d2f8c0622caa84cb).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856650077
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139473/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647639260
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
SGTM. +1 for REPARTITION_BY_NONE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647276457
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/OptimizeLocalShuffleReader.scala
##########
@@ -139,6 +139,11 @@ object OptimizeLocalShuffleReader extends CustomShuffleReaderRule {
def canUseLocalShuffleReader(plan: SparkPlan): Boolean = plan match {
case s: ShuffleQueryStageExec =>
s.mapStats.isDefined && supportLocalReader(s.shuffle)
+ case CustomShuffleReaderExec(s: ShuffleQueryStageExec, partitionSpecs)
+ if isUsedToCoalescePartitions(s.shuffle) =>
+ // Use LocalShuffleReader only when we can't CoalesceShufflePartitions
Review comment:
Why? The general rule is to always use the local reader if possible. If coalesce happens first, local reader will respect the parallelism.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce small output files through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-854466174
**[Test build #139326 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139326/testReport)** for PR 32781 at commit [`2c21944`](https://github.com/apache/spark/commit/2c2194468c64d8799ffd8950e3886e5fa0d1c645).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647283102
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderRule.scala
##########
@@ -30,4 +31,12 @@ trait CustomShuffleReaderRule extends Rule[SparkPlan] {
* Returns the list of [[ShuffleOrigin]]s supported by this rule.
*/
def supportedShuffleOrigins: Seq[ShuffleOrigin]
+
+ /**
+ * Whether it is used to coalesce partitions through AQE.
+ */
+ def isUsedToCoalescePartitions(s: ShuffleExchangeLike): Boolean = {
Review comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857502656
**[Test build #139563 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139563/testReport)** for PR 32781 at commit [`6424885`](https://github.com/apache/spark/commit/6424885eb94159f8f16ba3b3ed2afeb5565a11f0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r646673351
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -1351,6 +1351,45 @@ object RepartitionByExpression {
}
}
+/**
+ * This method repartitions data using [[Expression]]s into `numShufflePartitions` defined in
+ * `SQLConf`, and could be coalesced by AQE. Usually used to merge small files.
+ */
+case class CoalescePartitions(
+ partitionExpressions: Seq[Expression],
+ child: LogicalPlan) extends RepartitionOperation {
+
+ override val numPartitions = conf.numShufflePartitions
+
+ override val partitioning: Partitioning = {
+ val (sortOrder, nonSortOrder) = partitionExpressions.partition(_.isInstanceOf[SortOrder])
+
+ require(sortOrder.isEmpty || nonSortOrder.isEmpty,
+ s"${getClass.getSimpleName} expects that either all its `partitionExpressions` are of type " +
+ "`SortOrder`, which means `RangePartitioning`, or none of them are `SortOrder`, which " +
+ "means `HashPartitioning`. In this case we have:" +
+ s"""
+ |SortOrder: $sortOrder
+ |NonSortOrder: $nonSortOrder
+ """.stripMargin)
+
+ if (numPartitions == 1) {
+ SinglePartition
+ } else if (sortOrder.nonEmpty) {
+ RangePartitioning(sortOrder.map(_.asInstanceOf[SortOrder]), numPartitions)
Review comment:
It needs to set its own `ShuffleOrigin`:
https://github.com/apache/spark/blob/ba75b09281f04e4376b6da0b402828c33075f8d1/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala#L721-L722
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856912084
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856762110
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139467/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647461751
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala
##########
@@ -1782,4 +1782,38 @@ class AdaptiveQueryExecSuite
}
}
}
+
+ test("Coalesce number of partitions by AEQ") {
+ withSQLConf(SQLConf.COALESCE_PARTITIONS_MIN_PARTITION_NUM.key -> "1") {
+ val df = sql("SELECT /*+ REPARTITION */ * FROM testData")
+ df.collect()
+
+ collect(df.queryExecution.executedPlan) {
+ case r: CustomShuffleReaderExec => r
+ } match {
+ case Seq(customShuffleReader) =>
+ assert(customShuffleReader.partitionSpecs.size === 1)
+ assert(!customShuffleReader.isLocalReader)
+ case _ =>
+ fail("There should be a CustomShuffleReaderExec")
+ }
+ }
+ }
+
+ test("Use local shuffle reader if can not coalesce number of partitions") {
+ withSQLConf(SQLConf.ADVISORY_PARTITION_SIZE_IN_BYTES.key -> "2") {
+ val df = sql("SELECT /*+ REPARTITION */ * FROM testData")
+ df.collect()
Review comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857281347
**[Test build #139514 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139514/testReport)** for PR 32781 at commit [`fe0ff1d`](https://github.com/apache/spark/commit/fe0ff1d0c05354de9b592c045bee15c5872e0514).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856072193
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43946/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647582632
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
Are we use `_WITH_XXX` to indicate the point from which shuffle starts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-855173702
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647459794
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
We can thing about the naming a bit more. e.g. `REPARTITION_WITH_COL`, `REPARTITION_WITH_NUM`, `REPARTITION_WITH_NOTHING`
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
We can think about the naming a bit more. e.g. `REPARTITION_WITH_COL`, `REPARTITION_WITH_NUM`, `REPARTITION_WITH_NOTHING`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #32781:
URL: https://github.com/apache/spark/pull/32781
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857654044
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44090/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647272859
##########
File path: docs/sql-performance-tuning.md
##########
@@ -218,7 +218,7 @@ For more details please refer to the documentation of [Join Hints](sql-ref-synta
Coalesce hints allows the Spark SQL users to control the number of output files just like the
`coalesce`, `repartition` and `repartitionByRange` in Dataset API, they can be used for performance
tuning and reducing the number of output files. The "COALESCE" hint only has a partition number as a
-parameter. The "REPARTITION" hint has a partition number, columns, or both of them as parameters.
+parameter. The "REPARTITION" hint has a partition number, columns, or both of them as parameters or neither of them as parameters.
Review comment:
`a partition number, columns, or both/neither of them as parameters.`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647618366
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
##########
@@ -92,6 +92,11 @@ case object REPARTITION extends ShuffleOrigin
// a certain partition number. Spark can't optimize it.
case object REPARTITION_WITH_NUM extends ShuffleOrigin
+// Indicates that the shuffle operator was added by the user-specified repartition operator.
+// Spark first tries to coalesce partitions, if it cannot be coalesced, then try to use the
+// local shuffle reader.
+case object REPARTITION_WITHOUT_COL_AND_NUM extends ShuffleOrigin
Review comment:
How about `REPARTITION_BY_COL`, `REPARTITION_BY_NUM`, `REPARTITION`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857385141
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139514/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857320602
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44039/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856034289
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139424/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32781:
URL: https://github.com/apache/spark/pull/32781#discussion_r647998499
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -712,7 +712,9 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
case r: logical.Range =>
execution.RangeExec(r) :: Nil
case r: logical.RepartitionByExpression =>
- val shuffleOrigin = if (r.optNumPartitions.isEmpty) {
+ val shuffleOrigin = if (r.partitionExpressions.isEmpty && r.optNumPartitions.isEmpty) {
+ REPARTITION_BY_NONE
+ } else if (r.optNumPartitions.isEmpty) {
REPARTITION
Review comment:
@JkSelf I think we don't need REPARTITION_BY_COL_AND_NUM, because REPARTITION_BY_NUM already indicates no optimization can be done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Coalesce partitions through AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-856096486
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43946/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857502656
**[Test build #139563 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139563/testReport)** for PR 32781 at commit [`6424885`](https://github.com/apache/spark/commit/6424885eb94159f8f16ba3b3ed2afeb5565a11f0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32781: [SPARK-35650][SQL] Enhance `RepartitionByExpression` to make it coalesce partitions efficiently by AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32781:
URL: https://github.com/apache/spark/pull/32781#issuecomment-857385141
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139514/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org