You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@uniffle.apache.org by "zuston (via GitHub)" <gi...@apache.org> on 2023/04/23 05:59:36 UTC
[GitHub] [incubator-uniffle] zuston opened a new issue, #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
zuston opened a new issue, #838:
URL: https://github.com/apache/incubator-uniffle/issues/838
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
### Search before asking
- [X] I have searched in the [issues](https://github.com/apache/incubator-uniffle/issues?q=is%3Aissue) and found no similar issues.
### What would you like to be improved?
In #630 , we disable the `localShuffleRead` for AQE in spark. For a pity, sometimes the partial partition is so huge, but the broadcastHashJoin won't optimize skew join. This optimization will be solved by spark community ASSP.
After that, I will propose a new patch for uniffle to better support Spark older version.
### How should we improve?
_No response_
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
--
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: dev-unsubscribe@uniffle.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1518999362
> This optimization will be solved by spark community ASSP. After that, I will propose a new patch for uniffle to better support Spark older version.
I mean this optimization should be proposed to Spark community. After that, I will propose a new patch to older spark version to support this optimization.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519081232
> In my opinion, this is just another optimize SkewJoin rule, which should be integrated into your internal Spark code if it's not merged into Spark upstream yet. Optimizing skew joins is out of the scope of Uniffle project. However, if these optimization rules is well maintained elsewhere out of Uniffle project(maybe kyubi?), I believe the community is more than happy to add it to the tuning document in details.
Yes. I have to re-claim the thought, just to share some performance ideas for Spark & Uniffle. I’m OK for keeping this patch out of uniffle.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519005619
https://github.com/apache/incubator-uniffle/tree/master/patch/spark. Like this.
I don't quite understand what is unclear about my expression.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519069871
> https://github.com/apache/kyuubi/blob/master/extensions/spark/kyuubi-extension-spark-3-3/src/main/scala/org/apache/kyuubi/sql/InsertShuffleNodeBeforeJoin.scala#L38
Thanks for pointing to this. It's interesting and clever way to add new rules and syntax using Spark extension mechanism..
> Oh, it has been proposed but still reviewing. https://github.com/apache/spark/pull/40312
In my opinion, this is just another optimize SkewJoin rule, which should be integrated into your internal Spark code if it's not merged into Spark upstream yet. Optimizing skew joins is out of the scope of Uniffle project. However, if these optimization rules is well maintained elsewhere out of Uniffle project(maybe kyubi?), I believe the community is more than happy to add it to the tuning document in details.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] jerqi commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519004835
> like spark3.1.1
I wanna ask you how not what.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519041507
> For a pity, sometimes the partial partition is so huge, but the broadcastHashJoin won't optimize skew join. This optimization will be solved by spark community ASSP
If skewed join should not be converted into broadcast join, users should disable broadcast join or lower threshold for broadcast join by tweaking relating configurations.
I didn't see how that a spark patch is related, even if the patch was to release users from manually setting here, I don't think the patch belongs to Uniffle's project. However we can add documentations about that cases.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] pan3793 commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519061061
Kyuubi project tackles this issue by allowing to forcibly shuffle before the join operator https://github.com/apache/kyuubi/blob/master/extensions/spark/kyuubi-extension-spark-3-3/src/main/scala/org/apache/kyuubi/sql/InsertShuffleNodeBeforeJoin.scala#L38
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] jerqi commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1518961371
I can't get your point. If you want to push a Spark's feature patch to `patch` directory of project Uniffle, I will vote -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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] jerqi commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519011256
You can modify the document https://github.com/apache/incubator-uniffle-website/blob/master/docs/07-tuning.md.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519082322
> Kyuubi project tackles this issue by allowing to forcibly shuffle before the join operator https://github.com/apache/kyuubi/blob/master/extensions/spark/kyuubi-extension-spark-3-3/src/main/scala/org/apache/kyuubi/sql/InsertShuffleNodeBeforeJoin.scala#L38
Good way!
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] jerqi commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519006868
> https://github.com/apache/incubator-uniffle/tree/master/patch/spark. Like this.
>
> I don't quite understand what is unclear about my expression.
I reject. Because we should avoid modifying the Spark code. It's unrelated to Uniffle although it will improve the performance of Uniffle. If you need this, you should cherry-pick this feature to your internal branch.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519010257
> I reject. Because we should avoid modifying the Spark code. It's unrelated to Uniffle although it will improve the performance of Uniffle. If you need this, you should cherry-pick this feature to your internal branch.
It's OK.
The reason of proposing this is to expose the problem of using uniffle(RSS) with spark AQE poor performance. Those using uniffle should pay more attension on this. And all optimization about this could be tracked here.
If you reject this, we could keep this issue open and attach the related spark optimization PR here if exists.
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519012171
Let's discuss first if anyone is interested
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1518953252
cc @advancedxy @jerqi
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519064664
Oh, it has been proposed but still reviewing. https://github.com/apache/spark/pull/40312
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] jerqi commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519000500
> > This optimization will be solved by spark community ASSP. After that, I will propose a new patch for uniffle to better support Spark older version.
>
> I mean this optimization should be proposed to Spark community. After that, I will propose a new patch to older spark version to support this optimization.
How do you support the old spark versions?
--
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: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #838: [Improvement] Better support for AQE sortMergeJoin -> broadcastHashJoin
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #838:
URL: https://github.com/apache/incubator-uniffle/issues/838#issuecomment-1519000771
like spark3.1.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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org