tgravescs commented on pull request #34672: URL: https://github.com/apache/spark/pull/34672#issuecomment-1012225659
It does feel a bit odd that ShuffleBlockResolver doesn't really have any real apis but I get what you are going for here. They aren't necessarily needed for other implementations. Its weird now though too because ShuffleWriteProcessor would access the shuffle block resolver but its behind a feature flag and there is explicitly checks for specific IndexShuffleBlockResolver impl. Which really isn't ideal in the sense if someone wanted to implement a shuffle manager that supported shuffle merging, they have no way to tie in there. I definitely think if we have basically 4 implementations of the shuffle we should make an api that fits and get something done rather then getting caught up in trying to cover all future cases. so looking at other things the ExternalBlockStoreClient is not part of the shuffle manager, the intention there would be to keep like the api introduced in this PR? Just trying to go through what all changes may be required. -- 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]
