JoshRosen commented on code in PR #49350:
URL: https://github.com/apache/spark/pull/49350#discussion_r1908117844
##########
core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala:
##########
@@ -402,6 +402,20 @@ final class ShuffleBlockFetcherIterator(
val fallback = FallbackStorage.FALLBACK_BLOCK_MANAGER_ID.executorId
val localExecIds = Set(blockManager.blockManagerId.executorId, fallback)
+
+ @inline def isHostLocal(blockAddress: BlockManagerId): Boolean = {
+ if (blockManager.hostLocalDirManager.isDefined
+ && blockAddress.host == blockManager.blockManagerId.host) {
+ if (blockManager.isLocalSparkCluster &&
blockManager.externalShuffleServiceEnabled) {
+ blockAddress.hostPort == blockManager.shuffleServerId.hostPort
+ } else {
+ true
+ }
+ } else {
+ false
+ }
+ }
Review Comment:
I think we can do
```suggestion
val isTestingExternalShuffleInLocalClusterMode = {
Utils.isTesting &&
blockManager.externalShuffleServiceEnabled &&
blockManager.conf.getOption("spark.master").exists(_.startsWith("local-cluster"))
}
@inline def isHostLocal(blockAddress: BlockManagerId): Boolean = {
if (blockManager.hostLocalDirManager.isDefined
&& blockAddress.host == blockManager.blockManagerId.host) {
if (isTestingExternalShuffleInLocalClusterMode) {
blockAddress.hostPort == blockManager.shuffleServerId.hostPort
} else {
true
}
} else {
false
}
}
```
which mirrors existing logic at
https://github.com/apache/spark/blob/bbb8aca0b51008bf65ba8f9232ba96c166e84f8e/core/src/main/scala/org/apache/spark/resource/yesourceProfile.scala#L404-L407
for directly checking whether we're in local cluster mode.
This should let us roll back the changes in `BlockManager.scala`.
It also makes it more apparent to readers that this path is only hit in
tests, which was a bit obscured before: to figure that out, a reader would have
to carefully parse the definition in BlockManager.
--
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]
