zjuwangg commented on PR #10201: URL: https://github.com/apache/incubator-gluten/pull/10201#issuecomment-3112605405
> Thanks for working on this @zjuwangg. > > The solution looks promising, except that it adds some layers of complexity (e.g., more reflections) which needs extra effort from reader / user. I may need to have a deeper look to understand the cost of refactoring the existing code. Let me know if you have relevant insights. Sure, it indeed introduces complexity. Now we have gluten-common/ backend-specific-impl / rss-specific-impl, and backend depends on gluten-common, while rss-specific-impl depends on backends. Current design `VeloxCelebornColumnarBatchSerlizerFactory` works just like the way of `VeloxCelebornColumnarShuffleWriterFactory` does. Besides that, we can also leverage `CelebornShuffleWriterFactory` interface, add the columnarBatchSeriliazer interface and change the interface name to reduce reflections. In the long run, we may also can extract RSS-related configuration and behavior like backend. We may need introduce CelebornShuffleBackend and UniffleShuffleBackend and pass current job shuffle backend to execution-backend to do more specific backend-related and rss-related config. -- 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]
