kerwin-zk commented on PR #8084: URL: https://github.com/apache/incubator-gluten/pull/8084#issuecomment-2507033809
> @kerwin-zk > > > Moreover, from a certain perspective, the introduction of GlutenShuffleManager couples the various ShuffleManagers together, whereas the current fully decoupled setup seems clearer. > > I haven't clearly gotten what's the new code coupling come from. Say ideally we can remove the proxied calls from RSS shuffle managers to default shuffle manager because of this work (each shuffle manager can be registered for certain shuffle dependency type it can handle). So I see this a decoupling of code rather than coupling of code. > > > Essentially, this is no different from the current setup where spark.shuffle.manager is configured to different managers. > > But I may understand your concern totally especially this one. The reason I am proposing this is not simply because of code refactor. The solution's kind of a trade-off for letting user enable different backends at the same time. See the umbrella #6920. For that purpose, we have to develop a way to make the shuffle managers be registered together so Gluten could choose the right one to use. For example, if we introduce GPU backend, we should give back the flexibility to developers when they decide not to let GPU backend rely on Velox backend, but they may still want query planner use Velox shuffle when the query plan is not offloaded to GPU. > > May be we can put off deprecating the current use of `CelebornShuffleManager` / `UniffleShuffleManager`? We can keep the support for the legacy configurations during a considerable long period. I agree this is very normal that a user wants to specify a shuffle manager via Spark config explicitly. Does that work for you? @zhztheplayer Thank you very much for your detailed explanation. I believe that #6920 indeed requires this feature. At the same time, retaining the current option that allows users to specify the ShuffleManager through configuration is also necessary. I think this is acceptable. -- 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]
