zjuwangg commented on code in PR #10616:
URL:
https://github.com/apache/incubator-gluten/pull/10616#discussion_r2336854429
##########
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala:
##########
@@ -553,30 +553,19 @@ class VeloxSparkPlanExecApi extends SparkPlanExecApi {
partitioning: Partitioning,
output: Seq[Attribute]): ShuffleWriterType = {
val conf = GlutenConfig.get
- // todo: remove isUseCelebornShuffleManager here
- if (conf.isUseCelebornShuffleManager) {
- if (conf.celebornShuffleWriterType ==
ReservedKeys.GLUTEN_SORT_SHUFFLE_WRITER) {
- if (conf.useCelebornRssSort) {
- RssSortShuffleWriterType
- } else if (partitioning != SinglePartition) {
+ SparkEnv.get.shuffleManager match {
+ case shuffleManager: NeedCustomShuffleWriterType =>
Review Comment:
> In my opinion, the implementation of sort shuffle writer type for
velox+celeborn is a special case. Perhaps we can add a function like
`getShuffleWriterTypeForCeleborn` to the BackendsApi to handle this special
case, and call it in the `getColumnarShuffleWriterType` in
`CelebornShuffleManager`.
Do you mean in this way
https://github.com/apache/incubator-gluten/pull/10616/commits/47f2edc4789a3c19e014b48f30984bca986a2ce7
?
I still think `getColumnarShuffleWriterType` can not put in the
`SupportsColumnarShuffle ` but need a separate trait to mark shuffle manage
need to do special process
--
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]
