Skip to content

Conversation

@attilapiros
Copy link
Contributor

What changes were proposed in this pull request?

Cleaning up ShuffleBlockResolver from polluted methods to create a developer API.

Why are the changes needed?

ShuffleBlockResolver is intended to be part of a generic Shuffle API but currently it contains local disk specific (and pushed based shuffle) methods.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing unit tests.

@hiboyang
Copy link

Thanks for the PR!

@attilapiros attilapiros changed the title [SPARK-37881][CORE][WIP] Cleanup ShuffleBlockResolver from polluted methods to create a developer API [SPARK-37881][CORE] Cleanup ShuffleBlockResolver from polluted methods to create a developer API Jan 14, 2022
@mridulm
Copy link
Contributor

mridulm commented Jan 16, 2022

If we only have stop in ShuffleBlockResolver in this proposed api.
How would a custom resolver be used in/integrate with spark ?

@attilapiros
Copy link
Contributor Author

My idea for a follow-up PR is to introduce interfaces along the capabilities so instead of the IndexShuffleBlockResolver we would cast to those capabilities.
Example SameHostStoredShuffleBlockResolver with the getBlockData method.
In this case when a custom shuffle manager gives back a block resolver which implements this interface the getBlockData can be used by the block manager.

But right now I am busy with the MapStatus (I would like to open a PR for the MapStatus next week).
We can let this PR open and when I have time I can proceed with the capability based solution, although I still think this cleanup is useful on its own as stabilizes the ShuffleBlockResolver which can cause build error in the 3rd party solutions.

@mridulm
Copy link
Contributor

mridulm commented Jan 18, 2022

I am fine with either direction you want to take @attilapiros - either we can merge this PR or make it WIP and fix MapStatus/surrounding infra before circling back to this - your call !

@github-actions
Copy link

github-actions bot commented May 8, 2022

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label May 8, 2022
@github-actions github-actions bot closed this May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants