tgravescs commented on code in PR #43627:
URL: https://github.com/apache/spark/pull/43627#discussion_r1380262372
##########
core/src/main/scala/org/apache/spark/SparkEnv.scala:
##########
@@ -423,7 +425,6 @@ object SparkEnv extends Logging {
conf,
memoryManager,
Review Comment:
there is a comment above that says " // NB: blockManager is not valid
until initialize() is called later." We may want to update that to mention
having to set shuffle manager
##########
core/src/main/scala/org/apache/spark/storage/BlockManager.scala:
##########
@@ -186,12 +186,14 @@ private[spark] class BlockManager(
val conf: SparkConf,
memoryManager: MemoryManager,
mapOutputTracker: MapOutputTracker,
- shuffleManager: ShuffleManager,
val blockTransferService: BlockTransferService,
securityManager: SecurityManager,
externalBlockStoreClient: Option[ExternalBlockStoreClient])
extends BlockDataManager with BlockEvictionHandler with Logging {
+ // this is set after the ShuffleManager is instantiated in SparkContext and
Executor
+ private var shuffleManager: ShuffleManager = _
Review Comment:
update description above to mention having to set the shuffle manager as
well.
##########
core/src/main/scala/org/apache/spark/SparkEnv.scala:
##########
@@ -71,6 +69,9 @@ class SparkEnv (
val outputCommitCoordinator: OutputCommitCoordinator,
val conf: SparkConf) extends Logging {
+ // We set the ShuffleManager in SparkContext and Executor
Review Comment:
nit update comment to say something like: the ShuffleManager is initialized
later in... to allow it being defined in user specified jars.
##########
core/src/test/scala/org/apache/spark/storage/BlockManagerReplicationSuite.scala:
##########
@@ -104,8 +105,8 @@ trait BlockManagerReplicationBehavior extends SparkFunSuite
val blockManagerInfo = new mutable.HashMap[BlockManagerId,
BlockManagerInfo]()
master = new BlockManagerMaster(rpcEnv.setupEndpoint("blockmanager",
new BlockManagerMasterEndpoint(rpcEnv, true, conf,
- new LiveListenerBus(conf), None, blockManagerInfo, mapOutputTracker,
sc.env.shuffleManager,
- isDriver = true)),
+ new LiveListenerBus(conf), None, blockManagerInfo, mapOutputTracker,
+ sc.env.shuffleManager.shuffleBlockResolver.getBlocksForShuffle, true)),
Review Comment:
put back the isDriver = true as last parameter
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
