Skip to content

Conversation

@JoshRosen
Copy link
Contributor

This is intended to fix SPARK-2977. Before, there was an implicit ordering dependency where we needed to know the ShuffleManager implementation before creating the ShuffleBlockManager. This patch makes that dependency explicit by adding ShuffleManager to a bunch of constructors.

I think it's a little odd for BlockManager to take a ShuffleManager only to pass it to ShuffleBlockManager without using it itself; there's an opportunity to clean this up later if we sever the circular dependencies between BlockManager and other components and pass those components to BlockManager's constructor.

…ger.

This is intended to fix SPARK-2977.  Before, there was an implicit ordering
dependency where we needed to know the ShuffleManager implementation before
creating the ShuffleBlockManager.  This patch makes that dependency explicit
by adding ShuffleManager to a bunch of constructors.

I think it's a little odd for BlockManager to take a ShuffleManager only to
pass it to ShuffleBlockManager without using it itself; there's an opportunity
to clean this up later if we sever the circular dependencies between
BlockManager and other components and pass those components to BlockManager's
constructor.
@SparkQA
Copy link

SparkQA commented Aug 15, 2014

QA tests have started for PR 1976 at commit a9cd1e1.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 16, 2014

QA tests have finished for PR 1976 at commit a9cd1e1.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class ShuffleBlockManager(blockManager: BlockManager,

@JoshRosen
Copy link
Contributor Author

/cc @mateiz for review (this is the issue I mentioned in #1799).

@mateiz
Copy link
Contributor

mateiz commented Aug 16, 2014

Looks good to me, feel free to merge it.

@asfgit asfgit closed this in 20fcf3d Aug 16, 2014
asfgit pushed a commit that referenced this pull request Aug 16, 2014
This is intended to fix SPARK-2977.  Before, there was an implicit ordering dependency where we needed to know the ShuffleManager implementation before creating the ShuffleBlockManager.  This patch makes that dependency explicit by adding ShuffleManager to a bunch of constructors.

I think it's a little odd for BlockManager to take a ShuffleManager only to pass it to ShuffleBlockManager without using it itself; there's an opportunity to clean this up later if we sever the circular dependencies between BlockManager and other components and pass those components to BlockManager's constructor.

Author: Josh Rosen <[email protected]>

Closes #1976 from JoshRosen/SPARK-2977 and squashes the following commits:

a9cd1e1 [Josh Rosen] [SPARK-2977] Ensure ShuffleManager is created before ShuffleBlockManager.

(cherry picked from commit 20fcf3d)
Signed-off-by: Josh Rosen <[email protected]>
xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
This is intended to fix SPARK-2977.  Before, there was an implicit ordering dependency where we needed to know the ShuffleManager implementation before creating the ShuffleBlockManager.  This patch makes that dependency explicit by adding ShuffleManager to a bunch of constructors.

I think it's a little odd for BlockManager to take a ShuffleManager only to pass it to ShuffleBlockManager without using it itself; there's an opportunity to clean this up later if we sever the circular dependencies between BlockManager and other components and pass those components to BlockManager's constructor.

Author: Josh Rosen <[email protected]>

Closes apache#1976 from JoshRosen/SPARK-2977 and squashes the following commits:

a9cd1e1 [Josh Rosen] [SPARK-2977] Ensure ShuffleManager is created before ShuffleBlockManager.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants