viirya commented on a change in pull request #33172:
URL: https://github.com/apache/spark/pull/33172#discussion_r662409931



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -525,8 +525,31 @@ object SQLConf {
       .booleanConf
       .createWithDefault(true)
 
+  private val MIN_PARTITION_SIZE_KEY = 
"spark.sql.adaptive.coalescePartitions.minPartitionSize"
+
+  val COALESCE_PARTITIONS_PARALLELISM_FIRST =
+    buildConf("spark.sql.adaptive.coalescePartitions.parallelismFirst")
+      .doc("When true, Spark will coalesce contiguous shuffle partitions with 
" +
+        s"'$MIN_PARTITION_SIZE_KEY' as the target size, instead of " +
+        s"'${ADVISORY_PARTITION_SIZE_IN_BYTES.key}', to maximize the " +
+        "parallelism. This is to avoid performance regression when enabling 
adaptive query " +
+        "execution. It's recommended to set this config to false and use " +

Review comment:
       It explains benefits of setting it as true. But it doesn't explain when 
it is good to set it as false or why it is recommended to set it as false. 
Maybe a bit hard for user to follow and understand. 
   
   Can we add a few words explaining why it is good to set this as false?




-- 
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.

To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to