tgravescs commented on pull request #34672: URL: https://github.com/apache/spark/pull/34672#issuecomment-1009002175
> If there is a better, actionable solution, I would love to explore more. The ideal solution is what I've been saying, Spark actually makes a public pluggable interface for this. Your entire plugin could be consider a "hack" because it overrides a private Spark api. the shuffle manager config was initially created to switch between internal Spark shuffle manager implementations, not for external 3rd party implementations. I assume spark is creating an ExternalBlockStoreClient for this if you have the external shuffle service enabled. That api assumes that you have implemented a certain api and registration is one of them. I guess you probably override the shuffle reader so maybe its not really used for much else. But then why are we creating a ExternalBlockStoreClient (For this case should there be some api change that doesn't create it at all.).. again goes back to making a proper interface for this. I do understand that is a much bigger chunk of work, which is why I said I was on the fence about it. If it has enough impact and there is no other way around it perhaps we let it in short term. But at the same time if we keep hacking on this private interface to allow this or that and doing temporary fixes it will never get done properly. If its something you can do on the remote shuffle side already then it doesn't require yet another hack in Spark itself. How are you currently hacking it to work now? > this is a common problem Where else is this a problem? -- 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]
