c21 commented on pull request #35552: URL: https://github.com/apache/spark/pull/35552#issuecomment-1044220919
> Would this PR's config will be useful, when combined with your internal work mentioned here? https://github.com/apache/spark/pull/28804#issuecomment-854089520 @sigmod - maybe, but it still not covers all cases. Note the feature to skip partial aggregate is only for code-gen mode of hash aggregate. For other code paths (iterator mode of hash aggregate, object hash aggregate, sort aggregate), this is not covered. Though technically, nothing stops us to add support for all code paths. Based on our experience, the runtime skipping feature is useful but not panacea. We still have to pay some cost for partial aggregate to process some number of rows before realizing we can skip. > Say, key1 is gender and key2 is customerId, parallelism 2 is too low even if you can smartly skip partial agg. In addition, one can argue the manually tuned config might be not that useful. Let's just imagine the debugging workflow here. When users/developers realize a query has this data skew issue. Without this newly added config, we can also work around the issue by several options: (1).disable bucketing config if bucketed table is skewed, (2).remove unnecessary repartition() in query which caused skew, (3).add a repartition() on full group-by keys / `DISTRIBUTE BY (...)` in SQL query before aggregate. So I feel this seems to render the config useless. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
